[GENERIC viewer] Re-initialize the viewer-toolbar ColorPicker for e…

…ach PDF document Steps to reproduce this in `master`: 1. Open https://mozilla.github.io/pdf.js/web/viewer.html 2. Use the "Open"-button (in the secondaryToolbar), or drag-and-drop, to load another PDF document. 3. Enable the highlight-editor. 4. Try to pick a new colour. Note how it's no longer possible to change the default highlight-colour. The reason for this is that we're only initializing the viewer-toolbar `ColorPicker` *once*, which doesn't work since every PDF document gets its own `AnnotationEditorUIManager`-instance. To address this we simply need to re-initialize the viewer-toolbar `ColorPicker`, and note that this patch won't affect the Firefox PDF Viewer.
mozilla · Jan 23, 2025 · dd31967 · dd31967
1 parent 2132552
commit dd31967
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 19 deletions.
diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js
@@ -907,6 +907,8 @@ class AnnotationEditorUIManager {
     this.#altTextManager?.destroy();
     this.#highlightToolbar?.hide();
     this.#highlightToolbar = null;
+    this.#mainHighlightColorPicker?.destroy();
+    this.#mainHighlightColorPicker = null;
     if (this.#focusMainContainerTimeoutId) {
       clearTimeout(this.#focusMainContainerTimeoutId);
       this.#focusMainContainerTimeoutId = null;

diff --git a/web/annotation_editor_params.js b/web/annotation_editor_params.js
@@ -52,8 +52,10 @@ class AnnotationEditorParams {
     editorFreeHighlightThickness,
     editorHighlightShowAll,
   }) {
+    const { eventBus } = this;
+
     const dispatchEvent = (typeStr, value) => {
-      this.eventBus.dispatch("switchannotationeditorparams", {
+      eventBus.dispatch("switchannotationeditorparams", {
         source: this,
         type: AnnotationEditorParamsType[typeStr],
         value,
@@ -93,7 +95,7 @@ class AnnotationEditorParams {
       dispatchEvent("HIGHLIGHT_SHOW_ALL", !checked);
     });
 
-    this.eventBus._on("annotationeditorparamschanged", evt => {
+    eventBus._on("annotationeditorparamschanged", evt => {
       for (const [type, value] of evt.details) {
         switch (type) {
           case AnnotationEditorParamsType.FREETEXT_SIZE:
@@ -111,6 +113,12 @@ class AnnotationEditorParams {
           case AnnotationEditorParamsType.INK_OPACITY:
             editorInkOpacity.value = value;
             break;
+          case AnnotationEditorParamsType.HIGHLIGHT_DEFAULT_COLOR:
+            eventBus.dispatch("mainhighlightcolorpickerupdatecolor", {
+              source: this,
+              value,
+            });
+            break;
           case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
             editorFreeHighlightThickness.value = value;
             break;

diff --git a/web/toolbar.js b/web/toolbar.js
@@ -44,6 +44,8 @@ import {
  */
 
 class Toolbar {
+  #colorPicker = null;
+
   #opts;
 
   /**
@@ -139,12 +141,6 @@ class Toolbar {
     document.documentElement.setAttribute("data-toolbar-density", name);
   }
 
-  #setAnnotationEditorUIManager(uiManager, parentContainer) {
-    const colorPicker = new ColorPicker({ uiManager });
-    uiManager.setMainHighlightColorPicker(colorPicker);
-    parentContainer.append(colorPicker.renderMainDropdown());
-  }
-
   setPageNumber(pageNumber, pageLabel) {
     this.pageNumber = pageNumber;
     this.pageLabel = pageLabel;
@@ -164,6 +160,7 @@ class Toolbar {
   }
 
   reset() {
+    this.#colorPicker = null;
     this.pageNumber = 0;
     this.pageLabel = null;
     this.hasPageLabels = false;
@@ -255,17 +252,15 @@ class Toolbar {
     eventBus._on("toolbardensity", this.#updateToolbarDensity.bind(this));
 
     if (editorHighlightColorPicker) {
-      eventBus._on(
-        "annotationeditoruimanager",
-        ({ uiManager }) => {
-          this.#setAnnotationEditorUIManager(
-            uiManager,
-            editorHighlightColorPicker
-          );
-        },
-        // Once the color picker has been added, we don't want to add it again.
-        { once: true }
-      );
+      eventBus._on("annotationeditoruimanager", ({ uiManager }) => {
+        const cp = (this.#colorPicker = new ColorPicker({ uiManager }));
+        uiManager.setMainHighlightColorPicker(cp);
+        editorHighlightColorPicker.append(cp.renderMainDropdown());
+      });
+
+      eventBus._on("mainhighlightcolorpickerupdatecolor", ({ value }) => {
+        this.#colorPicker?.updateColor(value);
+      });
     }
   }