Fix flickering on text selection

[nicolo-ribaudo]

When seleciting on a touch screen device, whenever the finger moves to a blank area (so over div.textLayer directly rather than on a <span>), the selection jumps to include all the text between the beginning of the .textLayer and the selection side that is not being moved.

The existing selection flickering fix when using the mouse cannot be trivially re-used on mobile, because when modifying a selection on a touchscreen device Firefox will not emit any pointer event (and Chrome will emit them inconsistently). Instead, we have to listen to the 'selectionchange' event.

The fix is different in Firefox and Chrome:

• on Firefox, we have to make sure that, when modifying the selection, hovering on blank areas will hover on the .endOfContent element rather than on the .textLayer element. This is done by adjusting the z-indexes so that .endOfContent is above .textLayer.
• on Chrome, hovering on blank areas needs to trigger hovering on an element that is either immediately after (or immediately before, depending on which side of the selection the user is moving) the currently selected text. This is done by moving the .endOfContent element around between the correct <span>s in the text layer.

In Safari mobile it is still broken in random and fun ways, the fix for Safari would be completely different, and out-of-scope for this PR.

Before/after videos (mobile)

	Before	After
Firefox	pdf1_firefox_before.mp4	pdf1_firefox_after.mp4
Chrome	pdf1_chrome_before.mp4	pdf1_chrome_after.mp4

Before/after videos (desktop)

	Before	After
Chrome	pdfjs-recording-before.mp4	pdfjs-recording-after.mp4
Firefox (multi-page selection)	2024-04-17.14-08-35.mp4	2024-04-17.13-42-39.mp4

Fixes #17912

I'm not sure about how to add a test for this, if it's even possible.

[timvandermeij]

/botio-linux preview

[timvandermeij]

I'm not sure if it's possible to add an integration test for this kind of change because even though we can probably do the selection with Puppeteer I don't really see what we could assert in the test. In any case, let's first check if all existing tests still pass with this change applied:

/botio test

[timvandermeij]

It looks like the integration tests did notice the change, so we fortunately at least have some coverage on this (although we don't specifically test this selection issue, but it's most likely noticed as a side-effect of other selection/scrolling tests). However, sadly they seem to fail on both Linux and Windows.

Tip: what usually helps for quick debugging is enabling one of failing tests from the logs by changing it to fit in the test function definition (which tells the Jasmine test runner to only run that particular test instead of the entire suite which takes a much longer time) and then running npx gulp integrationtest to find out what's breaking or changed compared to the master branch.

[nicolo-ribaudo]

@timvandermeij Thanks for the tip! The flickering behavior is very consistent/predictable, so if with puppeteer I could simulate "long-tap at (x1,y2) to select, then hold the selection bubble and drag to (x2,y2), and check what is selected" I would be able to add a test.

The problem is the "drag the selection bubble" part, since it's not dragging on the page (the selection bubble is browser UI and doesn't trigger touch events)

[nicolo-ribaudo]

I pushed two commits:

• the first one unregisters the selectionchange event listeners in .cancel(), so that it doesn't leak (fixing Fix flickering on text selection #17923 (comment))
• the second one fixes flickering also in the case of <span>s nested inside .markedContent, and removes the old anti-flickering code for Chrome when using a mouse (I added a before/after video in the issue description), as I noticed that the mobile fix also improves the selection on desktop. This de-duplicates the isFirefox ??= ... check, solving Fix flickering on text selection #17923 (comment).

I still need to investigate testing.

[timvandermeij]

The problem is the "drag the selection bubble" part, since it's not dragging on the page (the selection bubble is browser UI and doesn't trigger touch events)

I have looked that the Puppeteer API documentation, but I can't really find anything related to this. However, if the flickering effects are also consistently visible in Firefox or Chrome on desktop then we might be able to write a test that starts a selection, moves the mouse to certain coordinates and gets the selected text. We'd have to try it out to see if it actually works, but it sounds like it might be doable.

(By the way, thank you for providing such detailed issue/PR descriptions, visuals and inline comments! That really helps to understand the change, especially for these kinds of bugs that are usually a bit tricky and difficult to fully test in an automated way, and therefore require additional manual testing.)

[nicolo-ribaudo]

@timvandermeij I managed to add a test when using a mouse (which before this PR fails in Chrome), but unfortunately puppeteer's API doesn't seem to allow selecting text emulating a touchscreen input.

In Chrome this test will trigger the selectionchange event, so it covers some of the code paths of Firefox mobile too (except that obviously there is some Chrome-specific additional code).

[calixteman]

In Firefox (desktop), it's possible to reproduce the flickering in setting the pref layout.accessiblecaret.hide_carets_for_mouse_input to false and in using the blue anchors to modify the selection.

[nicolo-ribaudo]

Oh thank you :) Can I somehow set it in Puppeteer?

[timvandermeij]

Yes, Firefox-specific preferences are configured here: https://github.com/mozilla/pdf.js/blob/master/test/test.mjs#L907-L942. If it helps, I also found some more documentation about this preference at https://firefox-source-docs.mozilla.org/layout/AccessibleCaret.html.

[nicolo-ribaudo]

I pushed 7bb30a4 (#17923) to do what suggested by @calixteman in #17923 (comment). Commit description:

Handle selectionchange events globally

This commit moves the selectionchange and pointerup event
listeners to be shared across multiple TextLayerBuilder
instances. This has two effects:

• we only need to register one global listener that then "dispatches"
  the DOM updates in the correct .textLayer, rather than having one
  per instance that needs to be careful about only handling selection
  changes that affect it.
• the fix now works with multi-page selections, which were currently
  also broken on Firefox desktop

I am working on a test for this multi-page flickering on Firefox desktop, then I'll try if layout.accessiblecaret.hide_carets_for_mouse_input lets me test the flickering behavior when using the carets. I also still need to go through the snapshot failures.

[nicolo-ribaudo]

All tests are passing locally, except for Chrome reftests because one of them kept timing out while I was trying to generate the reference images from master.

I had to add one more commit to fix a regression: de1e73b (#17923)

Explicitly add z-index: 0 to .textLayer

This creates a "stacking context", making sure that the elements
inside .textLayer (on the z axis) remain inside .textLayer.
Without this change, the <span>s inside .textLayer end up being
on top of .annotationLayer.

This was caught by the freetext editor integration tests, that
failed because we were trying to draw on top of some text.

This is ready for re-review -- I will squash once it's ready :)

[nicolo-ribaudo]

Rebased!

[timvandermeij]

/botio-windows integrationtest

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

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

Live output at: http://54.193.163.58:8877/edfbf086f943cfc/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/edfbf086f943cfc/output.txt

Total script time: 1.73 mins

[timvandermeij]

/botio-windows integrationtest

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

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

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

[timvandermeij]

/botio-windows integrationtest

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 1

Live output at: http://54.193.163.58:8877/60e691075d73896/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Failed

Full output at http://54.193.163.58:8877/60e691075d73896/output.txt

Total script time: 21.60 mins

• Integration Tests: FAILED

[timvandermeij]

@nicolo-ribaudo We're down to just one failure on Windows now, so we have improved since the last fixup. Looking at the logs above, could this be a problem with the "roughly equals" function given that the assertions look very close at least on a quick glance? Fortunately you've logged the actual/expected values in the message, so hopefully this can be checked in isolation more easily now.

[nicolo-ribaudo]

@timvandermeij I relaxed the assertion a bit, hopefully it will pass now. The "roughly equals" function checks that the expected value is an almost-complete substring of the actual value. It cannot get a substring of the expected value because it also supports regexp, and slicing a regexp is... complex.

[timvandermeij]

/botio integrationtest

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 1

Live output at: http://54.241.84.105:8877/078a82ccad8684f/output.txt

[moz-tools-bot]

From: Bot.io (Windows)

---

Received

Command cmd_integrationtest from @timvandermeij received. Current queue size: 2

Live output at: http://54.193.163.58:8877/0002a3c99d2cf86/output.txt

[moz-tools-bot]

From: Bot.io (Linux m4)

---

Success

Full output at http://54.241.84.105:8877/078a82ccad8684f/output.txt

Total script time: 7.32 mins

• Integration Tests: Passed

[nicolo-ribaudo]

Windows is still running, but so far the selection tests passed :)

�[32m.�[0mTEST-PASSED | doesn't jump when hovering on an empty area
�[32m.�[0mTEST-PASSED | doesn't jump when hovering on an empty area (multi-page)
�[32m.�[0mTEST-PASSED | doesn't jump when moving selection

[moz-tools-bot]

From: Bot.io (Windows)

---

Success

Full output at http://54.193.163.58:8877/0002a3c99d2cf86/output.txt

Total script time: 20.79 mins

• Integration Tests: Passed

[timvandermeij]

Nice work; thank you for the effort you put into this!