Skip to content

Commit 4149ac8

Browse files
AntikodeAntikode
authored andcommitted
fix: fill-form dropdown skip-check can't distinguish placeholder from a real blank-value PDF option
The write-back skip logic compared only el.value against initialValue, but a PDF dropdown option whose own value is "" is indistinguishable from the synthetic "(unset)" placeholder via .value alone (both are ""). This silently discarded a user's explicit selection of that real blank option whenever the PDF had no prior selection -- reintroducing the exact bug #299 fixed, via #326's switch from a selectedIndex-based check to a value-based one. Fixed by tracking "PDF had no selection" as undefined (distinct from a real "" value) in both renderFields() and fillForm(), and using selectedIndex === 0 only to detect the untouched-placeholder case.
1 parent 48f9f83 commit 4149ac8

1 file changed

Lines changed: 16 additions & 10 deletions

File tree

fill-form.js

Lines changed: 16 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,10 @@ function renderFields() {
137137
} else if (f.kind === "dropdown") {
138138
const row = document.createElement("div");
139139
row.className = "field-row";
140-
const currentValue = (f.selected && f.selected[0]) || "";
140+
// undefined (not "") means the PDF has no current selection — kept distinct
141+
// from a real PDF option whose own value happens to be the empty string, so
142+
// the synthetic "(unset)" placeholder never collides with that real option.
143+
const currentValue = f.selected && f.selected.length > 0 ? f.selected[0] : undefined;
141144
const options = (f.options || [])
142145
.map(
143146
(opt) =>
@@ -146,7 +149,7 @@ function renderFields() {
146149
.join("");
147150
row.innerHTML = `
148151
<label for="${id}">${escapeHtml(f.name)}</label>
149-
<select id="${id}"><option value=""${currentValue === "" ? " selected" : ""}>(unset)</option>${options}</select>
152+
<select id="${id}"><option value=""${currentValue === undefined ? " selected" : ""}>(unset)</option>${options}</select>
150153
`;
151154
fieldsContainer.appendChild(row);
152155
} else if (f.kind === "optionlist") {
@@ -327,14 +330,17 @@ async function fillForm() {
327330
} else if (f.kind === "dropdown") {
328331
const el = document.getElementById(id);
329332
if (!el) return;
330-
const initialValue = (f.selected && f.selected[0]) || "";
331-
// el.value === "" is always the synthetic "(unset)" placeholder — never a
332-
// real PDF option value — so it's always skipped, matching renderFields()'s
333-
// real options never carrying value="". el.value === initialValue means the
334-
// dropdown is untouched (or re-set to its own pre-existing selection): skip
335-
// rather than re-select, so a field the user never interacted with can never
336-
// trigger assertEncodable() on a non-Latin option the PDF's own author chose.
337-
if (el.value === "" || el.value === initialValue) return;
333+
// undefined means the PDF had no selection — kept distinct from a real PDF
334+
// option whose own value is "", matching renderFields()'s sentinel so a user
335+
// who explicitly picks that real blank option isn't mistaken for someone who
336+
// left the synthetic "(unset)" placeholder untouched (el.value alone can't
337+
// tell the two apart, since both carry value="").
338+
const initialValue = f.selected && f.selected.length > 0 ? f.selected[0] : undefined;
339+
// selectedIndex 0 is always the synthetic placeholder (real options never
340+
// occupy index 0) — only treat it as untouched when the PDF genuinely had no
341+
// selection; otherwise fall through so a real current value (including "")
342+
// is still correctly recognized as unchanged via the value comparison.
343+
if ((el.selectedIndex === 0 && initialValue === undefined) || el.value === initialValue) return;
338344
assertEncodable(el.value, f.name);
339345
field.select(el.value);
340346
filledCount++;

0 commit comments

Comments
 (0)