Skip to content

Split the src/display/display_utils.js file into multiple ones (PR 21860 follow-up) - #22101

Merged
Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:dom_utils
Oct 8, 2026
Merged

Snuffleupagus merged 1 commit into
mozilla:masterfrom
Snuffleupagus:dom_utils

Conversation

@Snuffleupagus

@Snuffleupagus Snuffleupagus commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

The built pdf.renderer.mjs file currently contains a lot of dead code, since it pulls in the entire src/display/display_utils.js file as-is and that one contains many functions that only make sense in the main-thread.
Hence we make the following changes:

  • Move a couple of helpers into the existing src/display/api_utils.js file.
  • Move all functions that access the DOM, or ones that are only invoked in the main-thread, into a new src/display/dom_utils.js file.
  • Keep functions that make sense in both the main- and worker-threads in the existing src/display/display_utils.js file.

Note: This reduces the size of the gulp mozcentral bundle by 20787 bytes, i.e. over 20 kilo-bytes, which is way too much to ignore.

@Snuffleupagus Snuffleupagus added core test release-blocker Blocker for the upcoming release labels Oct 7, 2026
@codecov-commenter

codecov-commenter commented Oct 7, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.77778% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.21%. Comparing base (5d923c9) to head (a4411d2).

Files with missing lines Patch % Lines
src/display/dom_utils.js 95.78% 7 Missing ⚠️
src/display/api_utils.js 53.84% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #22101      +/-   ##
==========================================
- Coverage   90.22%   90.21%   -0.01%     
==========================================
  Files         276      277       +1     
  Lines       67811    67811              
==========================================
- Hits        61182    61178       -4     
- Misses       6629     6633       +4     
Flag Coverage Δ
browsertest 66.19% <38.88%> (+0.01%) ⬆️
fonttest 8.98% <ø> (ø)
integrationtest 69.29% <70.55%> (-0.02%) ⬇️
unittest 60.00% <72.22%> (+0.02%) ⬆️
unittestcli 58.25% <54.44%> (ø)

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

☔ View full report in Codecov by Harness.
📢 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.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 7, 2026
@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Viewer preview

🗑️ Viewer previews removed.

@github-actions

github-actions Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Reference tests

Results for a4411d2 (run, references from mozilla/pdf.js.refs@05be816b55).

Platform Status Tests Total runtime Errors Different FBF No reference Report
Linux ✅ 1382 11m 27s 0 0 0 0
Windows ✅ 1382 16m 27s 0 0 0 0

The report is kept for 30 days and replaced by the next run.

Problems on Linux
TEST-UNEXPECTED-FAIL | test failed issue16114-partial | in firefox-1 | page1 round 1 | Optimized rendering differs from full rendering.
Problems on Windows
TEST-UNEXPECTED-FAIL | test failed issue16114-partial | in firefox-0 | page1 round 1 | Optimized rendering differs from full rendering.

sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 7, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 7, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 8, 2026
…21860 follow-up)

The *built* `pdf.renderer.mjs` file currently contains *a lot* of dead code, since it pulls in the entire `src/display/display_utils.js` file as-is and that one contains many functions that only make sense in the main-thread.
Hence we make the following changes:
 - Move a couple of helpers into the existing `src/display/api_utils.js` file.
 - Move all functions that access the DOM, or ones that are only invoked in the main-thread, into a new `src/display/dom_utils.js` file.
 - Keep functions that make sense in both the main- and worker-threads in the existing `src/display/display_utils.js` file.

*Note:* This reduces the size of the `gulp mozcentral` bundle by `20787` bytes, i.e. over 20 kilo-bytes, which is way too much to ignore.
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 8, 2026
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 8, 2026

@calixteman calixteman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you.

@Snuffleupagus
Snuffleupagus merged commit 3431b45 into mozilla:master Oct 8, 2026
26 checks passed
sync-pdfs-for-pdf-js Bot pushed a commit to mozilla/pdf.js.refs that referenced this pull request Oct 8, 2026
@Snuffleupagus
Snuffleupagus deleted the dom_utils branch October 8, 2026 21:03

This branch was successfully deployed

1 active deployment
sync_pdfs — a4411d2c Deployed Oct 8, 2026 by Snuffleupagus via request #322
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core release-blocker Blocker for the upcoming release test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants