Skip to content

Move text layer scaling logic to CSS#20491

Merged
timvandermeij merged 1 commit intomozilla:masterfrom
nicolo-ribaudo:move-text-scaling-logic-to-css
Dec 11, 2025
Merged

Move text layer scaling logic to CSS#20491
timvandermeij merged 1 commit intomozilla:masterfrom
nicolo-ribaudo:move-text-scaling-logic-to-css

Conversation

@nicolo-ribaudo
Copy link
Contributor

@nicolo-ribaudo nicolo-ribaudo commented Dec 8, 2025

Commit message:

Move text layer scaling logic to CSS

This commit moves all the logic to scale up&down <span>s in the text layer, introduced in #18283, to CSS.

The motivation for this change is that #18283 is still not enough for all cases. That PR fixed the problem in Chrome&Firefox desktop, which allow users to set an actual minimum font size in the browser settings. However, other browsers (e.g. the Chrome-based WebView on Android) have more complex logic and they scale up small text rather than simply applying a minimum.

A workaround for that behavior is probably out of scope for PDF.js itself as it only affects not officially supported platforms. However, having access to the actual expected font height (through --font-height) allows embedders of PDF.js to implement a workaround by themselves.

In my app that uses PDF.js I'm currently patching the PDF.js code to export the text height as textDiv.dataset.fontHeight = fontHeight;. I find the approach proposed in this PR to be cleaner, as it has the benefit of moving style-related logic from the JS file to the CSS file.

What do you think about this change?

@nicolo-ribaudo nicolo-ribaudo force-pushed the move-text-scaling-logic-to-css branch 3 times, most recently from 2763604 to 9be3ac8 Compare December 8, 2025 15:28
@nicolo-ribaudo
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @nicolo-ribaudo received. Current queue size: 1

Live output at: http://54.241.84.105:8877/cf5aa7cfca2bd1b/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @nicolo-ribaudo received. Current queue size: 1

Live output at: http://54.193.163.58:8877/3c1cef802ac5954/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/cf5aa7cfca2bd1b/output.txt

Total script time: 40.17 mins

  • Unit tests: FAILED
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 58

Image differences available at: http://54.241.84.105:8877/cf5aa7cfca2bd1b/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/3c1cef802ac5954/output.txt

Total script time: 75.12 mins

  • Unit tests: FAILED
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 59

Image differences available at: http://54.193.163.58:8877/3c1cef802ac5954/reftest-analyzer.html#web=eq.log

@nicolo-ribaudo nicolo-ribaudo force-pushed the move-text-scaling-logic-to-css branch from 9be3ac8 to af6868b Compare December 9, 2025 10:35
This commit moves all the logic to scale up&down `<span>`s in the text
layer, introduced in mozilla#18283, to CSS.

The motivation for this change is that mozilla#18283 is still not enough for
all cases. That PR fixed the problem in Chrome&Firefox desktop, which
allow users to set an actual minimum font size in the browser settings.
However, other browsers (e.g. the Chrome-based WebView on Android) have
more complex logic and they scale up small text rather than simply
applying a minimum.

A workaround for that behavior is probably out of scope for PDF.js
itself as it only affects not officially supported platforms. However,
having access to the actual expected font height (through
`--font-height`) allows embedders of PDF.js to implement a workaround by
themselves.
@nicolo-ribaudo nicolo-ribaudo force-pushed the move-text-scaling-logic-to-css branch from af6868b to eb2b7c2 Compare December 9, 2025 12:42
@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Dec 9, 2025

I manually checked the reftest differences. They are all due to fraction of pixel differences in how the text layer <span>s are rendered, and they do not affect the text selection experience in any way.

There is also a difference in rounding, that again is effectively unobservable: master does (round(minFontHeight * fontHeight, 0.01px)), while this PR does minFontHeight * round(fontHeight, 0.01px) (because the multiplication is moved to the CSS side).

What's interesting, is that although I can see the difference between master and this PR locally (by showing the text later and having two tabs, with the same zoom level and sceoll location, and switching between them so that everything should be in the exact same place), the difference is only visual: their computed location and size, font size and transform are exactly the same.

The rendering difference is caused by having vs not having transform: matrix(1, 0, 0, 1, 0, 0) on some elements, which should be a no-op but it's not and it causes Firefox to draw the borders that happen at a fraction of a pixel in a different place.

@nicolo-ribaudo
Copy link
Contributor Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_test from @nicolo-ribaudo received. Current queue size: 0

Live output at: http://54.193.163.58:8877/1d55449e9756ece/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_test from @nicolo-ribaudo received. Current queue size: 0

Live output at: http://54.241.84.105:8877/4013f1d58194339/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/4013f1d58194339/output.txt

Total script time: 39.94 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  different ref/snapshot: 58

Image differences available at: http://54.241.84.105:8877/4013f1d58194339/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/1d55449e9756ece/output.txt

Total script time: 74.57 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: FAILED
  different ref/snapshot: 59

Image differences available at: http://54.193.163.58:8877/1d55449e9756ece/reftest-analyzer.html#web=eq.log

Copy link
Contributor

@calixteman calixteman left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you.

@timvandermeij timvandermeij merged commit 4af193b into mozilla:master Dec 11, 2025
9 checks passed
@timvandermeij
Copy link
Contributor

Thank you for improving this!

@timvandermeij
Copy link
Contributor

/botio makeref

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/c849109f85b68fa/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

Command cmd_makeref from @timvandermeij received. Current queue size: 0

Live output at: http://54.193.163.58:8877/21dfe8066e0b4fe/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Success

Full output at http://54.241.84.105:8877/c849109f85b68fa/output.txt

Total script time: 17.64 mins

  • Make references: Passed
  • Check references: Passed

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Success

Full output at http://54.193.163.58:8877/21dfe8066e0b4fe/output.txt

Total script time: 30.30 mins

  • Make references: Passed
  • Check references: Passed

@nicolo-ribaudo nicolo-ribaudo deleted the move-text-scaling-logic-to-css branch December 12, 2025 10:36
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.

4 participants