Repository navigation
fix(forms): ignore print requests from automatic field scripts - #4166
enriquemelzer-bit wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Filter automatic-script print requests and verify behavior crates/engine/src/js.rs, crates/engine/src/tests.rs, crates/automation/src/tests.rs |
JsOutput::absorb drops and logs Print requests from automatic field scripts. Tests cover all four automatic field events, retained field values and other script output, error reporting, calculation-triggered requests, and explicit button and console printing. |
Document and record printing behavior crates/engine/README.md, parity/acrobat-features.toml, ROADMAP.md, docs/roadmap.md |
The README and parity record describe the request handling and explicit printing. Roadmap entries record the M6 integration and additional milestone updates. |
Contributor metadata
| Layer / File(s) | Summary |
|---|---|
Add contributor synchronization entry contributors/people.toml |
Adds the enriquemelzer-bit contributor entry with GitHub-name synchronization enabled. |
Estimated code review effort: 2 (Simple) | ~15 minutes
Change: Bug fix · Severity of issue fixed: Medium
Suggested reviewers: dylanmtaylor, aiongg
Merge Risk: 
add4d
-
Automatic field-script printing is suppressed while explicit printing remains available. No merge-blocking risk is identified.
Architecture risk: 
add4d
-
The change affects 5 systems.
Changed systems:
crates,contributors,docs,parity,ROADMAP.mdArchitecture concerns
No architecture-level concerns identified.Review details
-
Systems and components
- observed — crates (service) was modified; 4 changed files map to changed impact.
- observed — contributors (service) was modified; 1 changed file maps to changed impact.
- observed — docs (service) was modified; 1 changed file maps to changed impact.
- observed — parity (service) was modified; 1 changed file maps to changed impact.
Before / after behavior
- observed — Modified behavior in crates/engine/README.md: Added documentation of automatic AcroForm script execution and request handling: Print, ShowPanel, and SaveAs requests are ignored with a console diagnostic; field values, alerts, and other requests are retained; explicit button and console scripts can request printing.
- observed — Modified behavior in crates/automation/src/tests.rs: Added a test exercising print requests from form-validation scripts versus explicit
js_run: form filling is expected to preserve the field value and console output while yielding no print request or errors, whereas explicit script execution is expected to return a print request. - observed — Modified behavior in crates/engine/src/js.rs: The
absorbdocumentation now includes print requests among the requests noted on the console and dropped. - observed — Modified behavior in crates/engine/src/js.rs: The comment now says only explicit button or console scripts may print, show, or hide a side panel, or open Save As. Automatic field-script
Request::Printrequests are now dropped with a console message directing users to a button or the JavaScript console; previously they were passed through torequests.
-
Pre-merge checks: 
-
Check name Status Explanation Resolution Out of Scope Changes check The supplied whole-PR summary identifies a new [people.enriquemelzer-bit]entry withsync_github_name = trueincontributors/people.toml. Contributor credit does not implement or support the Pri…Remove the [people.enriquemelzer-bit]contributor entry and itssync_github_name = truesetting from this pull request. Move the contributor credit to a separate maintenance pull request.-
Check name Status Explanation Title check The title clearly and concisely identifies the main change: ignoring print requests from automatic field scripts. Description check The description covers the change, verification results, regression tests, never-crash, provenance, security, parity, reach, and reviewer considerations. It is complete and directly related to the pul… Linked Issues check Issue #4088requires suppression of Print requests from automatic AcroForm Keystroke, Format, Validate, and Calculate scripts, with a JavaScript console diagnostic. It requires Print to remain availab…Full details: Description check
-
Explanation
The description covers the change, verification results, regression tests, never-crash, provenance, security, parity, reach, and reviewer considerations. It is complete and directly related to the pull request.
Full details: Linked Issues check
-
Explanation
Issue
#4088requires suppression of Print requests from automatic AcroForm Keystroke, Format, Validate, and Calculate scripts, with a JavaScript console diagnostic. It requires Print to remain available to button Mouse Up scripts. TheJsOutput::absorbdiff adds aRequest::Printfilter and the required diagnostic in the automatic field-script path. The supplied change summary reports engine and automation tests for all four events, preserved values and errors, and explicit button and console printing. The implementation meets the coding requirements in#4088.
-
Full details: Out of Scope Changes check
-
Explanation
The supplied whole-PR summary identifies a new
[people.enriquemelzer-bit]entry withsync_github_name = trueincontributors/people.toml. Contributor credit does not implement or support the Print suppression required by#4088. The engine change, tests, parity entry, and documentation are connected to the issue.Resolution
Remove the
[people.enriquemelzer-bit]contributor entry and itssync_github_name = truesetting from this pull request. Move the contributor credit to a separate maintenance pull request.
-
Finishing Touches
-
Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
ahmadmuj
left a comment
ahmadmuj
left a comment
There was a problem hiding this comment.
Checked f948111 with engine, automation and headless UI tests. Automatic K/V/C/F printing stays suppressed through repeated edits and save/reopen, with values and errors retained.
Two existing UI limits affect the explicit-printing claim:
- A button running
getField('greeting').value=6; this.print();opens Print but skips the assignment.button_scriptclassifies it asNamed(Print); the bracket-call fixturethis['print']()runs the assignment and opens Print. - Console Run dispatches Print, but dialog rendering then restores
JsConsole, so the print dialog does not appear.
Both paths are unchanged from the base; these are not regressions from this filter. Could the description qualify the UI coverage? Native printer transport remains untested.
f948111 to
cadf570
Compare
cadf570 to
138d5fa
Compare
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai resume |
|
@coderabbitai review |
Action performed
|
Action performed
|
What this changes
Fixes #4088. Automatic AcroForm Keystroke, Validate, Calculate and Format scripts can no longer open printing by calling
this.print(). The collector ignores those requests and records a JavaScript console diagnostic. Field values, structured alerts, other requests and errors remain intact. Explicit button and console scripts can still request printing, including when they trigger automatic calculations.Integrated with main
f137a289baadefbd29e0e46930899896068cc41d(PdfCraft 0.16.1). Main's ShowPanel/SaveAs filtering, document-error provenance, structured message metadata and module boundaries are preserved. Cargo manifests and Cargo.lock match incoming main exactly, including craft-libsc5a1cbfb5f9049f9270955caffb5bee5997aebf7. This retains the merged craft-libs #366 correction; the temporary dependency adjustment is no longer needed. CI workflows are unchanged relative to main.How you verified it
Current integration validation — 2026-10-31, Windows x64 / Rust 1.95:
-D warningspassed.cargo xtask layerspassed for 37 crates; assets and parity checks passed. Parity retains main's 100 existing headless notices.wasm32-unknown-unknowncheck passed. Five existing renderer platform warnings and three optional egui patch notices remain.verify4166-main-20261031.pypassed all 13 synthetic steps: Qty=4 recalculates Total=8, explicit button/console scripts retain Print requests, and save/render succeeds. The generatedfilled.pngwas inspected and shows the values 4 and 8. This is CLI render evidence, not a native printer or UI/GPU-suite qualification.The three engine regressions and
form_fill_cannot_print_but_js_run_cancover all four automatic events, requests emitted before an error, document-error provenance and explicit scripts that trigger calculations. The Rust regression checks the automatic request queue directly, whichform_fillJSON does not expose. Static integration review found no lost feature changes.The current runs used the exact CI font input
b076417b5880bd4154bd22f00695b1d28c6bea83, with system fonts disabled, two build jobs, debug symbols omitted and incremental compilation disabled.Historical evidence, 2026-10-29 only: the previous 0.15 integration recorded 727 engine/automation tests passed, zero failed and four ignored, the scoped verification gates, and a 13-step synthetic CLI exercise with an inspected render. The original regressions had been observed failing with leaked Print requests before the fix. Those results do not validate this 0.16.1 integration.
Never-crash, provenance and security
No new production panic paths, unsafe code, assets, fonts, filesystem/network access or control-channel permissions. Existing JavaScript request limits bound the added diagnostics.
Parity and reach
The existing
form.js-securityparity entry cites the three engine tests and the automation regression.form_fillandjs_runexercise the boundary headlessly. Engine guidance and roadmaps describe the behavior. The contributor's public GitHub-name opt-in was explicitly authorized; only their own entry is added, preserving all incoming credits.Anything a reviewer should look at first
The production change is the additional Print arm in
JsOutput::absorb. Previously documented UI limitations remain outside this fix: literal button-scriptthis.print()classification can skip preceding assignments, and console dialog restoration can hide the Print dialog. Native printer transport is untested.Summary by CodeRabbit