fix(pdf-server): form-field save robustness#591
Merged
Conversation
#577 dropped the per-field try/catch in buildAnnotatedPdfBytes when it swapped to type-dispatch, so the first field whose pdf-lib write throws (max-length text, missing /Yes appearance, radio buttonValue mapping to neither label nor index, ...) bubbled to the outer catch and silently dropped every subsequent field. Compounded by getAnnotatedPdfBytes passing every baseline field, so even an untouched problematic field in the PDF poisoned the whole save. - Re-wrap each field write in its own try/catch (warn + continue). - getAnnotatedPdfBytes now only sends fields whose value differs from baseline (still includes explicit ''/false sentinels for cleared fields so pdf-lib overwrites the original /V). - Regression test: a maxLength=2 text field fed a long string, followed by a normal field; assert the second one still lands.
@modelcontextprotocol/ext-apps
@modelcontextprotocol/server-basic-preact
@modelcontextprotocol/server-basic-react
@modelcontextprotocol/server-basic-solid
@modelcontextprotocol/server-basic-svelte
@modelcontextprotocol/server-basic-vanillajs
@modelcontextprotocol/server-basic-vue
@modelcontextprotocol/server-budget-allocator
@modelcontextprotocol/server-cohort-heatmap
@modelcontextprotocol/server-customer-segmentation
@modelcontextprotocol/server-debug
@modelcontextprotocol/server-map
@modelcontextprotocol/server-pdf
@modelcontextprotocol/server-scenario-modeler
@modelcontextprotocol/server-shadertoy
@modelcontextprotocol/server-sheet-music
@modelcontextprotocol/server-system-monitor
@modelcontextprotocol/server-threejs
@modelcontextprotocol/server-transcript
@modelcontextprotocol/server-video-resource
@modelcontextprotocol/server-wiki-explorer
commit: |
…hosen widget Some PDFs (e.g. third-party forms like the IRS f1040 family, and the demo Form.pdf) omit the /Ff Radio flag bit on button fields, so pdf-lib classifies a multi-widget radio as PDFCheckBox. The viewer stores pdf.js's buttonValue (the widget's on-state name, e.g. '0'/'1') as a string. The PDFCheckBox branch did 'if (value) field.check()', which always sets the FIRST widget's on-state - so any choice saved as the first option. When the value is a string on a PDFCheckBox, treat it as a radio on-value: write /V and per-widget /AS directly via the low-level acroField (mirroring PDFAcroRadioButton.setValue minus its first-widget-only onValues guard). Booleans keep check()/uncheck(). Test: build a radio fixture, clear the Radio flag so reload sees PDFCheckBox, save with '1', assert /V = /1 and second widget /AS = /1.
clearFieldInStorage wrote the same string clearValue to every widget's
storage, hitting pdf.js's inverted string-coercion bug (the one
setFieldInStorage already works around): the wrong widget rendered
checked. For radio, write {value:false} to every widget instead.
… radio A single-widget checkbox getting a string value (e.g. 'Yes'/'Off' if something upstream stores the export string instead of boolean) was being routed through setButtonGroupValue, which no-ops when no widget's on-state matches the value - leaving the box unchanged. Gate the radio-path on widgets.length > 1; single-widget falls through to check()/uncheck() with liberal truthy/'Off' semantics.
PDFs whose checkbox/radio widgets lack an /AP/N/<onValue> stream: check() sets /V and /AS but Preview/Acrobat can't render a state with no appearance, so the field looks unchanged. Call form.updateFieldAppearances() after the write loop so pdf-lib synthesizes defaults for dirty fields.
Language field in Form.pdf is a PDFOptionList; the save loop skipped it. Parse the viewer's comma-joined string, select() the subset that exists in getOptions() (pdf-lib throws on unknowns and there's no edit-mode fallback like Dropdown has).
target.value on a multiselect is only the first option; join selectedOptions so the save path gets the full set.
ochafik
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
buildAnnotatedPdfBytesform loop:try/catchso one throwing write doesn't abort the restPDFCheckBox(missing/Ffflag): write/V+ per-widget/ASdirectly viasetButtonGroupValue(); gated onwidgets.length > 1so real single-widget checkboxes still usecheck()/uncheck()PDFOptionList(multiselect listbox) support —select()the comma-split values that exist ingetOptions()form.updateFieldAppearances()after the loop so widgets missing an on-state appearance stream get one (Preview/Acrobat otherwise show "unchanged")Viewer:
<select multiple>input listener now joins allselectedOptionsinstead oftarget.value(first only){value:false}per widget (was hitting pdf.js's inverted string-coercion)getAnnotatedPdfBytes: only sends fields whose value differs from baseline; cleared fields still get explicit""/falsesentinels.Tests
"1"lands on widget[1], not widget[0]