Ensure TextLayer hiddenCanvasElement is layout-neutral by default by RolandWArnold · Pull Request #21218 · mozilla/pdf.js

RolandWArnold · 2026-05-03T17:51:42Z

TextLayer creates a helper canvas with class hiddenCanvasElement and appends it directly to document.body, which can adversely affect the host-page layout/scrolling behavior.

The stock viewer stylesheet web/pdf_viewer.css already makes this element layout-neutral via .hiddenCanvasElement { ... display: none; }, but direct pdfjs-dist / TextLayer consumers are not required to load web/pdf_viewer.css. Consumers that scope the viewer stylesheet to avoid global CSS leakage also lose this protection, since the helper canvas is appended outside the scoped viewer subtree.

This change applies the essential layout-neutralizing rule at the creation site, so the display-layer helper does not depend on viewer CSS to avoid participating in host-page layout.

Related context:

cleanup hiddenCanvasElement when using renderTextLayer #18086 discussed the same body-appended helper element from a lifecycle/cleanup perspective.
react-pdf added the .hiddenCanvasElement CSS rule downstream in Hidden canvas element may break layout wojtekmaj/react-pdf#1815 to avoid layout side effects.

codecov-commenter · 2026-05-03T17:55:55Z

Codecov Report

❌ Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 55.92%. Comparing base (a55cec4) to head (ceb7875).
⚠️ Report is 12 commits behind head on master.

Files with missing lines	Patch %	Lines
web/pdf_viewer.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #21218      +/-   ##
==========================================
- Coverage   55.97%   55.92%   -0.06%     
==========================================
  Files         218      220       +2     
  Lines       59033    58991      -42     
==========================================
- Hits        33041    32988      -53     
- Misses      25992    26003      +11

Flag	Coverage Δ
fonttest	`8.65% <ø> (-0.02%)`	⬇️
unittest	`55.18% <50.00%> (-0.05%)`	⬇️
unittestcli	`55.66% <0.00%> (-0.07%)`	⬇️

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.

Snuffleupagus

Given that using the textLayer requires also using the pdf_viewer.css file, since otherwise things are simply not guaranteed to work, it's not clear (at least to me) why this effectively duplicated code should be added here!?

RolandWArnold · 2026-05-03T18:02:14Z

Thanks for the quick review.

I understand that TextLayer is expected to be used with web/pdf_viewer.css. The case I was trying to cover here is that this particular rule is unusual because the element it targets is not inside the viewer subtree: TextLayer creates the canvas and appends it directly to document.body.

Consumers building custom viewer chrome may include the viewer CSS but scope/rewrite its selectors to avoid leaking PDF.js viewer styles into the surrounding application. In that setup, the rest of the text-layer CSS is present, but .hiddenCanvasElement can be scoped along with everything else and no longer matches the body-appended helper canvas.

That's why I thought applying the essential layout-neutralizing property at the creation site might be appropriate: the body-appended helper would not depend on a viewer-subtree stylesheet rule to avoid participating in host-page layout. react-pdf also had to add the .hiddenCanvasElement rule downstream in wojtekmaj/react-pdf#1815.

If the project’s position is that TextLayer consumers must load web/pdf_viewer.css without selector scoping, and scoped or rewritten viewer CSS is unsupported, I’m happy to close this.

nicolo-ribaudo · 2026-05-03T18:22:48Z

      ctx = canvas.getContext("2d", {
        alpha: false,
        willReadFrequently: true,
      });


If we removed canvas from the document after getting the ctx instead, does the Context2D retain the locale inherited from the body? If it does, then it might be preferable to just fully remove the canvas from the DOM rather than "trying harder" to hide it.

Tested this - the CanvasRenderingContext2D does retain the relevant language state after detachment. With lang set on the canvas before getContext, measureText produces language-sensitive widths (for example, Korean differs from Japanese/Simplified Chinese for some CJK characters), and those widths are preserved after canvas.remove().

Detaching the canvas is the cleaner fix. I’ve updated this accordingly.

Tested this - the CanvasRenderingContext2D does retain the relevant language state after detachment. With lang set on the canvas before getContext, measureText produces language-sensitive widths (for example, Korean differs from Japanese/Simplified Chinese for some CJK characters), and those widths are preserved after canvas.remove().

Detaching the canvas is the cleaner fix. I’ve updated this accordingly.

Tested this - the CanvasRenderingContext2D does retain the relevant language state after detachment. With lang set on the canvas before getContext, measureText produces language-sensitive widths (for example, Korean differs from Japanese/Simplified Chinese for some CJK characters), and those widths are preserved after canvas.remove().

Detaching the canvas is the cleaner fix. I’ve updated this accordingly.

Do you have a link around the canvas specification explaining that we've the guarantee the lang setting will be constant over the time ?
If not, I don't think we'll take this patch as is.
But I'm not opposed to have something like:

canvas.style.cssText = "position:absolute;top:0;left:0;width:0;height:0;display:none";

@Snuffleupagus can you imagine any drawbacks ?

But I'm not opposed to have something like:

canvas.style.cssText = "position:absolute;top:0;left:0;width:0;height:0;display:none";

@Snuffleupagus can you imagine any drawbacks ?

Not technically. (Only maintainability wise, since it's generally better to not define styles inline. In this case though the element is explicitly invisible, so that's probably less of a concern here.)

However, if that change is done then the hiddenCopyElement in the viewer ought to be handled the same way such that the CSS rules can be removed from the web/pdf_viewer.css file.

Thanks both. I’ve updated the PR to:

apply inline layout-neutral styles to hiddenCanvasElement

apply the same treatment to hiddenCopyElement

remove the now-redundant shared rule from web/pdf_viewer.css

Snuffleupagus

Please also remember to write a good commit message, since a single line is rarely sufficient.

Snuffleupagus · 2026-05-05T18:14:06Z

@@ -473,6 +473,8 @@ class TextLayer {
      // OffscreenCanvas.
      const canvas = document.createElement("canvas");
      canvas.className = "hiddenCanvasElement";


Can't this line be removed now?

Removed - confirmed no other references.

Snuffleupagus · 2026-05-05T18:14:39Z

@@ -967,6 +967,8 @@ class PDFViewer {
          const element = (this.#hiddenCopyElement =
            document.createElement("div"));
          element.id = "hiddenCopyElement";


Can't this line be removed now?

Kept this one: #hiddenCopyElement is used by test/integration/copy_paste_spec.mjs as a loadAndWait selector and via document.getElementById in the selectAll helper, so removing the id would break those tests.

Set layout-neutral styles at the creation sites for the hidden TextLayer canvas and PDFViewer copy element rather than relying on the shared web/pdf_viewer.css rule. This keeps the helper elements invisible and out of layout when viewer CSS selectors are scoped or omitted, and removes the obsolete hiddenCanvasElement class and shared CSS rule.

RolandWArnold · 2026-05-05T18:36:04Z

Updated again:

removed the now-unused hiddenCanvasElement class
kept hiddenCopyElement’s id because test/integration/copy_paste_spec.mjs uses #hiddenCopyElement to wait for/query the copy helper element
expanded the commit message

timvandermeij added the text-selection label May 3, 2026

RolandWArnold temporarily deployed to code-coverage May 3, 2026 17:54 — with GitHub Actions Inactive

Snuffleupagus requested changes May 3, 2026

View reviewed changes

nicolo-ribaudo reviewed May 3, 2026

View reviewed changes

RolandWArnold force-pushed the fix/text-layer-hidden-canvas-layout-neutral branch 3 times, most recently from fcfd0f3 to 3b05d2e Compare May 5, 2026 14:22

Snuffleupagus reviewed May 5, 2026

View reviewed changes

RolandWArnold force-pushed the fix/text-layer-hidden-canvas-layout-neutral branch from 3b05d2e to ceb7875 Compare May 5, 2026 18:34

RolandWArnold temporarily deployed to code-coverage May 5, 2026 20:59 — with GitHub Actions Inactive

RolandWArnold deployed to code-coverage May 5, 2026 20:59 — with GitHub Actions Active

RolandWArnold temporarily deployed to code-coverage May 5, 2026 20:59 — with GitHub Actions Inactive

Conversation

RolandWArnold commented May 3, 2026

Uh oh!

codecov-commenter commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Snuffleupagus left a comment

Choose a reason for hiding this comment

Uh oh!

RolandWArnold commented May 3, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RolandWArnold May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Snuffleupagus left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RolandWArnold commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov-commenter commented May 3, 2026 •

edited

Loading

RolandWArnold May 3, 2026 •

edited

Loading