diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 4c3d8fb09..489c781c7 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -63,16 +63,12 @@ class AnnotationEditorLayer { #boundPointerup = this.pointerup.bind(this); - #boundPointerUpAfterSelection = this.pointerUpAfterSelection.bind(this); - #boundPointerdown = this.pointerdown.bind(this); #boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this); #editorFocusTimeoutId = null; - #boundSelectionStart = this.selectionStart.bind(this); - #editors = new Map(); #hadPointerDown = false; @@ -338,7 +334,6 @@ class AnnotationEditorLayer { enableTextSelection() { if (this.#textLayer?.div) { - document.addEventListener("selectstart", this.#boundSelectionStart); this.#textLayer.div.addEventListener( "pointerdown", this.#boundTextLayerPointerDown @@ -349,7 +344,6 @@ class AnnotationEditorLayer { disableTextSelection() { if (this.#textLayer?.div) { - document.removeEventListener("selectstart", this.#boundSelectionStart); this.#textLayer.div.removeEventListener( "pointerdown", this.#boundTextLayerPointerDown @@ -686,54 +680,6 @@ class AnnotationEditorLayer { this.#uiManager.unselect(editor); } - /** - * SelectionChange callback. - * @param {Event} event - */ - selectionStart(event) { - if ( - !this.#textLayer || - event.target.parentElement.closest(".textLayer") !== this.#textLayer.div - ) { - return; - } - - if (this.#uiManager.isShiftKeyDown) { - const keyup = () => { - if (this.#uiManager.isShiftKeyDown) { - return; - } - - window.removeEventListener("keyup", keyup); - window.removeEventListener("blur", keyup); - this.pointerUpAfterSelection({}); - }; - window.addEventListener("keyup", keyup); - window.addEventListener("blur", keyup); - } else { - this.#textLayer.div.addEventListener( - "pointerup", - this.#boundPointerUpAfterSelection, - { once: true } - ); - } - } - - /** - * Called when the user releases the mouse button after having selected - * some text. - * @param {PointerEvent} event - */ - pointerUpAfterSelection(event) { - const boxes = this.#uiManager.getSelectionBoxes(this.#textLayer?.div); - if (boxes) { - this.createAndAddNewEditor(event, false, { - boxes, - }); - document.getSelection().empty(); - } - } - /** * Pointerup callback. * @param {PointerEvent} event diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index 09c9b14f3..2fb48452e 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -57,11 +57,19 @@ function opacityToHex(opacity) { class IdManager { #id = 0; + constructor() { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { + Object.defineProperty(this, "reset", { + value: () => (this.#id = 0), + }); + } + } + /** * Get a unique id. * @returns {string} */ - getId() { + get id() { return `${AnnotationEditorPrefix}${this.#id++}`; } } @@ -551,10 +559,10 @@ class AnnotationEditorUIManager { #focusMainContainerTimeoutId = null; - #hasSelection = false; - #highlightColors = null; + #highlightWhenShiftUp = false; + #idManager = new IdManager(); #isEnabled = false; @@ -780,6 +788,16 @@ class AnnotationEditorUIManager { rotation: 0, }; this.isShiftKeyDown = false; + + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { + Object.defineProperty(this, "reset", { + value: () => { + this.selectAll(); + this.delete(); + this.#idManager.reset(); + }, + }); + } } destroy() { @@ -958,8 +976,7 @@ class AnnotationEditorUIManager { #selectionChange() { const selection = document.getSelection(); if (!selection || selection.isCollapsed) { - if (this.#hasSelection) { - this.#hasSelection = false; + if (this.#selectedTextNode) { this.#selectedTextNode = null; this.#dispatchUpdateStates({ hasSelectedText: false, @@ -977,8 +994,7 @@ class AnnotationEditorUIManager { ? anchorNode.parentElement : anchorNode; if (!anchorElement.closest(".textLayer")) { - if (this.#hasSelection) { - this.#hasSelection = false; + if (this.#selectedTextNode) { this.#selectedTextNode = null; this.#dispatchUpdateStates({ hasSelectedText: false, @@ -986,11 +1002,30 @@ class AnnotationEditorUIManager { } return; } - this.#hasSelection = true; this.#selectedTextNode = anchorNode; this.#dispatchUpdateStates({ hasSelectedText: true, }); + + if (this.#mode !== AnnotationEditorType.HIGHLIGHT) { + return; + } + this.#highlightWhenShiftUp = this.isShiftKeyDown; + if (!this.isShiftKeyDown) { + const pointerup = e => { + if (e.type === "pointerup" && e.button !== 0) { + // Do nothing on right click. + return; + } + window.removeEventListener("pointerup", pointerup); + window.removeEventListener("blur", pointerup); + if (e.type === "pointerup") { + this.highlightSelection(); + } + }; + window.addEventListener("pointerup", pointerup); + window.addEventListener("blur", pointerup); + } } #addSelectionListener() { @@ -1013,6 +1048,10 @@ class AnnotationEditorUIManager { blur() { this.isShiftKeyDown = false; + if (this.#highlightWhenShiftUp) { + this.#highlightWhenShiftUp = false; + this.highlightSelection(); + } if (!this.hasSelection) { return; } @@ -1199,6 +1238,10 @@ class AnnotationEditorUIManager { keyup(event) { if (this.isShiftKeyDown && event.key === "Shift") { this.isShiftKeyDown = false; + if (this.#highlightWhenShiftUp) { + this.#highlightWhenShiftUp = false; + this.highlightSelection(); + } } } @@ -1287,7 +1330,7 @@ class AnnotationEditorUIManager { * @returns {string} */ getId() { - return this.#idManager.getId(); + return this.#idManager.id; } get currentLayer() { diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs index 188c65cc7..b368f0cd9 100644 --- a/test/integration/highlight_editor_spec.mjs +++ b/test/integration/highlight_editor_spec.mjs @@ -83,7 +83,11 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + // Here and elsewhere, we add a small delay between press and release + // to make sure that a pointerup event is triggered after + // selectionchange. + // It works with a value of 1ms, but we use 100ms to be sure. + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); @@ -128,7 +132,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector( @@ -179,7 +183,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector( @@ -225,7 +229,7 @@ describe("Highlight Editor", () => { let rect = await getSpanRectFromText(page, 1, "Abstract"); let x = rect.x + rect.width / 2; let y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector( @@ -248,7 +252,7 @@ describe("Highlight Editor", () => { rect = await getSpanRectFromText(page, 14, "References"); x = rect.x + rect.width / 2; y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(1)}`); await page.waitForSelector( `.page[data-page-number = "14"] svg.highlightOutline.selected` @@ -314,7 +318,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector( @@ -376,7 +380,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(sel); await page.waitForSelector( @@ -487,7 +491,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.waitForSelector( @@ -535,7 +539,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(sel); await page.waitForSelector( @@ -577,7 +581,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(sel); await page.waitForSelector( @@ -871,7 +875,7 @@ describe("Highlight Editor", () => { ); const x = rect.x + 0.75 * rect.width; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); const usedColor = await page.evaluate(() => { @@ -981,7 +985,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForFunction(() => window.editingEvents.length > 0); let editingEvent = await page.evaluate(() => { @@ -1008,7 +1012,7 @@ describe("Highlight Editor", () => { .withContext(`In ${browserName}`) .toBe(false); - await page.mouse.click(x, y, { count: 2 }); + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForFunction(() => window.editingEvents.length > 0); await page.evaluate(() => { @@ -1039,7 +1043,17 @@ describe("Highlight Editor", () => { "tracemonkey.pdf", ".annotationEditorLayer", null, - null, + async page => { + await page.evaluate(async () => { + await window.PDFViewerApplication.initializedPromise; + window.PDFViewerApplication.eventBus.on( + "annotationeditoruimanager", + ({ uiManager }) => { + window.uiManager = uiManager; + } + ); + }); + }, { highlightEditorColors: "red=#AB0000", supportsCaretBrowsingMode: true, @@ -1047,6 +1061,19 @@ describe("Highlight Editor", () => { ); }); + afterEach(async () => { + for (const [, page] of pages) { + await page.evaluate(() => { + window.uiManager.reset(); + }); + // Disable editing mode. + await page.click("#editorHighlight"); + await page.waitForSelector( + `.annotationEditorLayer:not(.highlightEditing)` + ); + } + }); + afterAll(async () => { await closePages(pages); }); @@ -1060,8 +1087,7 @@ describe("Highlight Editor", () => { const rect = await getSpanRectFromText(page, 1, "Abstract"); const x = rect.x + rect.width / 2; const y = rect.y + rect.height / 2; - await page.mouse.click(x, y, { count: 2 }); - + await page.mouse.click(x, y, { count: 2, delay: 100 }); await page.waitForSelector(`${getEditorSelector(0)}`); await page.keyboard.press("Escape"); await page.waitForSelector( @@ -1092,5 +1118,41 @@ describe("Highlight Editor", () => { }) ); }); + + it("must check that selection is correctly highlighted on arrow down key pressed", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorHighlight"); + await page.waitForSelector(".annotationEditorLayer.highlightEditing"); + + await page.evaluate(() => { + const text = + "Dynamic languages such as JavaScript are more difficult to com-"; + for (const el of document.querySelectorAll( + `.page[data-page-number="${1}"] > .textLayer > span` + )) { + if (el.textContent === text) { + window.getSelection().setPosition(el.firstChild, 15); + break; + } + } + }); + + await page.keyboard.down("Shift"); + await page.keyboard.press("ArrowDown"); + await page.keyboard.up("Shift"); + + await page.waitForSelector(getEditorSelector(0)); + const usedColor = await page.evaluate(() => { + const highlight = document.querySelector( + `.page[data-page-number = "1"] .canvasWrapper > svg.highlight` + ); + return highlight.getAttribute("fill"); + }); + + expect(usedColor).withContext(`In ${browserName}`).toEqual("#AB0000"); + }) + ); + }); }); }); diff --git a/test/test.mjs b/test/test.mjs index 91c0b8262..1080be89d 100644 --- a/test/test.mjs +++ b/test/test.mjs @@ -949,6 +949,8 @@ async function startBrowser({ browserName, headless, startUrl }) { "layout.css.round.enabled": true, // This allow to copy some data in the clipboard. "dom.events.asyncClipboard.clipboardItem": true, + // It's helpful to see where the caret is. + "accessibility.browsewithcaret": true, }; }