[Editor] Disable existing highlights when drawing a new one (bug 1879…

…035) When the mouse was hovering an existing highlight, all the text in the page was selected. So when the user is selecting some text or drawing a free highlight, the mouse is disabled for the existing editors.
mozilla · Jul 11, 2024 · 4e7c30d · 4e7c30d
1 parent df5bc54
commit 4e7c30d
Show file tree

Hide file tree

Showing 4 changed files with 172 additions and 14 deletions.
diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js
@@ -230,6 +230,10 @@ class AnnotationEditorLayer {
     this.#uiManager.addCommands(params);
   }
 
+  toggleDrawing(enabled = false) {
+    this.div.classList.toggle("drawing", !enabled);
+  }
+
   togglePointerEvents(enabled = false) {
     this.div.classList.toggle("disabled", !enabled);
   }
@@ -388,7 +392,12 @@ class AnnotationEditorLayer {
     // Unselect all the editors in order to let the user select some text
     // without being annoyed by an editor toolbar.
     this.#uiManager.unselectAll();
-    if (event.target === this.#textLayer.div) {
+    const { target } = event;
+    if (
+      target === this.#textLayer.div ||
+      (target.classList.contains("endOfContent") &&
+        this.#textLayer.div.contains(target))
+    ) {
       const { isMac } = FeatureTest.platform;
       if (event.button !== 0 || (event.ctrlKey && isMac)) {
         // Do nothing on right click.
@@ -400,6 +409,7 @@ class AnnotationEditorLayer {
         /* updateButton = */ true
       );
       this.#textLayer.div.classList.add("free");
+      this.toggleDrawing();
       HighlightEditor.startHighlighting(
         this,
         this.#uiManager.direction === "ltr",
@@ -409,6 +419,7 @@ class AnnotationEditorLayer {
         "pointerup",
         () => {
           this.#textLayer.div.classList.remove("free");
+          this.toggleDrawing(true);
         },
         { once: true, signal: this.#uiManager._signal }
       );

diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js
@@ -967,6 +967,19 @@ class AnnotationEditorUIManager {
       : anchorNode;
   }
 
+  #getLayerForTextLayer(textLayer) {
+    const { currentLayer } = this;
+    if (currentLayer.hasTextLayer(textLayer)) {
+      return currentLayer;
+    }
+    for (const layer of this.#allLayers.values()) {
+      if (layer.hasTextLayer(textLayer)) {
+        return layer;
+      }
+    }
+    return null;
+  }
+
   highlightSelection(methodOfCreation = "") {
     const selection = document.getSelection();
     if (!selection || selection.isCollapsed) {
@@ -988,19 +1001,17 @@ class AnnotationEditorUIManager {
       });
       this.showAllEditors("highlight", true, /* updateButton = */ true);
     }
-    for (const layer of this.#allLayers.values()) {
-      if (layer.hasTextLayer(textLayer)) {
-        layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
-          methodOfCreation,
-          boxes,
-          anchorNode,
-          anchorOffset,
-          focusNode,
-          focusOffset,
-          text,
-        });
-        break;
-      }
+    const layer = this.#getLayerForTextLayer(textLayer);
+    if (layer) {
+      layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
+        methodOfCreation,
+        boxes,
+        anchorNode,
+        anchorOffset,
+        focusNode,
+        focusOffset,
+        text,
+      });
     }
   }
 
@@ -1062,6 +1073,7 @@ class AnnotationEditorUIManager {
       }
       return;
     }
+
     this.#highlightToolbar?.hide();
     this.#selectedTextNode = anchorNode;
     this.#dispatchUpdateStates({
@@ -1081,12 +1093,19 @@ class AnnotationEditorUIManager {
 
     this.#highlightWhenShiftUp = this.isShiftKeyDown;
     if (!this.isShiftKeyDown) {
+      const activeLayer =
+        this.#mode === AnnotationEditorType.HIGHLIGHT
+          ? this.#getLayerForTextLayer(textLayer)
+          : null;
+      activeLayer?.toggleDrawing();
+
       const signal = this._signal;
       const pointerup = e => {
         if (e.type === "pointerup" && e.button !== 0) {
           // Do nothing on right click.
           return;
         }
+        activeLayer?.toggleDrawing(true);
         window.removeEventListener("pointerup", pointerup);
         window.removeEventListener("blur", pointerup);
         if (e.type === "pointerup") {

diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs
@@ -1766,4 +1766,128 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Draw a free highlight with the pointer hovering an existing highlight", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that an existing highlight is ignored on hovering", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToHighlight(page);
+
+          const rect = await getSpanRectFromText(page, 1, "Abstract");
+          const editorSelector = getEditorSelector(0);
+          const x = rect.x + rect.width / 2;
+          let y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 2, delay: 100 });
+          await page.waitForSelector(editorSelector);
+          await waitForSerialized(page, 1);
+          await page.keyboard.press("Escape");
+          await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);
+
+          const counterHandle = await page.evaluateHandle(sel => {
+            const el = document.querySelector(sel);
+            const counter = { count: 0 };
+            el.addEventListener(
+              "pointerover",
+              () => {
+                counter.count += 1;
+              },
+              { capture: true }
+            );
+            return counter;
+          }, editorSelector);
+
+          const clickHandle = await waitForPointerUp(page);
+          y = rect.y - rect.height;
+          await page.mouse.move(x, y);
+          await page.mouse.down();
+          for (
+            const endY = rect.y + 2 * rect.height;
+            y <= endY;
+            y += rect.height / 10
+          ) {
+            await page.mouse.move(x, y);
+          }
+          await page.mouse.up();
+          await awaitPromise(clickHandle);
+
+          const { count } = await counterHandle.jsonValue();
+          expect(count).withContext(`In ${browserName}`).toEqual(0);
+        })
+      );
+    });
+  });
+
+  describe("Select text with the pointer hovering an existing highlight", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that an existing highlight is ignored on hovering", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToHighlight(page);
+
+          const rect = await getSpanRectFromText(
+            page,
+            1,
+            "ternative compilation technique for dynamically-typed languages"
+          );
+          const editorSelector = getEditorSelector(0);
+          const x = rect.x + rect.width / 2;
+          let y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 3, delay: 100 });
+          await page.waitForSelector(editorSelector);
+          await waitForSerialized(page, 1);
+          await page.keyboard.press("Escape");
+          await page.waitForSelector(`${editorSelector}:not(.selectedEditor)`);
+
+          const counterHandle = await page.evaluateHandle(sel => {
+            const el = document.querySelector(sel);
+            const counter = { count: 0 };
+            el.addEventListener(
+              "pointerover",
+              () => {
+                counter.count += 1;
+              },
+              { capture: true }
+            );
+            return counter;
+          }, editorSelector);
+
+          const clickHandle = await waitForPointerUp(page);
+          y = rect.y - 3 * rect.height;
+          await page.mouse.move(x, y);
+          await page.mouse.down();
+          for (
+            const endY = rect.y + 3 * rect.height;
+            y <= endY;
+            y += rect.height / 10
+          ) {
+            await page.mouse.move(x, y);
+          }
+          await page.mouse.up();
+          await awaitPromise(clickHandle);
+
+          const { count } = await counterHandle.jsonValue();
+          expect(count).withContext(`In ${browserName}`).toEqual(0);
+        })
+      );
+    });
+  });
 });
diff --git a/web/annotation_editor_layer_builder.css b/web/annotation_editor_layer_builder.css
@@ -116,6 +116,10 @@
   .selectedEditor {
     z-index: 100000 !important;
   }
+
+  &.drawing * {
+    pointer-events: none !important;
+  }
 }
 
 .annotationEditorLayer.waiting {