Merge pull request #17498 from calixteman/editor_dont_throw_highlight…

…_color [Editor] Don't throw when changing the color of an invisible highlight
mozilla · Jan 11, 2024 · 35daccf · 35daccf
2 parents 1287535 + b4a7007
commit 35daccf
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 26 deletions.
diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js
@@ -151,12 +151,12 @@ class HighlightEditor extends AnnotationEditor {
     this.addCommands({
       cmd: () => {
         this.color = color;
-        this.parent.drawLayer.changeColor(this.#id, color);
+        this.parent?.drawLayer.changeColor(this.#id, color);
         this.#colorPicker?.updateColor(color);
       },
       undo: () => {
         this.color = savedColor;
-        this.parent.drawLayer.changeColor(this.#id, savedColor);
+        this.parent?.drawLayer.changeColor(this.#id, savedColor);
         this.#colorPicker?.updateColor(savedColor);
       },
       mustExec: true,

diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs
@@ -16,10 +16,36 @@
 import {
   closePages,
   getEditorSelector,
+  kbSelectAll,
   loadAndWait,
   scrollIntoView,
 } from "./test_utils.mjs";
 
+const selectAll = async page => {
+  await kbSelectAll(page);
+  await page.waitForFunction(
+    () => !document.querySelector(".highlightEditor:not(.selectedEditor)")
+  );
+};
+
+const getSpanRectFromText = (page, pageNumber, text) => {
+  return page.evaluate(
+    (number, content) => {
+      for (const el of document.querySelectorAll(
+        `.page[data-page-number="${number}"] > .textLayer > span`
+      )) {
+        if (el.textContent === content) {
+          const { x, y, width, height } = el.getBoundingClientRect();
+          return { x, y, width, height };
+        }
+      }
+      return null;
+    },
+    pageNumber,
+    text
+  );
+};
+
 describe("Highlight Editor", () => {
   describe("Editor must be removed without exception", () => {
     let pages;
@@ -38,18 +64,7 @@ describe("Highlight Editor", () => {
           await page.click("#editorHighlight");
           await page.waitForSelector(".annotationEditorLayer.highlightEditing");
 
-          const rect = await page.evaluate(() => {
-            for (const el of document.querySelectorAll(
-              `.page[data-page-number="1"] > .textLayer > span`
-            )) {
-              if (el.textContent === "Abstract") {
-                const { x, y, width, height } = el.getBoundingClientRect();
-                return { x, y, width, height };
-              }
-            }
-            return null;
-          });
-
+          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 });
@@ -94,18 +109,7 @@ describe("Highlight Editor", () => {
           await page.click("#editorHighlight");
           await page.waitForSelector(".annotationEditorLayer.highlightEditing");
 
-          const rect = await page.evaluate(() => {
-            for (const el of document.querySelectorAll(
-              `.page[data-page-number="1"] > .textLayer > span`
-            )) {
-              if (el.textContent === "Abstract") {
-                const { x, y, width, height } = el.getBoundingClientRect();
-                return { x, y, width, height };
-              }
-            }
-            return null;
-          });
-
+          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 });
@@ -189,4 +193,93 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("Invisible editor must change its color", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "tracemonkey.pdf",
+        ".annotationEditorLayer",
+        null,
+        null,
+        { highlightEditorColors: "yellow=#FFFF98,green=#53FFBC" }
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must scroll and change the color without exceptions", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await page.click("#editorHighlight");
+          await page.waitForSelector(".annotationEditorLayer.highlightEditing");
+
+          let rect = await getSpanRectFromText(page, 1, "Abstract");
+          let x = rect.x + rect.width / 2;
+          let y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 2 });
+
+          await page.waitForSelector(`${getEditorSelector(0)}`);
+          await page.waitForSelector(
+            `.page[data-page-number = "1"] svg.highlightOutline.selected`
+          );
+
+          for (const pageNumber of Array.from(
+            new Array(13).keys(),
+            n => n + 2
+          )) {
+            await scrollIntoView(
+              page,
+              `.page[data-page-number = "${pageNumber}"]`
+            );
+          }
+          await page.waitForSelector(
+            `.page[data-page-number = "14"] .textLayer .endOfContent`
+          );
+
+          rect = await getSpanRectFromText(page, 14, "References");
+          x = rect.x + rect.width / 2;
+          y = rect.y + rect.height / 2;
+          await page.mouse.click(x, y, { count: 2 });
+          await page.waitForSelector(`${getEditorSelector(1)}`);
+          await page.waitForSelector(
+            `.page[data-page-number = "14"] svg.highlightOutline.selected`
+          );
+          await selectAll(page);
+          await page.waitForSelector(
+            `${getEditorSelector(1)} .editToolbar button.colorPicker`
+          );
+          await page.click(
+            `${getEditorSelector(1)} .editToolbar button.colorPicker`
+          );
+          await page.waitForSelector(
+            `${getEditorSelector(1)} .editToolbar button[title = "Green"]`
+          );
+          await page.click(
+            `${getEditorSelector(1)} .editToolbar button[title = "Green"]`
+          );
+          await page.waitForSelector(
+            `.page[data-page-number = "14"] svg.highlight[fill = "#53FFBC"]`
+          );
+
+          for (const pageNumber of Array.from(
+            new Array(13).keys(),
+            n => 13 - n
+          )) {
+            await scrollIntoView(
+              page,
+              `.page[data-page-number = "${pageNumber}"]`
+            );
+          }
+
+          await page.waitForSelector(
+            `.page[data-page-number = "1"] svg.highlight[fill = "#53FFBC"]`
+          );
+        })
+      );
+    });
+  });
 });