From 342b5e20b42096d4dd12d06e283c392a5873e6da Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 23 Jan 2025 16:33:14 +0100 Subject: [PATCH] [GENERIC viewer] Re-initialize the viewer-toolbar `ColorPicker` for each 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. --- src/display/editor/tools.js | 2 ++ web/annotation_editor_params.js | 14 +++++++++++--- web/toolbar.js | 29 ++++++++++++----------------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index af2f49cd3..c0349c688 100644 --- 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 index a69206214..951ce86a5 100644 --- 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, @@ -75,7 +77,7 @@ class AnnotationEditorParams { dispatchEvent("INK_OPACITY", this.valueAsNumber); }); editorStampAddImage.addEventListener("click", () => { - this.eventBus.dispatch("reporttelemetry", { + eventBus.dispatch("reporttelemetry", { source: this, details: { type: "editing", @@ -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 index fa0d589ac..5d9725e8c 100644 --- 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); + }); } }