Prevent inherited spacing from affecting text layer#21321
Conversation
timvandermeij
left a comment
There was a problem hiding this comment.
Looks good to me, with the comment addressed and passing tests. Thank you for your contribution!
|
Could you squash the commits into one, see https://github.com/mozilla/pdf.js/wiki/Squashing-Commits, and include (a summary of) the context from the PR description in the actual commit message so that it's also clear from e.g. |
|
Sure , give me a moment |
d6c4c48 to
fbe9a52
Compare
|
@timvandermeij Squashed into one commit with the PR context included in the commit message. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #21321 +/- ##
==========================================
+ Coverage 81.39% 81.41% +0.01%
==========================================
Files 256 257 +1
Lines 65337 65339 +2
==========================================
+ Hits 53183 53196 +13
+ Misses 12154 12143 -11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/0e9ab44fdd528b3/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/0e9ab44fdd528b3/output.txt Total script time: 1.20 mins Published |
Fixes mozilla#21259. Reset letter-spacing and word-spacing on the text layer and hidden measurement canvas so inherited page styles do not affect text layer alignment. Add an integration regression test for inherited spacing.
fbe9a52 to
4ee9c39
Compare
|
/botio browsertest |
From: Bot.io (Linux m4)ReceivedCommand cmd_browsertest from @timvandermeij received. Current queue size: 0 Live output at: http://54.241.84.105:8877/0089e6b5470a51d/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_browsertest from @timvandermeij received. Current queue size: 0 Live output at: http://54.193.163.58:8877/5a0bdd56e8589f3/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/0089e6b5470a51d/output.txt Total script time: 18.28 mins
|
From: Bot.io (Windows)SuccessFull output at http://54.193.163.58:8877/5a0bdd56e8589f3/output.txt Total script time: 25.32 mins
|
Summary
Fixes #21259.
This prevents inherited
letter-spacingandword-spacingfrom affecting the text layer. External page styles can otherwise change the rendered width/positioning of text-layer spans while the PDF canvas remains unchanged, causing text selection/highlight alignment issues.The fix resets spacing on:
.textLayerA regression test was added to cover inherited spacing from the viewer container.
Testing
npx eslint src/display/text_layer.js test/integration/text_layer_spec.mjsnpx stylelint web/text_layer_builder.cssnpx gulp genericletter-spacing: 5pxandword-spacing: 5pxtobody/#mainContainerbodyhad inherited spacing.textLayerand text-layer spans stayed at neutral computed spacingNote: I also tried
npx gulp integrationtest --headless --noFirefox -t text_layer, but it did not stay limited to the text-layer test and started running unrelated integration tests, so I stopped it locally.