Merge pull request #19215 from calixteman/issue18911

[Editor] Avoid to focus an existing editor when enabling the layer

src/display/editor/annotation_editor_layer.js

@@ -74,6 +74,8 @@ class AnnotationEditorLayer {
 
   #isDisabling = false;
 
+  #isEnabling = false;
+
   #drawingAC = null;
 
   #focusedElement = null;
@@ -229,6 +231,7 @@ class AnnotationEditorLayer {
    * editor creation.
    */
   async enable() {
+    this.#isEnabling = true;
     this.div.tabIndex = 0;
     this.togglePointerEvents(true);
     const annotationElementIds = new Set();
@@ -242,6 +245,7 @@ class AnnotationEditorLayer {
     }
 
     if (!this.#annotationLayer) {
+      this.#isEnabling = false;
       return;
     }
 
@@ -262,6 +266,7 @@ class AnnotationEditorLayer {
       this.addOrRebuild(editor);
       editor.enableEditing();
     }
+    this.#isEnabling = false;
   }
 
   /**
@@ -508,7 +513,7 @@ class AnnotationEditorLayer {
 
     // The editor will be correctly moved into the DOM (see fixAndSetPosition).
     editor.fixAndSetPosition();
-    editor.onceAdded();
+    editor.onceAdded(/* focus = */ !this.#isEnabling);
     this.#uiManager.addToAnnotationStorage(editor);
     editor._reportTelemetry(editor.telemetryInitialData);
   }

src/display/editor/draw.js

@@ -345,7 +345,7 @@ class DrawingEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  onceAdded() {
+  onceAdded(focus) {
     if (!this.annotationElementId) {
       this.parent.addUndoableEditor(this);
     }
@@ -354,7 +354,7 @@ class DrawingEditor extends AnnotationEditor {
       this.#mustBeCommitted = false;
       this.commit();
       this.parent.setSelected(this);
-      if (this.isOnScreen) {
+      if (focus && this.isOnScreen) {
         this.div.focus();
       }
     }

src/display/editor/editor.js

@@ -1367,8 +1367,9 @@ class AnnotationEditor {
 
   /**
    * Executed once this editor has been rendered.
+   * @param {boolean} focus - true if the editor should be focused.
    */
-  onceAdded() {}
+  onceAdded(focus) {}
 
   /**
    * Check if the editor contains something.

src/display/editor/freetext.js

@@ -363,13 +363,15 @@ class FreeTextEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  onceAdded() {
+  onceAdded(focus) {
     if (this.width) {
       // The editor was created in using ctrl+c.
       return;
     }
     this.enableEditMode();
+    if (focus) {
       this.editorDiv.focus();
+    }
     if (this._initialOptions?.isCentered) {
       this.center();
     }

src/display/editor/highlight.js

@@ -439,12 +439,14 @@ class HighlightEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  onceAdded() {
+  onceAdded(focus) {
     if (!this.annotationElementId) {
       this.parent.addUndoableEditor(this);
     }
+    if (focus) {
       this.div.focus();
     }
+  }
 
   /** @inheritdoc */
   remove() {

src/display/editor/stamp.js

@@ -338,10 +338,12 @@ class StampEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  onceAdded() {
+  onceAdded(focus) {
     this._isDraggable = true;
+    if (focus) {
       this.div.focus();
     }
+  }
 
   /** @inheritdoc */
   isEmpty() {

test/integration/highlight_editor_spec.mjs

@@ -37,6 +37,7 @@ import {
   waitForAnnotationModeChanged,
   waitForSelectedEditor,
   waitForSerialized,
+  waitForTimeout,
 } from "./test_utils.mjs";
 import { fileURLToPath } from "url";
 import fs from "fs";
@@ -2613,4 +2614,59 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Highlight mustn't trigger a scroll when edited", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait("issue18911.pdf", ".annotationEditorLayer");
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that there is no scroll because of focus", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToHighlight(page);
+
+          const page4Selector = ".page[data-page-number = '4']";
+          const page5Selector = ".page[data-page-number = '5']";
+          await scrollIntoView(page, page4Selector);
+          await page.waitForSelector(`${page5Selector} .annotationEditorLayer`);
+
+          // When moving to page 4, the highlight editor mustn't be focused (it
+          // was causing a scroll to page 5).
+          // So here we're waiting a bit and checking that the page is still 4.
+          // eslint-disable-next-line no-restricted-syntax
+          await waitForTimeout(100);
+
+          // Get the length of the intersection between two ranges.
+          const inter = ([a, b], [c, d]) =>
+            d < a || b < c ? 0 : Math.min(b, d) - Math.max(a, c);
+
+          const page4Rect = await getRect(page, page4Selector);
+          const page5Rect = await getRect(page, page5Selector);
+          const viewportRect = await getRect(page, "#viewerContainer");
+          const viewportRange = [
+            viewportRect.y,
+            viewportRect.y + viewportRect.height,
+          ];
+
+          const interPage4 = inter(
+            [page4Rect.y, page4Rect.y + page4Rect.height],
+            viewportRange
+          );
+          const interPage5 = inter(
+            [page5Rect.y, page5Rect.y + page5Rect.height],
+            viewportRange
+          );
+          expect(interPage4)
+            .withContext(`In ${browserName}`)
+            .toBeGreaterThan(0.5 * interPage5);
+        })
+      );
+    });
+  });
 });

test/pdfs/.gitignore

@@ -689,3 +689,4 @@
 !inks.pdf
 !inks_basic.pdf
 !issue19182.pdf
+!issue18911.pdf