From 4e7c30da9a09ad3b46eaf1a6c4e93934a741db89 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 11 Jul 2024 16:18:50 +0200 Subject: [PATCH] [Editor] Disable existing highlights when drawing a new one (bug 1879035) 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. --- src/display/editor/annotation_editor_layer.js | 13 +- src/display/editor/tools.js | 45 +++++-- test/integration/highlight_editor_spec.mjs | 124 ++++++++++++++++++ web/annotation_editor_layer_builder.css | 4 + 4 files changed, 172 insertions(+), 14 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 5c4e9445a..52d76d356 100644 --- 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 index 4adebd986..17d7e497a 100644 --- 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 index 03ff854ca..b8b55ed89 100644 --- 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 index 139039bbb..eb13c7e46 100644 --- 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 {