Merge pull request #18440 from calixteman/fix_add_highlight

[Editor] When in non-editing mode, add a new editor only once the editing mode has switched

src/display/editor/tools.js

@@ -916,6 +916,18 @@ class AnnotationEditorUIManager {
     this.#altTextManager?.editAltText(this, editor);
   }
 
+  switchToMode(mode, callback) {
+    // Switching to a mode can be asynchronous.
+    this._eventBus.on("annotationeditormodechanged", callback, {
+      once: true,
+      signal: this._signal,
+    });
+    this._eventBus.dispatch("showannotationeditorui", {
+      source: this,
+      mode,
+    });
+  }
+
   onPageChanging({ pageNumber }) {
     this.#currentPageIndex = pageNumber - 1;
   }
@@ -1002,16 +1014,11 @@ class AnnotationEditorUIManager {
       return;
     }
     selection.empty();
-    if (this.#mode === AnnotationEditorType.NONE) {
-      this._eventBus.dispatch("showannotationeditorui", {
-        source: this,
-        mode: AnnotationEditorType.HIGHLIGHT,
-      });
-      this.showAllEditors("highlight", true, /* updateButton = */ true);
-    }
+
     const layer = this.#getLayerForTextLayer(textLayer);
-    if (layer) {
-      layer.createAndAddNewEditor({ x: 0, y: 0 }, false, {
+    const isNoneMode = this.#mode === AnnotationEditorType.NONE;
+    const callback = () => {
+      layer?.createAndAddNewEditor({ x: 0, y: 0 }, false, {
         methodOfCreation,
         boxes,
         anchorNode,
@@ -1020,7 +1027,15 @@ class AnnotationEditorUIManager {
         focusOffset,
         text,
       });
+      if (isNoneMode) {
+        this.showAllEditors("highlight", true, /* updateButton = */ true);
+      }
+    };
+    if (isNoneMode) {
+      this.switchToMode(AnnotationEditorType.HIGHLIGHT, callback);
+      return;
     }
+    callback();
   }
 
   #displayHighlightToolbar() {

test/integration/highlight_editor_spec.mjs

@@ -1890,4 +1890,46 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Highlight with the floating button in a pdf containing a FreeText", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "file_pdfjs_test.pdf",
+        ".annotationEditorLayer",
+        null,
+        null,
+        { highlightEditorColors: "red=#AB0000" }
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that the highlight is created", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          const rect = await getSpanRectFromText(page, 1, "In production");
+          const x = rect.x + rect.width / 2;
+          const y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 3, delay: 100 });
+
+          await page.waitForSelector(".textLayer .highlightButton");
+          await page.click(".textLayer .highlightButton");
+
+          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");
+        })
+      );
+    });
+  });
 });

test/pdfs/.gitignore

@@ -657,3 +657,4 @@
 !issue18305.pdf
 !issue18360.pdf
 !issue18099_reduced.pdf
+!file_pdfjs_test.pdf

web/pdf_viewer.js

@@ -2311,11 +2311,11 @@ class PDFViewer {
     const updater = () => {
       this.#cleanupSwitchAnnotationEditorMode();
       this.#annotationEditorMode = mode;
+      this.#annotationEditorUIManager.updateMode(mode, editId, isFromKeyboard);
       eventBus.dispatch("annotationeditormodechanged", {
         source: this,
         mode,
       });
-      this.#annotationEditorUIManager.updateMode(mode, editId, isFromKeyboard);
     };
 
     if (