From 17e1519410aedf1cd337a5e99d234635643810af Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Fri, 5 Jan 2024 10:10:01 +0100 Subject: [PATCH] [Editor] Init the default highlight color before creating the first editor instance We want to be able to draw an highlight with the default color but without having an instance of the HighlightEditor. --- src/display/editor/annotation_editor_layer.js | 2 +- src/display/editor/editor.js | 2 +- src/display/editor/freetext.js | 4 +- src/display/editor/highlight.js | 8 +-- src/display/editor/ink.js | 4 +- src/display/editor/stamp.js | 4 +- test/integration/highlight_editor_spec.mjs | 57 +++++++++++++++++++ test/integration/test_utils.mjs | 11 +++- web/app.js | 10 ++++ web/app_options.js | 2 +- 10 files changed, 89 insertions(+), 15 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 57edde4f7..973ae4636 100644 --- 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 index 0390228ac..c6a1a2460 100644 --- 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 index adc0e3d50..a55aee8ea 100644 --- 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 index 2845afee2..7a378d52c 100644 --- 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 index 08cac70ea..dea3e7fb4 100644 --- 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 index c99734639..30fa96cd0 100644 --- 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 index 2d18b1c1b..0cd9b4aba 100644 --- 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 index 521db3c43..f99b9e4e2 100644 --- 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 index 36ac82814..2e8c7f4aa 100644 --- 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 index 29968c582..aa529e19e 100644 --- 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: {