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 57edde4f7229f..973ae4636984a 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 0390228acc48f..c6a1a24604406 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 adc0e3d50744d..a55aee8ea5616 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 2845afee2d3ff..7a378d52c30d8 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 08cac70ea8b26..dea3e7fb4a336 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 c997346398efb..30fa96cd0a136 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 2d18b1c1bd3ff..0cd9b4aba84b6 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 521db3c436c11..f99b9e4e2555e 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 36ac828142d0b..2e8c7f4aa70e3 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 29968c5827e95..aa529e19eb0b6 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: {