From 12066af578cd18030807f011dc1a41726a054319 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Tue, 30 Sep 2025 18:41:45 +0200 Subject: [PATCH] [Editor] Add a fake annotation (in the annotation layer) associated with an editor in order to be able to show the comment button (bug 1989420) --- src/display/annotation_layer.js | 144 ++++++++++++++---- src/display/editor/annotation_editor_layer.js | 82 +++++----- src/display/editor/editor.js | 28 +++- src/display/editor/tools.js | 4 + test/integration/comment_spec.mjs | 74 ++++++++- web/annotation_editor_layer_builder.css | 7 + web/annotation_layer_builder.js | 32 +--- web/pdf_page_view.js | 41 ++--- 8 files changed, 293 insertions(+), 119 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 126bb7131..97640abec 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -284,6 +284,24 @@ class AnnotationElement { ); } + set commentText(text) { + const { data } = this; + const popup = { deleted: !text, contents: text || "" }; + if (!this.annotationStorage.updateEditor(data.id, { popup })) { + this.annotationStorage.setValue(`${AnnotationEditorPrefix}${data.id}`, { + id: data.id, + annotationType: data.annotationType, + pageIndex: this.parent.page._pageIndex, + popup, + popupRef: data.popupRef, + modificationDate: new Date(), + }); + } + if (!text) { + this.removePopup(); + } + } + removePopup() { (this.#popupElement?.popup || this.popup)?.remove(); this.#popupElement = this.popup = null; @@ -308,10 +326,8 @@ class AnnotationElement { let popup = this.#popupElement?.popup || this.popup; if (!popup && newPopup?.text) { - if (!this.parent._commentManager) { - this._createPopup(newPopup); - popup = this.#popupElement.popup; - } + this._createPopup(newPopup); + popup = this.#popupElement.popup; } if (!popup) { return; @@ -882,6 +898,56 @@ class AnnotationElement { } } +class EditorAnnotationElement extends AnnotationElement { + constructor(parameters) { + super(parameters, { isRenderable: true, ignoreBorder: true }); + this.editor = parameters.editor; + } + + render() { + this.container.className = "editorAnnotation"; + return this.container; + } + + createOrUpdatePopup() { + const { editor } = this; + if (!editor.hasComment) { + return; + } + this._createPopup(editor.comment); + this.extraPopupElement.popup.renderCommentButton(); + } + + get hasCommentButton() { + return this.enableComment && this.editor.hasComment; + } + + get commentButtonPosition() { + return this.editor.commentButtonPositionInPage; + } + + get commentText() { + return this.editor.comment.text; + } + + set commentText(text) { + this.editor.comment = text; + if (!text) { + this.removePopup(); + } + } + + get commentData() { + return this.editor.getData(); + } + + remove() { + this.container.remove(); + this.container = null; + this.removePopup(); + } +} + class LinkAnnotationElement extends AnnotationElement { constructor(parameters, options = null) { super(parameters, { @@ -2541,7 +2607,9 @@ class PopupElement { } #updateCommentButtonPosition() { - if (this.#firstElement.extraPopupElement) { + if (this.#firstElement.extraPopupElement && !this.#firstElement.editor) { + // If there's no editor associated with the annotation then the comment + // button position can't be changed. return; } this.renderCommentButton(); @@ -2596,30 +2664,10 @@ class PopupElement { } set comment(text) { - const element = this.#firstElement; - const { data } = element; if (text === this.comment) { return; } - const popup = { deleted: !text, contents: text || "" }; - if (!element.annotationStorage.updateEditor(data.id, { popup })) { - element.annotationStorage.setValue( - `${AnnotationEditorPrefix}${data.id}`, - { - id: data.id, - annotationType: data.annotationType, - pageIndex: element.parent.page._pageIndex, - popup, - popupRef: data.popupRef, - modificationDate: new Date(), - } - ); - } - - this.#commentText = text; - if (!text) { - element.removePopup(); - } + this.#firstElement.commentText = this.#commentText = text; } get parentBoundingClientRect() { @@ -3681,10 +3729,14 @@ class AnnotationLayer { #annotationCanvasMap = null; + #annotationStorage = null; + #editableAnnotations = new Map(); #structTreeLayer = null; + #linkService = null; + constructor({ div, accessibilityManager, @@ -3694,11 +3746,15 @@ class AnnotationLayer { viewport, structTreeLayer, commentManager, + linkService, + annotationStorage, }) { this.div = div; this.#accessibilityManager = accessibilityManager; this.#annotationCanvasMap = annotationCanvasMap; this.#structTreeLayer = structTreeLayer || null; + this.#linkService = linkService || null; + this.#annotationStorage = annotationStorage || new AnnotationStorage(); this.page = page; this.viewport = viewport; this.zIndex = 0; @@ -3762,12 +3818,12 @@ class AnnotationLayer { const elementParams = { data: null, layer, - linkService: params.linkService, + linkService: this.#linkService, downloadManager: params.downloadManager, imageResourcesPath: params.imageResourcesPath || "", renderForms: params.renderForms !== false, svgFactory: new DOMSVGFactory(), - annotationStorage: params.annotationStorage || new AnnotationStorage(), + annotationStorage: this.#annotationStorage, enableComment: params.enableComment === true, enableScripting: params.enableScripting === true, hasJSActions: params.hasJSActions, @@ -3832,11 +3888,11 @@ class AnnotationLayer { * @param {IPDFLinkService} linkService * @memberof AnnotationLayer */ - async addLinkAnnotations(annotations, linkService) { + async addLinkAnnotations(annotations) { const elementParams = { data: null, layer: this.div, - linkService, + linkService: this.#linkService, svgFactory: new DOMSVGFactory(), parent: this, }; @@ -3919,6 +3975,34 @@ class AnnotationLayer { return this.#editableAnnotations.get(id); } + addFakeAnnotation(editor) { + const { div } = this; + const { id, rotation } = editor; + const element = new EditorAnnotationElement({ + data: { + id, + rect: editor.getPDFRect(), + rotation, + }, + editor, + layer: div, + parent: this, + enableComment: !!this._commentManager, + linkService: this.#linkService, + annotationStorage: this.#annotationStorage, + }); + const htmlElement = element.render(); + div.append(htmlElement); + this.#accessibilityManager?.moveElementInDOM( + div, + htmlElement, + htmlElement, + /* isRemovable = */ false + ); + element.createOrUpdatePopup(); + return element; + } + /** * @private */ diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index 755561e21..3f48c099a 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -170,6 +170,7 @@ class AnnotationEditorLayer { this.#cleanup(); switch (mode) { case AnnotationEditorType.NONE: + this.div.classList.toggle("nonEditing", true); this.disableTextSelection(); this.togglePointerEvents(false); this.toggleAnnotationLayerPointerEvents(true); @@ -193,6 +194,7 @@ class AnnotationEditorLayer { this.toggleAnnotationLayerPointerEvents(false); const { classList } = this.div; + classList.toggle("nonEditing", false); if (mode === AnnotationEditorType.POPUP) { classList.toggle("commentEditing", true); } else { @@ -257,6 +259,7 @@ class AnnotationEditorLayer { this.#isEnabling = true; this.div.tabIndex = 0; this.togglePointerEvents(true); + this.div.classList.toggle("nonEditing", false); this.#textLayerDblClickAC?.abort(); this.#textLayerDblClickAC = null; const annotationElementIds = new Set(); @@ -269,27 +272,24 @@ class AnnotationEditorLayer { } } - if (!this.#annotationLayer) { - this.#isEnabling = false; - return; - } - - const editables = this.#annotationLayer.getEditableAnnotations(); - for (const editable of editables) { - // The element must be hidden whatever its state is. - editable.hide(); - if (this.#uiManager.isDeletedAnnotationElement(editable.data.id)) { - continue; + const annotationLayer = this.#annotationLayer; + if (annotationLayer) { + for (const editable of annotationLayer.getEditableAnnotations()) { + // The element must be hidden whatever its state is. + editable.hide(); + if (this.#uiManager.isDeletedAnnotationElement(editable.data.id)) { + continue; + } + if (annotationElementIds.has(editable.data.id)) { + continue; + } + const editor = await this.deserialize(editable); + if (!editor) { + continue; + } + this.addOrRebuild(editor); + editor.enableEditing(); } - if (annotationElementIds.has(editable.data.id)) { - continue; - } - const editor = await this.deserialize(editable); - if (!editor) { - continue; - } - this.addOrRebuild(editor); - editor.enableEditing(); } this.#isEnabling = false; this.#uiManager._eventBus.dispatch("editorsrendered", { @@ -305,6 +305,7 @@ class AnnotationEditorLayer { this.#isDisabling = true; this.div.tabIndex = -1; this.togglePointerEvents(false); + this.div.classList.toggle("nonEditing", true); if (this.#textLayer && !this.#textLayerDblClickAC) { this.#textLayerDblClickAC = new AbortController(); const signal = this.#uiManager.combinedSignal(this.#textLayerDblClickAC); @@ -351,26 +352,29 @@ class AnnotationEditorLayer { { signal, capture: true } ); } - const changedAnnotations = new Map(); - const resetAnnotations = new Map(); - for (const editor of this.#allEditorsIterator) { - editor.disableEditing(); - if (!editor.annotationElementId) { - continue; - } - if (editor.serialize() !== null) { - changedAnnotations.set(editor.annotationElementId, editor); - continue; - } else { - resetAnnotations.set(editor.annotationElementId, editor); - } - this.getEditableAnnotation(editor.annotationElementId)?.show(); - editor.remove(); - } - if (this.#annotationLayer) { + const annotationLayer = this.#annotationLayer; + if (annotationLayer) { + const changedAnnotations = new Map(); + const resetAnnotations = new Map(); + for (const editor of this.#allEditorsIterator) { + editor.disableEditing(); + if (!editor.annotationElementId) { + editor.updateFakeAnnotationElement(annotationLayer); + continue; + } + if (editor.serialize() !== null) { + changedAnnotations.set(editor.annotationElementId, editor); + continue; + } else { + resetAnnotations.set(editor.annotationElementId, editor); + } + this.getEditableAnnotation(editor.annotationElementId)?.show(); + editor.remove(); + } + // Show the annotations that were hidden in enable(). - const editables = this.#annotationLayer.getEditableAnnotations(); + const editables = annotationLayer.getEditableAnnotations(); for (const editable of editables) { const { id } = editable.data; if (this.#uiManager.isDeletedAnnotationElement(id)) { @@ -725,7 +729,7 @@ class AnnotationEditorLayer { /** * Create a new editor * @param {Object} data - * @returns {AnnotationEditor | null} + * @returns {Promise} */ async deserialize(data) { return ( diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index caffc59c2..22a2f54d1 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -69,6 +69,8 @@ class AnnotationEditor { #savedDimensions = null; + #fakeAnnotation = null; + #focusAC = null; #focusedResizerName = ""; @@ -382,6 +384,10 @@ class AnnotationEditor { } else { // The editor is being removed from the DOM, so we need to stop resizing. this.#stopResizing(); + + // Remove the fake annotation in the annotation layer. + this.#fakeAnnotation?.remove(); + this.#fakeAnnotation = null; } this.parent = parent; } @@ -1172,7 +1178,9 @@ class AnnotationEditor { addStandaloneCommentButton() { if (this.#commentStandaloneButton) { - this.#commentStandaloneButton.classList.remove("hidden"); + if (this._uiManager.isEditingMode()) { + this.#commentStandaloneButton.classList.remove("hidden"); + } return; } if (!this.hasComment) { @@ -2338,6 +2346,24 @@ class AnnotationEditor { this.#disabled = true; } + updateFakeAnnotationElement(annotationLayer) { + if (!this.#fakeAnnotation && !this.deleted) { + this.#fakeAnnotation = annotationLayer.addFakeAnnotation(this); + return; + } + if (this.deleted) { + this.#fakeAnnotation.remove(); + this.#fakeAnnotation = null; + return; + } + if (this.hasEditedComment || this._hasBeenMoved || this._hasBeenResized) { + this.#fakeAnnotation.updateEdited({ + rect: this.getPDFRect(), + popup: this.comment, + }); + } + } + /** * Render an annotation in the annotation layer. * @param {Object} annotation diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index e12a2776f..5d5934611 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -2668,6 +2668,10 @@ class AnnotationEditorUIManager { return this.#mode; } + isEditingMode() { + return this.#mode !== AnnotationEditorType.NONE; + } + get imageManager() { return shadow(this, "imageManager", new ImageManager()); } diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index ac3d55098..a28bc5984 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -41,6 +41,21 @@ const highlightSpan = async (page, pageIndex, text) => { await page.waitForSelector(getEditorSelector(0)); }; +const editComment = async (page, editorSelector, comment) => { + const commentButtonSelector = `${editorSelector} button.comment`; + await waitAndClick(page, commentButtonSelector); + + const textInputSelector = "#commentManagerTextInput"; + await page.waitForSelector(textInputSelector, { + visible: true, + }); + await page.type(textInputSelector, comment); + await waitAndClick(page, "#commentManagerSaveButton"); + await page.waitForSelector("#commentManagerDialog", { + visible: false, + }); +}; + describe("Comment", () => { describe("Comment edit dialog must be visible in ltr", () => { let pages; @@ -88,10 +103,10 @@ describe("Comment", () => { })); expect(dialogRect.x + dialogRect.width) .withContext(`In ${browserName}`) - .toBeLessThanOrEqual(viewport.width); + .toBeLessThanOrEqual(viewport.width + 1); expect(dialogRect.y + dialogRect.height) .withContext(`In ${browserName}`) - .toBeLessThanOrEqual(viewport.height); + .toBeLessThanOrEqual(viewport.height + 1); }) ); }); @@ -134,14 +149,14 @@ describe("Comment", () => { }); const dialogRect = await getRect(page, "#commentManagerDialog"); const viewport = await page.evaluate(() => ({ - height: document.documentElement.clientHeight, + height: window.innerHeight, })); expect(dialogRect.x + dialogRect.width) .withContext(`In ${browserName}`) - .toBeGreaterThanOrEqual(0); + .toBeGreaterThanOrEqual(-1); expect(dialogRect.y + dialogRect.height) .withContext(`In ${browserName}`) - .toBeLessThanOrEqual(viewport.height); + .toBeLessThanOrEqual(viewport.height + 1); }) ); }); @@ -309,6 +324,55 @@ describe("Comment", () => { }) ); }); + + it("must check that the comment button is added in the annotation layer", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + const rect = await getSpanRectFromText(page, 1, "Abstract"); + const x = rect.x + rect.width / 2; + const y = rect.y + rect.height / 2; + await page.mouse.click(x, y, { count: 2, delay: 100 }); + await page.waitForSelector(getEditorSelector(0)); + + const comment = "Hello world!"; + await editComment(page, getEditorSelector(0), comment); + await page.hover("#editorHighlightButton"); + let buttonSelector = + ".annotationEditorLayer .annotationCommentButton"; + await page.waitForSelector(buttonSelector, { visible: true }); + await page.hover(buttonSelector); + const popupSelector = "#commentPopup"; + await page.waitForSelector(popupSelector, { + visible: true, + }); + let popupText = await page.evaluate( + selector => document.querySelector(selector).textContent, + `${popupSelector} .commentPopupText` + ); + expect(popupText).withContext(`In ${browserName}`).toEqual(comment); + + await page.hover("#editorHighlightButton"); + await switchToHighlight(page, /* disable = */ true); + + buttonSelector = ".annotationLayer .annotationCommentButton"; + await page.waitForSelector(buttonSelector, { + visible: true, + }); + await page.hover(buttonSelector); + + await page.waitForSelector(popupSelector, { + visible: true, + }); + popupText = await page.evaluate( + selector => document.querySelector(selector).textContent, + `${popupSelector} .commentPopupText` + ); + expect(popupText).withContext(`In ${browserName}`).toEqual(comment); + }) + ); + }); }); describe("Focused element after editing", () => { diff --git a/web/annotation_editor_layer_builder.css b/web/annotation_editor_layer_builder.css index d93ffb724..e7d80a797 100644 --- a/web/annotation_editor_layer_builder.css +++ b/web/annotation_editor_layer_builder.css @@ -69,6 +69,13 @@ } } +.page:has(.annotationEditorLayer.nonEditing) + .annotationLayer + .editorAnnotation { + position: absolute; + pointer-events: none; +} + #viewerContainer.pdfPresentationMode:fullscreen, .annotationEditorLayer.disabled { .noAltTextBadge { diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js index 262d60240..40670a3d0 100644 --- a/web/annotation_layer_builder.js +++ b/web/annotation_layer_builder.js @@ -64,13 +64,6 @@ import { PresentationModeState } from "./ui_utils.js"; * @property {StructTreeLayerBuilder} [structTreeLayer] */ -/** - * @typedef {Object} InjectLinkAnnotationsOptions - * @property {Array} inferredLinks - * @property {PageViewport} viewport - * @property {StructTreeLayerBuilder} [structTreeLayer] - */ - class AnnotationLayerBuilder { #annotations = null; @@ -158,23 +151,19 @@ class AnnotationLayerBuilder { const div = (this.div = document.createElement("div")); div.className = "annotationLayer"; this.#onAppend?.(div); + this.#initAnnotationLayer(viewport, structTreeLayer); if (annotations.length === 0) { this.#annotations = annotations; - - this.hide(/* internal = */ true); + setLayerDimensions(this.div, viewport); return; } - this.#initAnnotationLayer(viewport, structTreeLayer); - await this.annotationLayer.render({ annotations, imageResourcesPath: this.imageResourcesPath, renderForms: this.renderForms, - linkService: this.linkService, downloadManager: this.downloadManager, - annotationStorage: this.annotationStorage, enableComment: this.enableComment, enableScripting: this.enableScripting, hasJSActions, @@ -207,10 +196,12 @@ class AnnotationLayerBuilder { accessibilityManager: this._accessibilityManager, annotationCanvasMap: this._annotationCanvasMap, annotationEditorUIManager: this._annotationEditorUIManager, + annotationStorage: this.annotationStorage, page: this.pdfPage, viewport: viewport.clone({ dontFlip: true }), structTreeLayer, commentManager: this.#commentManager, + linkService: this.linkService, }); } @@ -234,15 +225,11 @@ class AnnotationLayerBuilder { } /** - * @param {InjectLinkAnnotationsOptions} options + * @param {Array} inferredLinks * @returns {Promise} A promise that is resolved when the inferred links * are added to the annotation layer. */ - async injectLinkAnnotations({ - inferredLinks, - viewport, - structTreeLayer = null, - }) { + async injectLinkAnnotations(inferredLinks) { if (this.#annotations === null) { throw new Error( "`render` method must be called before `injectLinkAnnotations`." @@ -261,12 +248,7 @@ class AnnotationLayerBuilder { return; } - if (!this.annotationLayer) { - this.#initAnnotationLayer(viewport, structTreeLayer); - setLayerDimensions(this.div, viewport); - } - - await this.annotationLayer.addLinkAnnotations(newLinks, this.linkService); + await this.annotationLayer.addLinkAnnotations(newLinks); // Don't show the annotation layer if it was explicitly hidden previously. if (!this.#externalHide) { this.div.hidden = false; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index aca1ecdf0..fbec6cb3e 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -521,11 +521,9 @@ class PDFPageView extends BasePDFPageView { if (!this.annotationLayer) { return; // Rendering was cancelled while the textLayerPromise resolved. } - await this.annotationLayer.injectLinkAnnotations({ - inferredLinks: Autolinker.processLinks(this), - viewport: this.viewport, - structTreeLayer: this.structTreeLayer, - }); + await this.annotationLayer.injectLinkAnnotations( + Autolinker.processLinks(this) + ); } catch (ex) { console.error("#injectLinkAnnotations:", ex); error = ex; @@ -1120,20 +1118,25 @@ class PDFPageView extends BasePDFPageView { await this.#renderDrawLayer(); this.drawLayer.setParent(canvasWrapper); - this.annotationEditorLayer ||= new AnnotationEditorLayerBuilder({ - uiManager: annotationEditorUIManager, - pdfPage, - l10n, - structTreeLayer: this.structTreeLayer, - accessibilityManager: this._accessibilityManager, - annotationLayer: this.annotationLayer?.annotationLayer, - textLayer: this.textLayer, - drawLayer: this.drawLayer.getDrawLayer(), - onAppend: annotationEditorLayerDiv => { - this.#addLayer(annotationEditorLayerDiv, "annotationEditorLayer"); - }, - }); - this.#renderAnnotationEditorLayer(); + if ( + this.annotationLayer || + this.#annotationMode === AnnotationMode.DISABLE + ) { + this.annotationEditorLayer ||= new AnnotationEditorLayerBuilder({ + uiManager: annotationEditorUIManager, + pdfPage, + l10n, + structTreeLayer: this.structTreeLayer, + accessibilityManager: this._accessibilityManager, + annotationLayer: this.annotationLayer?.annotationLayer, + textLayer: this.textLayer, + drawLayer: this.drawLayer.getDrawLayer(), + onAppend: annotationEditorLayerDiv => { + this.#addLayer(annotationEditorLayerDiv, "annotationEditorLayer"); + }, + }); + this.#renderAnnotationEditorLayer(); + } }); if (pdfPage.isPureXfa) {