Merge pull request #17923 from nicolo-ribaudo/fix-text-selection

Fix flickering on text selection

test/integration-boot.mjs

@@ -35,6 +35,7 @@ async function runTests(results) {
       "scripting_spec.mjs",
       "stamp_editor_spec.mjs",
       "text_field_spec.mjs",
+      "text_layer_spec.mjs",
       "viewer_spec.mjs",
     ],
   });

test/integration/highlight_editor_spec.mjs

@@ -20,6 +20,7 @@ import {
   getEditorSelector,
   getFirstSerialized,
   getSerialized,
+  getSpanRectFromText,
   kbBigMoveLeft,
   kbBigMoveUp,
   kbFocusNext,
@@ -49,27 +50,6 @@ const getXY = (page, selector) =>
     return `${bbox.x}::${bbox.y}`;
   }, selector);
 
-const getSpanRectFromText = async (page, pageNumber, text) => {
-  await page.waitForSelector(
-    `.page[data-page-number="${pageNumber}"] > .textLayer .endOfContent`
-  );
-  return page.evaluate(
-    (number, content) => {
-      for (const el of document.querySelectorAll(
-        `.page[data-page-number="${number}"] > .textLayer > span`
-      )) {
-        if (el.textContent === content) {
-          const { x, y, width, height } = el.getBoundingClientRect();
-          return { x, y, width, height };
-        }
-      }
-      return null;
-    },
-    pageNumber,
-    text
-  );
-};
-
 describe("Highlight Editor", () => {
   describe("Editor must be removed without exception", () => {
     let pages;

test/integration/test_utils.mjs

@@ -148,6 +148,27 @@ function getSelectedEditors(page) {
   });
 }
 
+async function getSpanRectFromText(page, pageNumber, text) {
+  await page.waitForSelector(
+    `.page[data-page-number="${pageNumber}"] > .textLayer .endOfContent`
+  );
+  return page.evaluate(
+    (number, content) => {
+      for (const el of document.querySelectorAll(
+        `.page[data-page-number="${number}"] > .textLayer > span`
+      )) {
+        if (el.textContent === content) {
+          const { x, y, width, height } = el.getBoundingClientRect();
+          return { x, y, width, height };
+        }
+      }
+      return null;
+    },
+    pageNumber,
+    text
+  );
+}
+
 async function waitForEvent(page, eventName, timeout = 5000) {
   const handle = await page.evaluateHandle(
     (name, timeOut) => {
@@ -571,6 +592,7 @@ export {
   getSelectedEditors,
   getSelector,
   getSerialized,
+  getSpanRectFromText,
   hover,
   kbBigMoveDown,
   kbBigMoveLeft,

test/integration/text_layer_spec.mjs

@@ -0,0 +1,293 @@
+/* Copyright 2024 Mozilla Foundation
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { closePages, getSpanRectFromText, loadAndWait } from "./test_utils.mjs";
+import { startBrowser } from "../test.mjs";
+
+describe("Text layer", () => {
+  describe("Text selection", () => {
+    // page.mouse.move(x, y, { steps: ... }) doesn't work in Firefox, because
+    // puppeteer will send fractional intermediate positions and Firefox doesn't
+    // support them. Use this function to round each intermediate position to an
+    // integer.
+    async function moveInSteps(page, from, to, steps) {
+      const deltaX = to.x - from.x;
+      const deltaY = to.y - from.y;
+      for (let i = 0; i <= steps; i++) {
+        const x = Math.round(from.x + (deltaX * i) / steps);
+        const y = Math.round(from.y + (deltaY * i) / steps);
+        await page.mouse.move(x, y);
+      }
+    }
+
+    function middlePosition(rect) {
+      return { x: rect.x + rect.width / 2, y: rect.y + rect.height / 2 };
+    }
+
+    function middleLeftPosition(rect) {
+      return { x: rect.x + 1, y: rect.y + rect.height / 2 };
+    }
+
+    function belowEndPosition(rect) {
+      return { x: rect.x + rect.width, y: rect.y + rect.height * 1.5 };
+    }
+
+    beforeAll(() => {
+      jasmine.addAsyncMatchers({
+        // Check that a page has a selection containing the given text, with
+        // some tolerance for extra characters before/after.
+        toHaveRoughlySelected({ pp }) {
+          return {
+            async compare(page, expected) {
+              const TOLERANCE = 10;
+
+              const actual = await page.evaluate(() =>
+                // We need to normalize EOL for Windows
+                window.getSelection().toString().replaceAll("\r\n", "\n")
+              );
+
+              let start, end;
+              if (expected instanceof RegExp) {
+                const match = expected.exec(actual);
+                start = -1;
+                if (match) {
+                  start = match.index;
+                  end = start + match[0].length;
+                }
+              } else {
+                start = actual.indexOf(expected);
+                if (start !== -1) {
+                  end = start + expected.length;
+                }
+              }
+
+              const pass =
+                start !== -1 &&
+                start < TOLERANCE &&
+                end > actual.length - TOLERANCE;
+
+              return {
+                pass,
+                message: `Expected ${pp(
+                  actual.length > 200
+                    ? actual.slice(0, 100) + "[...]" + actual.slice(-100)
+                    : actual
+                )} to ${pass ? "not " : ""}roughly match ${pp(expected)}.`,
+              };
+            },
+          };
+        },
+      });
+    });
+
+    describe("using mouse", () => {
+      let pages;
+
+      beforeAll(async () => {
+        pages = await loadAndWait(
+          "tracemonkey.pdf",
+          `.page[data-page-number = "1"] .endOfContent`
+        );
+      });
+      afterAll(async () => {
+        await closePages(pages);
+      });
+
+      it("doesn't jump when hovering on an empty area", async () => {
+        await Promise.all(
+          pages.map(async ([browserName, page]) => {
+            const [positionStart, positionEnd] = await Promise.all([
+              getSpanRectFromText(
+                page,
+                1,
+                "(frequently executed) bytecode sequences, records"
+              ).then(middlePosition),
+              getSpanRectFromText(
+                page,
+                1,
+                "them, and compiles them to fast native code. We call such a se-"
+              ).then(belowEndPosition),
+            ]);
+
+            await page.mouse.move(positionStart.x, positionStart.y);
+            await page.mouse.down();
+            await moveInSteps(page, positionStart, positionEnd, 20);
+            await page.mouse.up();
+
+            await expectAsync(page)
+              .withContext(`In ${browserName}`)
+              .toHaveRoughlySelected(
+                "code sequences, records\n" +
+                  "them, and compiles them to fast native code. We call suc"
+              );
+          })
+        );
+      });
+
+      it("doesn't jump when hovering on an empty area (multi-page)", async () => {
+        await Promise.all(
+          pages.map(async ([browserName, page]) => {
+            const scrollTarget = await getSpanRectFromText(
+              page,
+              1,
+              "Unlike method-based dynamic compilers, our dynamic com-"
+            );
+            await page.evaluate(top => {
+              document.getElementById("viewerContainer").scrollTop = top;
+            }, scrollTarget.y - 50);
+
+            const [
+              positionStartPage1,
+              positionEndPage1,
+              positionStartPage2,
+              positionEndPage2,
+            ] = await Promise.all([
+              getSpanRectFromText(
+                page,
+                1,
+                "Each compiled trace covers one path through the program with"
+              ).then(middlePosition),
+              getSpanRectFromText(
+                page,
+                1,
+                "or that the same types will occur in subsequent loop iterations."
+              ).then(middlePosition),
+              getSpanRectFromText(
+                page,
+                2,
+                "Hence, recording and compiling a trace"
+              ).then(middlePosition),
+              getSpanRectFromText(
+                page,
+                2,
+                "cache. Alternatively, the VM could simply stop tracing, and give up"
+              ).then(belowEndPosition),
+            ]);
+
+            await page.mouse.move(positionStartPage1.x, positionStartPage1.y);
+            await page.mouse.down();
+
+            await moveInSteps(page, positionStartPage1, positionEndPage1, 20);
+            await moveInSteps(page, positionEndPage1, positionStartPage2, 20);
+
+            await expectAsync(page)
+              .withContext(`In ${browserName}, first selection`)
+              .toHaveRoughlySelected(
+                /path through the program .*Hence, recording a/s
+              );
+
+            await moveInSteps(page, positionStartPage2, positionEndPage2, 20);
+            await page.mouse.up();
+
+            await expectAsync(page)
+              .withContext(`In ${browserName}, second selection`)
+              .toHaveRoughlySelected(
+                /path through.*Hence, recording and .* tracing, and give/s
+              );
+          })
+        );
+      });
+    });
+
+    describe("using selection carets", () => {
+      let browser;
+      let page;
+
+      beforeAll(async () => {
+        // Chrome does not support simulating caret-based selection, so this
+        // test only runs in Firefox.
+        browser = await startBrowser({
+          browserName: "firefox",
+          startUrl: "",
+          extraPrefsFirefox: {
+            "layout.accessiblecaret.enabled": true,
+            "layout.accessiblecaret.hide_carets_for_mouse_input": false,
+          },
+        });
+        page = await browser.newPage();
+        await page.goto(
+          `${global.integrationBaseUrl}?file=/test/pdfs/tracemonkey.pdf#zoom=page-fit`
+        );
+        await page.bringToFront();
+        await page.waitForSelector(
+          `.page[data-page-number = "1"] .endOfContent`,
+          { timeout: 0 }
+        );
+      });
+      afterAll(async () => {
+        await browser.close();
+      });
+
+      it("doesn't jump when moving selection", async () => {
+        const [initialStart, initialEnd, finalEnd] = await Promise.all([
+          getSpanRectFromText(
+            page,
+            1,
+            "(frequently executed) bytecode sequences, records"
+          ).then(middleLeftPosition),
+          getSpanRectFromText(
+            page,
+            1,
+            "(frequently executed) bytecode sequences, records"
+          ).then(middlePosition),
+          getSpanRectFromText(
+            page,
+            1,
+            "them, and compiles them to fast native code. We call such a se-"
+          ).then(belowEndPosition),
+        ]);
+
+        await page.mouse.move(initialStart.x, initialStart.y);
+        await page.mouse.down();
+        await moveInSteps(page, initialStart, initialEnd, 20);
+        await page.mouse.up();
+
+        await expectAsync(page)
+          .withContext(`first selection`)
+          .toHaveRoughlySelected("frequently executed) byt");
+
+        const initialCaretPos = {
+          x: initialEnd.x,
+          y: initialEnd.y + 10,
+        };
+        const intermediateCaretPos = {
+          x: finalEnd.x,
+          y: finalEnd.y + 5,
+        };
+        const finalCaretPos = {
+          x: finalEnd.x + 20,
+          y: finalEnd.y + 5,
+        };
+
+        await page.mouse.move(initialCaretPos.x, initialCaretPos.y);
+        await page.mouse.down();
+        await moveInSteps(page, initialCaretPos, intermediateCaretPos, 20);
+        await page.mouse.up();
+
+        await expectAsync(page)
+          .withContext(`second selection`)
+          .toHaveRoughlySelected(/frequently .* We call such a se/s);
+
+        await page.mouse.down();
+        await moveInSteps(page, intermediateCaretPos, finalCaretPos, 20);
+        await page.mouse.up();
+
+        await expectAsync(page)
+          .withContext(`third selection`)
+          .toHaveRoughlySelected(/frequently .* We call such a se/s);
+      });
+    });
+  });
+});

test/test.mjs

@@ -875,7 +875,12 @@ function unitTestPostHandler(req, res) {
   return true;
 }
 
-async function startBrowser({ browserName, headless, startUrl }) {
+async function startBrowser({
+  browserName,
+  headless = options.headless,
+  startUrl,
+  extraPrefsFirefox = {},
+}) {
   const options = {
     product: browserName,
     protocol: "cdp",
@@ -938,6 +943,7 @@ async function startBrowser({ browserName, headless, startUrl }) {
       "dom.events.asyncClipboard.clipboardItem": true,
       // It's helpful to see where the caret is.
       "accessibility.browsewithcaret": true,
+      ...extraPrefsFirefox,
     };
   }
 
@@ -991,7 +997,7 @@ async function startBrowsers({ baseUrl, initializeSession }) {
       startUrl = baseUrl + queryParameters;
     }
 
-    await startBrowser({ browserName, headless: options.headless, startUrl })
+    await startBrowser({ browserName, startUrl })
       .then(function (browser) {
         session.browser = browser;
         initializeSession(session);
@@ -1093,3 +1099,5 @@ var stats;
 var tempDir = null;
 
 main();
+
+export { startBrowser };

web/text_layer_builder.css

@@ -17,13 +17,14 @@
   position: absolute;
   text-align: initial;
   inset: 0;
-  overflow: hidden;
+  overflow: clip;
   opacity: 1;
   line-height: 1;
   text-size-adjust: none;
   forced-color-adjust: none;
   transform-origin: 0 0;
   caret-color: CanvasText;
+  z-index: 0;
 
   &.highlighting {
     touch-action: none;
@@ -37,6 +38,11 @@
     transform-origin: 0% 0%;
   }
 
+  > :not(.markedContent),
+  .markedContent span:not(.markedContent) {
+    z-index: 1;
+  }
+
   /* Only necessary in Google Chrome, see issue 14205, and most unfortunately
    * the problem doesn't show up in "text" reference tests. */
   /*#if !MOZCENTRAL*/
@@ -108,7 +114,7 @@
     display: block;
     position: absolute;
     inset: 100% 0 0;
-    z-index: -1;
+    z-index: 0;
     cursor: default;
     user-select: none;
 

web/text_layer_builder.js

@@ -47,6 +47,10 @@ class TextLayerBuilder {
 
   #textContentSource = null;
 
+  static #textLayers = new Map();
+
+  static #selectionChangeAbortController = null;
+
   constructor({
     highlighter = null,
     accessibilityManager = null,
@@ -75,7 +79,7 @@ class TextLayerBuilder {
     endOfContent.className = "endOfContent";
     this.div.append(endOfContent);
 
-    this.#bindMouse();
+    this.#bindMouse(endOfContent);
   }
 
   get numTextDivs() {
@@ -166,6 +170,7 @@ class TextLayerBuilder {
     this.textContentItemsStr.length = 0;
     this.textDivs.length = 0;
     this.textDivProperties = new WeakMap();
+    TextLayerBuilder.#removeGlobalSelectionListener(this.div);
   }
 
   /**
@@ -181,45 +186,13 @@ class TextLayerBuilder {
    * clicked. This reduces flickering of the content if the mouse is slowly
    * dragged up or down.
    */
-  #bindMouse() {
+  #bindMouse(end) {
     const { div } = this;
 
     div.addEventListener("mousedown", evt => {
-      const end = div.querySelector(".endOfContent");
-      if (!end) {
-        return;
-      }
-      if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
-        // On non-Firefox browsers, the selection will feel better if the height
-        // of the `endOfContent` div is adjusted to start at mouse click
-        // location. This avoids flickering when the selection moves up.
-        // However it does not work when selection is started on empty space.
-        let adjustTop = evt.target !== div;
-        if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) {
-          adjustTop &&=
-            getComputedStyle(end).getPropertyValue("-moz-user-select") !==
-            "none";
-        }
-        if (adjustTop) {
-          const divBounds = div.getBoundingClientRect();
-          const r = Math.max(0, (evt.pageY - divBounds.top) / divBounds.height);
-          end.style.top = (r * 100).toFixed(2) + "%";
-        }
-      }
       end.classList.add("active");
     });
 
-    div.addEventListener("mouseup", () => {
-      const end = div.querySelector(".endOfContent");
-      if (!end) {
-        return;
-      }
-      if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
-        end.style.top = "";
-      }
-      end.classList.remove("active");
-    });
-
     div.addEventListener("copy", event => {
       if (!this.#enablePermissions) {
         const selection = document.getSelection();
@@ -231,6 +204,131 @@ class TextLayerBuilder {
       event.preventDefault();
       event.stopPropagation();
     });
+
+    TextLayerBuilder.#textLayers.set(div, end);
+    TextLayerBuilder.#enableGlobalSelectionListener();
+  }
+
+  static #removeGlobalSelectionListener(textLayerDiv) {
+    this.#textLayers.delete(textLayerDiv);
+
+    if (this.#textLayers.size === 0) {
+      this.#selectionChangeAbortController?.abort();
+      this.#selectionChangeAbortController = null;
+    }
+  }
+
+  static #enableGlobalSelectionListener() {
+    if (TextLayerBuilder.#selectionChangeAbortController) {
+      // document-level event listeners already installed
+      return;
+    }
+    TextLayerBuilder.#selectionChangeAbortController = new AbortController();
+
+    const reset = (end, textLayer) => {
+      if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
+        textLayer.append(end);
+        end.style.width = "";
+        end.style.height = "";
+      }
+      end.classList.remove("active");
+    };
+
+    document.addEventListener(
+      "pointerup",
+      () => {
+        TextLayerBuilder.#textLayers.forEach(reset);
+      },
+      { signal: TextLayerBuilder.#selectionChangeAbortController.signal }
+    );
+
+    if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
+      // eslint-disable-next-line no-var
+      var isFirefox, prevRange;
+    }
+
+    document.addEventListener(
+      "selectionchange",
+      () => {
+        const selection = document.getSelection();
+        if (selection.rangeCount === 0) {
+          TextLayerBuilder.#textLayers.forEach(reset);
+          return;
+        }
+
+        // Even though the spec says that .rangeCount should be 0 or 1, Firefox
+        // creates multiple ranges when selecting across multiple pages.
+        // Make sure to collect all the .textLayer elements where the selection
+        // is happening.
+        const activeTextLayers = new Set();
+        for (let i = 0; i < selection.rangeCount; i++) {
+          const range = selection.getRangeAt(i);
+          for (const textLayerDiv of TextLayerBuilder.#textLayers.keys()) {
+            if (
+              !activeTextLayers.has(textLayerDiv) &&
+              range.intersectsNode(textLayerDiv)
+            ) {
+              activeTextLayers.add(textLayerDiv);
+            }
+          }
+        }
+
+        for (const [textLayerDiv, endDiv] of TextLayerBuilder.#textLayers) {
+          if (activeTextLayers.has(textLayerDiv)) {
+            endDiv.classList.add("active");
+          } else {
+            reset(endDiv, textLayerDiv);
+          }
+        }
+
+        if (typeof PDFJSDev === "undefined" || !PDFJSDev.test("MOZCENTRAL")) {
+          if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("CHROME")) {
+            isFirefox = false;
+          } else {
+            isFirefox ??=
+              getComputedStyle(
+                TextLayerBuilder.#textLayers.values().next().value
+              ).getPropertyValue("-moz-user-select") === "none";
+          }
+
+          if (!isFirefox) {
+            // In non-Firefox browsers, when hovering over an empty space (thus,
+            // on .endOfContent), the selection will expand to cover all the
+            // text between the current selection and .endOfContent. By moving
+            // .endOfContent to right after (or before, depending on which side
+            // of the selection the user is moving), we limit the selection jump
+            // to at most cover the enteirety of the <span> where the selection
+            // is being modified.
+            const range = selection.getRangeAt(0);
+            const modifyStart =
+              prevRange &&
+              (range.compareBoundaryPoints(Range.END_TO_END, prevRange) === 0 ||
+                range.compareBoundaryPoints(Range.START_TO_END, prevRange) ===
+                  0);
+            let anchor = modifyStart
+              ? range.startContainer
+              : range.endContainer;
+            if (anchor.nodeType === Node.TEXT_NODE) {
+              anchor = anchor.parentNode;
+            }
+
+            const parentTextLayer = anchor.parentElement.closest(".textLayer");
+            const endDiv = TextLayerBuilder.#textLayers.get(parentTextLayer);
+            if (endDiv) {
+              endDiv.style.width = parentTextLayer.style.width;
+              endDiv.style.height = parentTextLayer.style.height;
+              anchor.parentElement.insertBefore(
+                endDiv,
+                modifyStart ? anchor : anchor.nextSibling
+              );
+            }
+
+            prevRange = range.cloneRange();
+          }
+        }
+      },
+      { signal: TextLayerBuilder.#selectionChangeAbortController.signal }
+    );
   }
 }