Merge pull request #17481 from calixteman/editor_default_init

[Editor] Init the default highlight color before creating the first editor instance
mozilla · Jan 5, 2024 · 6c5e237 · 6c5e237
2 parents 130a0fe + 17e1519
commit 6c5e237
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 15 deletions.
diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js
@@ -110,7 +110,7 @@ class AnnotationEditorLayer {
     if (!AnnotationEditorLayer._initialized) {
       AnnotationEditorLayer._initialized = true;
       for (const editorType of editorTypes) {
-        editorType.initialize(l10n);
+        editorType.initialize(l10n, uiManager);
       }
     }
     uiManager.registerEditorTypes(editorTypes);

diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js
@@ -185,7 +185,7 @@ class AnnotationEditor {
    * Initialize the l10n stuff for this type of editor.
    * @param {Object} l10n
    */
-  static initialize(l10n, options = null) {
+  static initialize(l10n, _uiManager, options) {
     AnnotationEditor._l10nPromise ||= new Map(
       [
         "pdfjs-editor-alt-text-button-label",

diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js
@@ -144,8 +144,8 @@ class FreeTextEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  static initialize(l10n) {
-    AnnotationEditor.initialize(l10n, {
+  static initialize(l10n, uiManager) {
+    AnnotationEditor.initialize(l10n, uiManager, {
       strings: ["pdfjs-free-text-default-content"],
     });
     const style = getComputedStyle(document.documentElement);

diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js
@@ -59,8 +59,6 @@ class HighlightEditor extends AnnotationEditor {
 
   constructor(params) {
     super({ ...params, name: "highlightEditor" });
-    HighlightEditor._defaultColor ||=
-      this._uiManager.highlightColors?.values().next().value || "#fff066";
     this.color = params.color || HighlightEditor._defaultColor;
     this.#opacity = params.opacity || HighlightEditor._defaultOpacity;
     this.#boxes = params.boxes || null;
@@ -97,8 +95,10 @@ class HighlightEditor extends AnnotationEditor {
     ];
   }
 
-  static initialize(l10n) {
-    AnnotationEditor.initialize(l10n);
+  static initialize(l10n, uiManager) {
+    AnnotationEditor.initialize(l10n, uiManager);
+    HighlightEditor._defaultColor ||=
+      uiManager.highlightColors?.values().next().value || "#fff066";
   }
 
   static updateDefaultParams(type, value) {

diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js
@@ -84,8 +84,8 @@ class InkEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  static initialize(l10n) {
-    AnnotationEditor.initialize(l10n);
+  static initialize(l10n, uiManager) {
+    AnnotationEditor.initialize(l10n, uiManager);
   }
 
   /** @inheritdoc */

diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js
@@ -55,8 +55,8 @@ class StampEditor extends AnnotationEditor {
   }
 
   /** @inheritdoc */
-  static initialize(l10n) {
-    AnnotationEditor.initialize(l10n);
+  static initialize(l10n, uiManager) {
+    AnnotationEditor.initialize(l10n, uiManager);
   }
 
   static get supportedTypes() {

diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs
@@ -132,4 +132,61 @@ describe("Highlight Editor", () => {
       );
     });
   });
+
+  describe("The default color must have the correct value", () => {
+    let pages;
+
+    beforeAll(async () => {
+      pages = await loadAndWait(
+        "tracemonkey.pdf",
+        ".annotationEditorLayer",
+        null,
+        null,
+        { highlightEditorColors: "red=#AB0000" }
+      );
+    });
+
+    afterAll(async () => {
+      await closePages(pages);
+    });
+
+    it("must highlight with red color", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          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 x = rect.x + rect.width / 2;
+          const 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`
+          );
+
+          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");
+        })
+      );
+    });
+  });
 });
diff --git a/test/integration/test_utils.mjs b/test/integration/test_utils.mjs
@@ -16,7 +16,7 @@
 import os from "os";
 const isMac = os.platform() === "darwin";
 
-function loadAndWait(filename, selector, zoom, pageSetup) {
+function loadAndWait(filename, selector, zoom, pageSetup, options) {
   return Promise.all(
     global.integrationSessions.map(async session => {
       const page = await session.browser.newPage();
@@ -36,9 +36,16 @@ function loadAndWait(filename, selector, zoom, pageSetup) {
         });
       });
 
+      let app_options = "";
+      if (options) {
+        // Options must be handled in app.js::_parseHashParams.
+        for (const [key, value] of Object.entries(options)) {
+          app_options += `&${key}=${encodeURIComponent(value)}`;
+        }
+      }
       const url = `${
         global.integrationBaseUrl
-      }?file=/test/pdfs/${filename}#zoom=${zoom ?? "page-fit"}`;
+      }?file=/test/pdfs/${filename}#zoom=${zoom ?? "page-fit"}${app_options}`;
 
       await page.goto(url);
       if (pageSetup) {

diff --git a/web/app.js b/web/app.js
@@ -353,6 +353,16 @@ const PDFViewerApplication = {
     ) {
       AppOptions.set("locale", params.get("locale"));
     }
+
+    // Set some specific preferences for tests.
+    if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
+      if (params.has("highlighteditorcolors")) {
+        AppOptions.set(
+          "highlightEditorColors",
+          params.get("highlighteditorcolors")
+        );
+      }
+    }
   },
 
   /**

diff --git a/web/app_options.js b/web/app_options.js
@@ -203,7 +203,7 @@ const defaultOptions = {
   },
   pdfBugEnabled: {
     /** @type {boolean} */
-    value: typeof PDFJSDev === "undefined",
+    value: typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING"),
     kind: OptionKind.VIEWER + OptionKind.PREFERENCE,
   },
   printResolution: {