Skip to content

Move editToolbar and fix overlapping annotations with it - #21723

Open
LoukasPap wants to merge 5 commits into
mozilla:masterfrom
LoukasPap:fix/link-overlaps-floating-toolbar
Open

Move editToolbar and fix overlapping annotations with it#21723
LoukasPap wants to merge 5 commits into
mozilla:masterfrom
LoukasPap:fix/link-overlaps-floating-toolbar

Conversation

@LoukasPap

@LoukasPap LoukasPap commented Aug 6, 2026

Copy link
Copy Markdown

Fixes #21643

I moved the editToolbar up to page level and added isolation: isolate to .annotationLayer, .annotationEditorLayer and .editor toolbar elements so that z-index works at their level. That way we avoid the z-index of a nested annotation (e.g. a linkAnnotation) "compete" with the z-index of the editToolbar.

This PR also fixes a bug where a textAnnotation was rendered above the editToolbar.

Tests

I used document.elementFromPoint(x,y) to check every time which elements is rendered on top.
I also moved createFreeTextEditor() to test_utils.mjs to make it available to all tests that may need it

Test 1 — "the edit toolbar is rendered above link annotations"

  1. Load the PDF at 120% zoom, wait for the text layer to be ready.
  2. Find the link pointing to https://github.com/mozilla/pdf.js and get its position.
  3. Check what's on top at that spot right now — confirm it's the link itself (pdfjs_internal_id_14R), i.e. nothing is covering it yet.
  4. Double-click on "A:" to select it, which makes the floating toolbar appear.
  5. Wait for the toolbar's highlight button to show up.
  6. Check what's on top at the link's position again — confirm it's now the highlight button, proving the toolbar renders above the link instead of being hidden behind it.

Test 2 — "the edit toolbar is rendered above text annotations"

  1. Switch into FreeText editing mode.
  2. Find "My text" on the page and note its position.
  3. Create a new text annotation containing "CHECK", placed just below "My text".
  4. Check what's on top at the middle of that new "CHECK" box — confirm it's the text annotation itself (its own edit toolbar/label), i.e. nothing else is covering it yet.
  5. Close the text editor (exit FreeText mode).
  6. Double-click on "My text" to select it, which makes the floating toolbar appear.
  7. Wait for the toolbar's highlight button to show up.
  8. Check what's on top at the toolbar's position — confirm it's the highlight button.
  9. Re-open the text editor (re-enter FreeText mode) without closing the floating toolbar, so both are visible on the page at the same time.
  10. Check what's on top at the freetext's position — confirm it's still the highlight button, proving the toolbar stays rendered above the reopened text annotation rather than getting covered by it.

@LoukasPap LoukasPap changed the title Fix link overlapping floating toolbar Move editToolbar and fix overlapping annotations with it Aug 6, 2026
@LoukasPap
LoukasPap marked this pull request as draft August 6, 2026 20:49
@LoukasPap

Copy link
Copy Markdown
Author

This is currently a draft because I haven't yet added the integration tests. I pushed anyway to make sure we agree with the solution and then move on.

@LoukasPap
LoukasPap force-pushed the fix/link-overlaps-floating-toolbar branch 2 times, most recently from 7c9c34c to c48cbd1 Compare August 6, 2026 20:58
@LoukasPap
LoukasPap force-pushed the fix/link-overlaps-floating-toolbar branch from 0d85a6a to f1d551a Compare August 19, 2026 15:49
@LoukasPap
LoukasPap force-pushed the fix/link-overlaps-floating-toolbar branch from f1d551a to 356901b Compare August 19, 2026 16:01
@LoukasPap
LoukasPap marked this pull request as ready for review August 19, 2026 16:03
@codecov-commenter

codecov-commenter commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 75.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 75.75%. Comparing base (e051279) to head (356901b).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
src/display/editor/tools.js 75.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           master   #21723       +/-   ##
===========================================
- Coverage   90.19%   75.75%   -14.44%     
===========================================
  Files         264      264               
  Lines       67324    67327        +3     
===========================================
- Hits        60721    51005     -9716     
- Misses       6603    16322     +9719     
Flag Coverage Δ
browsertest 66.33% <0.00%> (+0.01%) ⬆️
fonttest 8.91% <ø> (ø)
integrationtest 25.15% <75.00%> (-44.24%) ⬇️
unittest 58.35% <0.00%> (-0.01%) ⬇️
unittestcli 56.91% <0.00%> (-0.01%) ⬇️

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Overlapping link can be clicked through editor toolbar

3 participants