Skip to content

Don't needlessly wrap the marked content id-attribute in a string#21545

Merged
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:TextLayer-setAttribute-id-string
Jul 5, 2026
Merged

Don't needlessly wrap the marked content id-attribute in a string#21545
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:TextLayer-setAttribute-id-string

Conversation

@Snuffleupagus

Copy link
Copy Markdown
Collaborator

When the marked content id is defined it'll always be a string, hence wrapping it in a string when setting the attribute is pointless; note

pdf.js/src/core/evaluator.js

Lines 3529 to 3531 in e148b15

id: Number.isInteger(mcid)
? `${self.idFactory.getPageObjId()}_mc${mcid}`
: null,

When the marked content `id` is defined it'll always be a string, hence wrapping it in a string when setting the attribute is pointless; note https://github.com/mozilla/pdf.js/blob/e148b154cdff4344db1bc263fe3b546b03eff5ae/src/core/evaluator.js#L3529-L3531
@codecov-commenter

codecov-commenter commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.43%. Comparing base (e148b15) to head (cde4f9a).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #21545      +/-   ##
==========================================
+ Coverage   89.41%   89.43%   +0.01%     
==========================================
  Files         262      262              
  Lines       66739    66739              
==========================================
+ Hits        59677    59685       +8     
+ Misses       7062     7054       -8     
Flag Coverage Δ
browsertest 66.52% <100.00%> (-0.01%) ⬇️
fonttest 9.05% <ø> (ø)
integrationtest 68.74% <100.00%> (+0.01%) ⬆️
unittest 57.34% <0.00%> (+<0.01%) ⬆️
unittestcli 56.30% <0.00%> (ø)

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.

@timvandermeij timvandermeij merged commit d008aba into mozilla:master Jul 5, 2026
21 checks passed
@timvandermeij

Copy link
Copy Markdown
Collaborator

Thanks!

@Snuffleupagus Snuffleupagus deleted the TextLayer-setAttribute-id-string branch July 5, 2026 21:22
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.

3 participants