Merge pull request #19969 from calixteman/editor_inconsistencies

[Editor] Change mode when double clicking on an editor

Author: calixteman

src/display/editor/annotation_editor_layer.js

@@ -148,10 +148,10 @@ class AnnotationEditorLayer {
 
   /**
    * Update the toolbar if it's required to reflect the tool currently used.
-   * @param {number} mode
+   * @param {Object} options
    */
-  updateToolbar(mode) {
-    this.#uiManager.updateToolbar(mode);
+  updateToolbar(options) {
+    this.#uiManager.updateToolbar(options);
   }
 
   /**
@@ -618,12 +618,12 @@ class AnnotationEditorLayer {
 
   /**
    * Paste some content into a new editor.
-   * @param {number} mode
+   * @param {Object} options
    * @param {Object} params
    */
-  async pasteEditor(mode, params) {
-    this.#uiManager.updateToolbar(mode);
-    await this.#uiManager.updateMode(mode);
+  async pasteEditor(options, params) {
+    this.updateToolbar(options);
+    await this.#uiManager.updateMode(options.mode);
 
     const { offsetX, offsetY } = this.#getCenterPoint();
     const id = this.getNextId();

src/display/editor/editor.js

@@ -90,6 +90,8 @@ class AnnotationEditor {
 
   #touchManager = null;
 
+  isSelected = false;
+
   _isCopy = false;
 
   _editToolbar = null;
@@ -1170,7 +1172,7 @@ class AnnotationEditor {
     const [tx, ty] = this.getInitialTranslation();
     this.translate(tx, ty);
 
-    bindEvents(this, div, ["keydown", "pointerdown"]);
+    bindEvents(this, div, ["keydown", "pointerdown", "dblclick"]);
 
     if (this.isResizable && this._uiManager._supportsPinchToZoom) {
       this.#touchManager ||= new TouchManager({
@@ -1279,10 +1281,6 @@ class AnnotationEditor {
     this.#selectOnPointerEvent(event);
   }
 
-  get isSelected() {
-    return this._uiManager.isSelected(this);
-  }
-
   #selectOnPointerEvent(event) {
     const { isMac } = FeatureTest.platform;
     if (
@@ -1499,16 +1497,30 @@ class AnnotationEditor {
 
   /**
    * Enable edit mode.
+   * @returns {boolean} - true if the edit mode has been enabled.
    */
   enableEditMode() {
+    if (this.isInEditMode()) {
+      return false;
+    }
+    this.parent.setEditingState(false);
     this.#isInEditMode = true;
+
+    return true;
   }
 
   /**
    * Disable edit mode.
+   * @returns {boolean} - true if the edit mode has been disabled.
    */
   disableEditMode() {
+    if (!this.isInEditMode()) {
+      return false;
+    }
+    this.parent.setEditingState(true);
     this.#isInEditMode = false;
+
+    return true;
   }
 
   /**
@@ -1832,6 +1844,10 @@ class AnnotationEditor {
    * Select this editor.
    */
   select() {
+    if (this.isSelected && this._editToolbar) {
+      return;
+    }
+    this.isSelected = true;
     this.makeResizable();
     this.div?.classList.add("selectedEditor");
     if (!this._editToolbar) {
@@ -1853,6 +1869,10 @@ class AnnotationEditor {
    * Unselect this editor.
    */
   unselect() {
+    if (!this.isSelected) {
+      return;
+    }
+    this.isSelected = false;
     this.#resizersDiv?.classList.add("hidden");
     this.div?.classList.remove("selectedEditor");
     if (this.div?.contains(document.activeElement)) {
@@ -1885,10 +1905,38 @@ class AnnotationEditor {
    */
   enableEditing() {}
 
+  /**
+   * Check if the content of this editor can be changed.
+   * For example, a FreeText editor can be changed (the user can change the
+   * text), but a Stamp editor cannot.
+   * @returns {boolean}
+   */
+  get canChangeContent() {
+    return false;
+  }
+
   /**
    * The editor is about to be edited.
    */
-  enterInEditMode() {}
+  enterInEditMode() {
+    if (!this.canChangeContent) {
+      return;
+    }
+    this.enableEditMode();
+    this.div.focus();
+  }
+
+  /**
+   * ondblclick callback.
+   * @param {MouseEvent} event
+   */
+  dblclick(event) {
+    this.enterInEditMode();
+    this.parent.updateToolbar({
+      mode: this.constructor._editorType,
+      editId: this.id,
+    });
+  }
 
   /**
    * @returns {HTMLElement | null} the element requiring an alt text.

src/display/editor/freetext.js

@@ -24,11 +24,7 @@ import {
   shadow,
   Util,
 } from "../../shared/util.js";
-import {
-  AnnotationEditorUIManager,
-  bindEvents,
-  KeyboardManager,
-} from "./tools.js";
+import { AnnotationEditorUIManager, KeyboardManager } from "./tools.js";
 import { AnnotationEditor } from "./editor.js";
 import { FreeTextAnnotationElement } from "../annotation_layer.js";
 
@@ -284,13 +280,10 @@ class FreeTextEditor extends AnnotationEditor {
 
   /** @inheritdoc */
   enableEditMode() {
-    if (this.isInEditMode()) {
-      return;
+    if (!super.enableEditMode()) {
+      return false;
     }
 
-    this.parent.setEditingState(false);
-    this.parent.updateToolbar(AnnotationEditorType.FREETEXT);
-    super.enableEditMode();
     this.overlayDiv.classList.remove("enabled");
     this.editorDiv.contentEditable = true;
     this._isDraggable = false;
@@ -322,16 +315,16 @@ class FreeTextEditor extends AnnotationEditor {
     this.editorDiv.addEventListener("paste", this.editorDivPaste.bind(this), {
       signal,
     });
+
+    return true;
   }
 
   /** @inheritdoc */
   disableEditMode() {
-    if (!this.isInEditMode()) {
-      return;
+    if (!super.disableEditMode()) {
+      return false;
     }
 
-    this.parent.setEditingState(true);
-    super.disableEditMode();
     this.overlayDiv.classList.add("enabled");
     this.editorDiv.contentEditable = false;
     this.div.setAttribute("aria-activedescendant", this.#editorDivId);
@@ -349,6 +342,8 @@ class FreeTextEditor extends AnnotationEditor {
     // In case the blur callback hasn't been called.
     this.isEditing = false;
     this.parent.div.classList.add("freetextEditing");
+
+    return true;
   }
 
   /** @inheritdoc */
@@ -498,18 +493,7 @@ class FreeTextEditor extends AnnotationEditor {
     this.editorDiv.focus();
   }
 
-  /**
-   * ondblclick callback.
-   * @param {MouseEvent} event
-   */
-  dblclick(event) {
-    this.enterInEditMode();
-  }
-
-  /**
-   * onkeydown callback.
-   * @param {KeyboardEvent} event
-   */
+  /** @inheritdoc */
   keydown(event) {
     if (event.target === this.div && event.key === "Enter") {
       this.enterInEditMode();
@@ -546,6 +530,11 @@ class FreeTextEditor extends AnnotationEditor {
     this.editorDiv.setAttribute("aria-multiline", true);
   }
 
+  /** @inheritdoc */
+  get canChangeContent() {
+    return true;
+  }
+
   /** @inheritdoc */
   render() {
     if (this.div) {
@@ -579,8 +568,6 @@ class FreeTextEditor extends AnnotationEditor {
     this.overlayDiv.classList.add("overlay", "enabled");
     this.div.append(this.overlayDiv);
 
-    bindEvents(this, this.div, ["dblclick", "keydown"]);
-
     if (this._isCopy || this.annotationElementId) {
       // This editor was created in using copy (ctrl+c).
       const [parentWidth, parentHeight] = this.parentDimensions;

src/display/editor/stamp.js

@@ -71,9 +71,10 @@ class StampEditor extends AnnotationEditor {
 
   /** @inheritdoc */
   static paste(item, parent) {
-    parent.pasteEditor(AnnotationEditorType.STAMP, {
-      bitmapFile: item.getAsFile(),
-    });
+    parent.pasteEditor(
+      { mode: AnnotationEditorType.STAMP },
+      { bitmapFile: item.getAsFile() }
+    );
   }
 
   /** @inheritdoc */

src/display/editor/tools.js

@@ -1711,7 +1711,7 @@ class AnnotationEditorUIManager {
     }
 
     for (const editor of this.#allEditors.values()) {
-      if (editor.annotationElementId === editId) {
+      if (editor.annotationElementId === editId || editor.id === editId) {
         this.setSelected(editor);
         editor.enterInEditMode();
       } else {
@@ -1730,16 +1730,17 @@ class AnnotationEditorUIManager {
 
   /**
    * Update the toolbar if it's required to reflect the tool currently used.
+   * @param {Object} options
    * @param {number} mode
    * @returns {undefined}
    */
-  updateToolbar(mode) {
-    if (mode === this.#mode) {
+  updateToolbar(options) {
+    if (options.mode === this.#mode) {
       return;
     }
     this._eventBus.dispatch("switchannotationeditormode", {
       source: this,
-      mode,
+      ...options,
     });
   }
 

test/integration/ink_editor_spec.mjs

@@ -1229,3 +1229,65 @@ describe("The pen-drawn shape must maintain correct curvature regardless of the
     );
   });
 });
+
+describe("Should switch from an editor and mode to others by double clicking", () => {
+  let pages;
+
+  beforeEach(async () => {
+    pages = await loadAndWait("empty.pdf", ".annotationEditorLayer");
+  });
+
+  afterEach(async () => {
+    await closePages(pages);
+  });
+
+  it("must check that the editor an the mode are correct", async () => {
+    await Promise.all(
+      pages.map(async ([browserName, page]) => {
+        await switchToInk(page);
+
+        const editorLayerRect = await getRect(page, ".annotationEditorLayer");
+        const drawStartX = editorLayerRect.x + 100;
+        const drawStartY = editorLayerRect.y + 100;
+
+        const inkSelector = getEditorSelector(0);
+        const clickHandle = await waitForPointerUp(page);
+        await page.mouse.move(drawStartX, drawStartY);
+        await page.mouse.down();
+        await page.mouse.move(drawStartX + 50, drawStartY + 50);
+        await page.mouse.up();
+        await awaitPromise(clickHandle);
+        await commit(page);
+
+        await switchToEditor("FreeText", page);
+
+        const freeTextSelector = getEditorSelector(1);
+        const data = "Hello PDF.js World !!";
+        await page.mouse.click(
+          editorLayerRect.x + 200,
+          editorLayerRect.y + 200
+        );
+        await page.waitForSelector(freeTextSelector, { visible: true });
+        await page.type(`${freeTextSelector} .internal`, data);
+        await page.keyboard.press("Escape");
+        await page.waitForSelector(
+          ".freeTextEditor.selectedEditor .overlay.enabled"
+        );
+
+        await page.waitForSelector("#editorInkButton:not(.toggled)");
+        let modeChangedHandle = await waitForAnnotationModeChanged(page);
+        await selectEditor(page, inkSelector, 2);
+        await awaitPromise(modeChangedHandle);
+        await page.waitForSelector("#editorInkButton.toggled");
+        await waitForSelectedEditor(page, inkSelector);
+
+        await page.waitForSelector("#editorFreeTextButton:not(.toggled)");
+        modeChangedHandle = await waitForAnnotationModeChanged(page);
+        await selectEditor(page, freeTextSelector, 2);
+        await awaitPromise(modeChangedHandle);
+        await page.waitForSelector("#editorFreeTextButton.toggled");
+        await waitForSelectedEditor(page, freeTextSelector);
+      })
+    );
+  });
+});

web/annotation_editor_layer_builder.css

@@ -147,6 +147,11 @@
 
 .annotationEditorLayer.disabled {
   pointer-events: none;
+
+  &.highlightEditing
+    :is(.freeTextEditor, .inkEditor, .stampEditor, .signatureEditor) {
+    pointer-events: auto;
+  }
 }
 
 .annotationEditorLayer.freetextEditing {