Merge pull request #19371 from Snuffleupagus/issue-19369

[Editor] Ensure that `highlightSelection` waits until we've fully updated the editing-mode (issue 19369)
mozilla · Jan 23, 2025 · 2132552 · 2132552
2 parents 877f698 + 3b4758a
commit 2132552
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 17 deletions.
diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs
@@ -1481,23 +1481,33 @@ describe("Highlight Editor", () => {
     it("must check that clicking on the highlight floating button triggers an highlight", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {
-          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, delay: 100 });
+          async function floatingHighlight(text, editorId) {
+            const rect = await getSpanRectFromText(page, 1, text);
+            const x = rect.x + rect.width / 2;
+            const y = rect.y + rect.height / 2;
+            await page.mouse.click(x, y, { count: 2, delay: 100 });
+
+            await page.waitForSelector(".textLayer .highlightButton");
+            await page.click(".textLayer .highlightButton");
+
+            await page.waitForSelector(getEditorSelector(editorId));
+            const usedColor = await page.evaluate(() => {
+              const highlight = document.querySelector(
+                `.page[data-page-number = "1"] .canvasWrapper > svg.highlight`
+              );
+              return highlight.getAttribute("fill");
+            });
 
-          await page.waitForSelector(".textLayer .highlightButton");
-          await page.click(".textLayer .highlightButton");
+            expect(usedColor)
+              .withContext(`In ${browserName}`)
+              .toEqual("#AB0000");
+          }
 
-          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");
-          });
+          await floatingHighlight("Abstract", 0);
 
-          expect(usedColor).withContext(`In ${browserName}`).toEqual("#AB0000");
+          // Disable editing mode, and highlight another string (issue 19369).
+          await switchToHighlight(page, /* disable */ true);
+          await floatingHighlight("Introduction", 1);
         })
       );
     });

diff --git a/web/pdf_viewer.js b/web/pdf_viewer.js
@@ -2339,11 +2339,22 @@ class PDFViewer {
       this.#mlManager?.loadModel("altText");
     }
 
-    const { eventBus } = this;
-    const updater = () => {
+    const { eventBus, pdfDocument } = this;
+    const updater = async () => {
       this.#cleanupSwitchAnnotationEditorMode();
       this.#annotationEditorMode = mode;
-      this.#annotationEditorUIManager.updateMode(mode, editId, isFromKeyboard);
+      await this.#annotationEditorUIManager.updateMode(
+        mode,
+        editId,
+        isFromKeyboard
+      );
+      if (
+        mode !== this.#annotationEditorMode ||
+        pdfDocument !== this.pdfDocument
+      ) {
+        // Since `updateMode` is async, the active mode could have changed.
+        return;
+      }
       eventBus.dispatch("annotationeditormodechanged", {
         source: this,
         mode,