Merge pull request #18424 from calixteman/avoid_hovering_when_selecting

[Editor] Disable existing highlights when drawing a new one (bug 1879035)
mozilla · Jul 12, 2024 · 2d25437 · 2d25437
2 parents df5bc54 + 4e7c30d
commit 2d25437
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 {