Skip to content

Add a pref to postMessage when mozPrintCallback has completed (bug 2036265)#21308

Merged
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:bug2036265
May 29, 2026
Merged

Add a pref to postMessage when mozPrintCallback has completed (bug 2036265)#21308
calixteman merged 1 commit into
mozilla:masterfrom
calixteman:bug2036265

Conversation

@calixteman
Copy link
Copy Markdown
Contributor

Summary

  • Adds a new postMessageAfterPrintCallback browser option (default false).
  • When enabled by Firefox via the pdfjs.postMessageAfterPrintCallback pref, web/firefox_print_service.js posts "ready" or "error" to the window after each page's mozPrintCallback resolves.
  • Allows embedders (e.g. print-preview test harnesses) to observe when print rendering for a page has actually finished, instead of relying on STATE_STOP.

This upstreams the viewer-side portion of Phabricator D297837 (original patch by @dholbert). The Firefox-side pref wiring (PdfJsDefaultPrefs.js, PdfStreamConverter.sys.mjs) lives in mozilla-central; only the PDF.js bits land here so they aren't overwritten at the next pdf.js import.

See bug 2036265 for context.

Test plan

  • npx gulp lint passes.
  • With pdfjs.postMessageAfterPrintCallback flipped on in Firefox, printing a PDF results in one window.postMessage("ready", "*") per page on success and "error" on failure.
  • Default behavior (pref off) is unchanged.

🤖 Generated with Claude Code

@calixteman
Copy link
Copy Markdown
Contributor Author

@dholbert, is there anything you want to add ?

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented May 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.16%. Comparing base (5a4d93a) to head (567f585).
⚠️ Report is 48 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21308      +/-   ##
==========================================
- Coverage   81.33%   80.16%   -1.18%     
==========================================
  Files         256      257       +1     
  Lines       65138    66843    +1705     
==========================================
+ Hits        52979    53582     +603     
- Misses      12159    13261    +1102     
Flag Coverage Δ
integrationtest 67.00% <ø> (+0.10%) ⬆️
unittest 56.87% <ø> (+0.02%) ⬆️
unittestcli 56.07% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread web/app_options.js Outdated
Comment thread web/firefox_print_service.js Outdated
@dholbert
Copy link
Copy Markdown
Contributor

dholbert commented May 22, 2026

Thank you @calixteman! Sorry, I'd been meaning to turn this into a PR - I appreciate you doing it for me.

@dholbert, is there anything you want to add ?

No, I think I'm good with this as-is without adding other things (aside from addressing review comments).

(I had started exploring formalizing this as a per-page postMessage callback, and making sure that worked properly as a way to test print-preview of multi-page PDFs - that's what I was sort of trying to sort out before posting a PR. But I didn't yet get that working end-to-end with the tests that I was writing for print-preview of multi-page PDFs -- and in any case that's not something I was as concerned with since the Firefox print-preview tests are largely single-page anyway, for better or for worse. We can always add those tests later on, in any case.)

@Snuffleupagus's review feedback looks valid (likely just accounting for me not knowing what I'm doing in this code). I can look at addressing that and post a diff (not sure if I can push updates to the PR directly since it's owned by @calixteman)

@dholbert
Copy link
Copy Markdown
Contributor

@calixteman here's a patch (which should apply cleanly on top of this PR) that I think addresses @Snuffleupagus's review feedback - would you mind updating the PR with this?

0001-address-review-feedback.patch

I tested the same changes in my firefox clone (with the PdfStreamConverter.sys.mjs change reverted as well, per @Snuffleupagus's suggestion) and I confirmed that my test still runs as-expected -- i.e. the postMessage is getting sent as I expect it to, if-and-only-if this testing pref is set.

…k has completed (bug 2036265)

Adds a `postMessageAfterPrintCallback` browser option (off by default).
When enabled by Firefox, the print service posts "ready" or "error" to
the window after each page's mozPrintCallback resolves, so embedders
(e.g. print-preview test harnesses) can observe when rendering is done.

Upstreams the viewer-side portion of Phabricator D297837.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Daniel Holbert <dholbert@cs.stanford.edu>
@calixteman
Copy link
Copy Markdown
Contributor Author

@calixteman here's a patch (which should apply cleanly on top of this PR) that I think addresses @Snuffleupagus's review feedback - would you mind updating the PR with this?

0001-address-review-feedback.patch

I don't really know if we can commandeer a PR as we do on Phab... so I added you as a co-author.

@Snuffleupagus
Copy link
Copy Markdown
Collaborator

Should this also be done when printing XFA documents?
If so, the following branch needs to be updated as well:

if (pdfDocument.isPureXfa) {
getXfaHtmlForPrinting(printContainer, pdfDocument);
return;
}

@calixteman
Copy link
Copy Markdown
Contributor Author

Should this also be done when printing XFA documents? If so, the following branch needs to be updated as well:

if (pdfDocument.isPureXfa) {
getXfaHtmlForPrinting(printContainer, pdfDocument);
return;
}

I don't know if we want to text printing XFA so it's probably useless but it probably doesn't hurt to have the "feature" just in case it's useful.

@dholbert
Copy link
Copy Markdown
Contributor

Yeah, until/unless we have print-preview tests for XFA in Firefox (we don't currently), we could go either way on that branch; doesn't matter much to me.

@dholbert
Copy link
Copy Markdown
Contributor

@calixteman / @Snuffleupagus is this good to be merged? Let me know if there's anything else I can do to help here. (I'm looking forward to being able to land a patch that adds some tests in https://bugzilla.mozilla.org/show_bug.cgi?id=2022056 , once this PR is merged and then synced into Firefox.)

@calixteman calixteman merged commit 80c8e62 into mozilla:master May 29, 2026
18 checks passed
@calixteman calixteman deleted the bug2036265 branch May 29, 2026 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants