From ce4144eee46770d7a0d5aaddebdbe0eaf3aaf6bc Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Wed, 27 Jul 2022 14:11:29 +0200 Subject: [PATCH] [Editor] Avoid editor creation/selection on right click (bug 1781762) --- src/display/editor/annotation_editor_layer.js | 14 ++- src/display/editor/editor.js | 1 + src/display/editor/freetext.js | 12 +++ src/display/editor/tools.js | 1 + test/integration/freetext_editor_spec.js | 86 +++++++++++++++---- test/integration/ink_editor_spec.js | 4 +- 6 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 8ffb0cecb..d1b031463 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -21,8 +21,8 @@ /** @typedef {import("../../web/interfaces").IL10n} IL10n */ import { AnnotationEditorType, shadow } from "../../shared/util.js"; +import { bindEvents, KeyboardManager } from "./tools.js"; import { binarySearchFirstItem } from "../display_utils.js"; -import { bindEvents } from "./tools.js"; import { FreeTextEditor } from "./freetext.js"; import { InkEditor } from "./ink.js"; @@ -577,6 +577,12 @@ class AnnotationEditorLayer { * @param {PointerEvent} event */ pointerup(event) { + const isMac = KeyboardManager.platform.isMac; + if (event.button !== 0 || (event.ctrlKey && isMac)) { + // Don't create an editor on right click. + return; + } + if (event.target !== this.div) { return; } @@ -594,6 +600,12 @@ class AnnotationEditorLayer { * @param {PointerEvent} event */ pointerdown(event) { + const isMac = KeyboardManager.platform.isMac; + if (event.button !== 0 || (event.ctrlKey && isMac)) { + // Do nothing on right click. + return; + } + if (event.target !== this.div) { return; } diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 76fcd6110..fcb912268 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -253,6 +253,7 @@ class AnnotationEditor { if (event.button !== 0 || (event.ctrlKey && isMac)) { // Avoid to focus this editor because of a non-left click. event.preventDefault(); + return; } if ( diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index 307f61e2a..5bd9e5f24 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -217,6 +217,10 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ enableEditMode() { + if (this.isInEditMode()) { + return; + } + this.parent.setEditingState(false); this.parent.updateToolbar(AnnotationEditorType.FREETEXT); super.enableEditMode(); @@ -230,6 +234,10 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ disableEditMode() { + if (!this.isInEditMode()) { + return; + } + this.parent.setEditingState(true); super.disableEditMode(); this.overlayDiv.classList.add("enabled"); @@ -239,6 +247,10 @@ class FreeTextEditor extends AnnotationEditor { this.editorDiv.removeEventListener("focus", this.#boundEditorDivFocus); this.editorDiv.removeEventListener("blur", this.#boundEditorDivBlur); + // On Chrome, the focus is given to when contentEditable is set to + // false, hence we focus the div. + this.div.focus(); + // In case the blur callback hasn't been called. this.isEditing = false; } diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index 77b433dc7..f67579587 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -244,6 +244,7 @@ class KeyboardManager { return; } callback.bind(self)(); + event.stopPropagation(); event.preventDefault(); } } diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index b5461e0c8..18ea0c004 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -25,7 +25,7 @@ describe("Editor", () => { let pages; beforeAll(async () => { - pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer"); + pages = await loadAndWait("aboutstacks.pdf", ".annotationEditorLayer"); }); afterAll(async () => { @@ -204,13 +204,13 @@ describe("Editor", () => { it("must check that aria-owns is correct", async () => { await Promise.all( pages.map(async ([browserName, page]) => { - const [adobeComRect, oldAriaOwns] = await page.$eval( + const [stacksRect, oldAriaOwns] = await page.$eval( ".textLayer", el => { for (const span of el.querySelectorAll( `span[role="presentation"]` )) { - if (span.innerText.includes("adobe.com")) { + if (span.innerText.includes("Stacks are simple to create")) { span.setAttribute("pdfjs", true); const { x, y, width, height } = span.getBoundingClientRect(); return [ @@ -227,21 +227,13 @@ describe("Editor", () => { const data = "Hello PDF.js World !!"; await page.mouse.click( - adobeComRect.x + adobeComRect.width + 10, - adobeComRect.y + adobeComRect.height / 2 + stacksRect.x + stacksRect.width + 1, + stacksRect.y + stacksRect.height / 2 ); await page.type(`${editorPrefix}5 .internal`, data); - const editorRect = await page.$eval(`${editorPrefix}5`, el => { - const { x, y, width, height } = el.getBoundingClientRect(); - return { x, y, width, height }; - }); - // Commit. - await page.mouse.click( - editorRect.x, - editorRect.y + 2 * editorRect.height - ); + await page.keyboard.press("Escape"); const ariaOwns = await page.$eval(".textLayer", el => { const span = el.querySelector(`span[pdfjs="true"]`); @@ -254,13 +246,77 @@ describe("Editor", () => { }) ); }); + + it("must check that right click doesn't select", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + const rect = await page.$eval(".annotationEditorLayer", el => { + const { x, y } = el.getBoundingClientRect(); + return { x, y }; + }); + + await page.keyboard.down("Control"); + await page.keyboard.press("a"); + await page.keyboard.up("Control"); + + await page.keyboard.down("Control"); + await page.keyboard.press("Backspace"); + await page.keyboard.up("Control"); + + const data = "Hello PDF.js World !!"; + await page.mouse.click(rect.x + 100, rect.y + 100); + await page.type(`${editorPrefix}6 .internal`, data); + + const editorRect = await page.$eval(`${editorPrefix}6`, el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { x, y, width, height }; + }); + + // Commit. + await page.keyboard.press("Escape"); + + expect(await getSelectedEditors(page)) + .withContext(`In ${browserName}`) + .toEqual([6]); + + await page.keyboard.press("Escape"); + expect(await getSelectedEditors(page)) + .withContext(`In ${browserName}`) + .toEqual([]); + + await page.mouse.click( + editorRect.x + editorRect.width / 2, + editorRect.y + editorRect.height / 2 + ); + + expect(await getSelectedEditors(page)) + .withContext(`In ${browserName}`) + .toEqual([6]); + + // Escape. + await page.keyboard.press("Escape"); + + expect(await getSelectedEditors(page)) + .withContext(`In ${browserName}`) + .toEqual([]); + + // TODO: uncomment that stuff once we've a way to dismiss + // the context menu. + /* await page.mouse.click( + editorRect.x + editorRect.width / 2, + editorRect.y + editorRect.height / 2, + { button: "right" } + ); */ + }) + ); + }); }); describe("FreeText (multiselection)", () => { let pages; beforeAll(async () => { - pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer"); + pages = await loadAndWait("aboutstacks.pdf", ".annotationEditorLayer"); }); afterAll(async () => { diff --git a/test/integration/ink_editor_spec.js b/test/integration/ink_editor_spec.js index fe1834199..7563ffd37 100644 --- a/test/integration/ink_editor_spec.js +++ b/test/integration/ink_editor_spec.js @@ -24,7 +24,7 @@ describe("Editor", () => { let pages; beforeAll(async () => { - pages = await loadAndWait("tracemonkey.pdf", ".annotationEditorLayer"); + pages = await loadAndWait("aboutstacks.pdf", ".annotationEditorLayer"); }); afterAll(async () => { @@ -60,7 +60,7 @@ describe("Editor", () => { expect(await getSelectedEditors(page)) .withContext(`In ${browserName}`) - .toEqual([0, 2, 3]); + .toEqual([0, 1, 2]); await page.keyboard.press("Backspace");