From 38b86cb97a23a7018eac837477dbdc2922561936 Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Mon, 8 Sep 2025 18:55:29 +0200 Subject: [PATCH] [Annotation] Use the new popup in reading mode (bug 1987426) --- src/display/annotation_layer.js | 283 ++++++++++++++++++++++++-------- src/display/editor/comment.js | 6 +- src/display/editor/editor.js | 5 +- web/annotation_layer_builder.js | 7 + web/app_options.js | 2 +- web/comment_manager.css | 1 + web/pdf_page_view.js | 6 + 7 files changed, 236 insertions(+), 74 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index e13e9036f..11217c1d7 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -24,6 +24,8 @@ /** @typedef {import("../src/display/editor/tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */ // eslint-disable-next-line max-len /** @typedef {import("../../web/struct_tree_layer_builder.js").StructTreeLayerBuilder} StructTreeLayerBuilder */ +// eslint-disable-next-line max-len +/** @typedef {import("../../web/comment_manager.js").CommentManager} CommentManager */ import { AnnotationBorderStyleType, @@ -38,9 +40,6 @@ import { warn, } from "../shared/util.js"; import { - applyOpacity, - CSSConstants, - findContrastColor, PDFDateString, renderRichText, setLayerDimensions, @@ -206,7 +205,7 @@ class AnnotationElement { } get hasCommentButton() { - return this.enableComment && this._isEditable && this.hasPopupElement; + return this.enableComment && this.hasPopupElement; } get commentButtonPosition() { @@ -230,16 +229,6 @@ class AnnotationElement { return null; } - get commentButtonColor() { - if (!this.data.color) { - return null; - } - return findContrastColor( - applyOpacity(...this.data.color, this.data.opacity), - CSSConstants.commentForegroundColor - ); - } - _normalizePoint(point) { const { page: { view }, @@ -253,6 +242,11 @@ class AnnotationElement { return point; } + removePopup() { + (this.#popupElement?.popup || this.popup)?.remove(); + this.#popupElement = this.popup = null; + } + updateEdited(params) { if (!this.container) { return; @@ -272,8 +266,10 @@ class AnnotationElement { let popup = this.#popupElement?.popup || this.popup; if (!popup && newPopup?.text) { - this._createPopup(newPopup); - popup = this.#popupElement.popup; + if (!this.parent._commentManager) { + this._createPopup(newPopup); + popup = this.#popupElement.popup; + } } if (!popup) { return; @@ -680,6 +676,9 @@ class AnnotationElement { * @memberof AnnotationElement */ _createPopup(popupData = null) { + if (this.parent._commentManager) { + return; + } const { data } = this; let contentsObj, modificationDate; @@ -2228,18 +2227,24 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement { class PopupAnnotationElement extends AnnotationElement { constructor(parameters) { - const { data, elements } = parameters; - super(parameters, { isRenderable: AnnotationElement._hasPopupData(data) }); + const { data, elements, parent } = parameters; + const hasCommentManager = !!parent._commentManager; + super(parameters, { + isRenderable: !hasCommentManager && AnnotationElement._hasPopupData(data), + }); this.elements = elements; - this.popup = null; + if (hasCommentManager && AnnotationElement._hasPopupData(data)) { + const popup = (this.popup = this.#createPopup()); + for (const element of elements) { + element.popup = popup; + } + } else { + this.popup = null; + } } - render() { - const { container } = this; - container.classList.add("popupAnnotation"); - container.role = "comment"; - - const popup = (this.popup = new PopupElement({ + #createPopup() { + return new PopupElement({ container: this.container, color: this.data.color, titleObj: this.data.titleObj, @@ -2251,8 +2256,16 @@ class PopupAnnotationElement extends AnnotationElement { parent: this.parent, elements: this.elements, open: this.data.open, - eventBus: this.linkService.eventBus, - })); + commentManager: this.parent._commentManager, + }); + } + + render() { + const { container } = this; + container.classList.add("popupAnnotation"); + container.role = "comment"; + + const popup = (this.popup = this.#createPopup()); const elementIds = []; for (const element of this.elements) { @@ -2272,6 +2285,8 @@ class PopupAnnotationElement extends AnnotationElement { } class PopupElement { + #commentManager = null; + #boundKeyDown = this.#keyDown.bind(this); #boundHide = this.#hide.bind(this); @@ -2290,8 +2305,6 @@ class PopupElement { #elements = null; - #eventBus = null; - #parent = null; #parentRect = null; @@ -2308,7 +2321,7 @@ class PopupElement { #commentButtonPosition = null; - #commentButtonColor = null; + #popupPosition = null; #rect = null; @@ -2320,6 +2333,8 @@ class PopupElement { #wasVisible = false; + #firstElement = null; + constructor({ container, color, @@ -2332,7 +2347,7 @@ class PopupElement { rect, parentRect, open, - eventBus = null, + commentManager = null, }) { this.#container = container; this.#titleObj = titleObj; @@ -2343,29 +2358,35 @@ class PopupElement { this.#rect = rect; this.#parentRect = parentRect; this.#elements = elements; - this.#eventBus = eventBus; + this.#commentManager = commentManager; + this.#firstElement = elements[0]; // The modification date is shown in the popup instead of the creation // date if it is available and can be parsed correctly, which is // consistent with other viewers such as Adobe Acrobat. this.#dateObj = PDFDateString.toDateObject(modificationDate); - this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup()); - this.#addEventListeners(); + if (commentManager) { + this.#popupAbortController = new AbortController(); + this.#renderCommentButton(); + } else { + this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup()); + this.#addEventListeners(); - this.#container.hidden = true; - if (open) { - this.#toggle(); - } + this.#container.hidden = true; + if (open) { + this.#toggle(); + } - if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { - // Since the popup is lazily created, we need to ensure that it'll be - // created and displayed during reference tests. - this.#parent.popupShow.push(async () => { - if (this.#container.hidden) { - this.#show(); - } - }); + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { + // Since the popup is lazily created, we need to ensure that it'll be + // created and displayed during reference tests. + this.#parent.popupShow.push(async () => { + if (this.#container.hidden) { + this.#show(); + } + }); + } } } @@ -2390,8 +2411,6 @@ class PopupElement { signal, }); } - - this.#renderCommentButton(); } #setCommentButtonPosition() { @@ -2402,7 +2421,6 @@ class PopupElement { this.#commentButtonPosition = element._normalizePoint( element.commentButtonPosition ); - this.#commentButtonColor = element.commentButtonColor; } #renderCommentButton() { @@ -2420,40 +2438,153 @@ class PopupElement { const button = (this.#commentButton = document.createElement("button")); button.className = "annotationCommentButton"; - const parentContainer = this.#elements[0].container; + const parentContainer = this.#firstElement.container; button.style.zIndex = parentContainer.style.zIndex + 1; button.tabIndex = 0; + button.ariaHasPopup = "dialog"; + button.ariaControls = "commentPopup"; const { signal } = this.#popupAbortController; - button.addEventListener("hover", this.#boundToggle, { signal }); button.addEventListener("keydown", this.#boundKeyDown, { signal }); button.addEventListener( "click", () => { - const [ - { - data: { id: editId }, - annotationEditorType: mode, - }, - ] = this.#elements; - this.#eventBus?.dispatch("switchannotationeditormode", { - source: this, - editId, - mode, - editComment: true, - }); + this.#commentManager.toggleCommentPopup(this, /* isSelected = */ true); + }, + { signal } + ); + button.addEventListener( + "pointerenter", + () => { + this.#commentManager.toggleCommentPopup( + this, + /* isSelected = */ false, + /* visibility = */ true + ); + }, + { signal } + ); + button.addEventListener( + "pointerleave", + () => { + this.#commentManager.toggleCommentPopup( + this, + /* isSelected = */ false, + /* visibility = */ false + ); }, { signal } ); const { style } = button; style.left = `calc(${this.#commentButtonPosition[0]}%)`; style.top = `calc(${this.#commentButtonPosition[1]}% - var(--comment-button-dim))`; - if (this.#commentButtonColor) { - style.backgroundColor = this.#commentButtonColor; + if (this.commentButtonColor) { + style.backgroundColor = this.commentButtonColor; } parentContainer.after(button); } + get commentButtonColor() { + const { + data: { color, opacity }, + } = this.#firstElement; + if (!color) { + return null; + } + return this.#parent._commentManager.makeCommentColor(color, opacity); + } + + getData() { + return this.#firstElement.data; + } + + get elementBeforePopup() { + return this.#commentButton; + } + + get comment() { + return this.#firstElement.data.contentsObj?.str || ""; + } + + set comment(text) { + const element = this.#firstElement; + if (text) { + element.data.contentsObj = { str: text }; + // TODO: Support saving the text. + // element.annotationStorage.setValue(element.data.id, { + // popup: { contents: text }, + // }); + } else { + element.data.contentsObj = null; + element.removePopup(); + } + element.data.modificationDate = new Date(); + } + + get parentBoundingClientRect() { + return this.#firstElement.layer.getBoundingClientRect(); + } + + setCommentButtonStates({ selected, hasPopup }) { + if (!this.#commentButton) { + return; + } + this.#commentButton.classList.toggle("selected", selected); + this.#commentButton.ariaExpanded = hasPopup; + } + + setSelectedCommentButton(selected) { + this.#commentButton.classList.toggle("selected", selected); + } + + get commentPopupPosition() { + if (this.#popupPosition) { + return this.#popupPosition; + } + const { x, y, height } = this.#commentButton.getBoundingClientRect(); + const { + x: parentX, + y: parentY, + width: parentWidth, + height: parentHeight, + } = this.#firstElement.layer.getBoundingClientRect(); + return [(x - parentX) / parentWidth, (y + height - parentY) / parentHeight]; + } + + set commentPopupPosition(pos) { + this.#popupPosition = pos; + } + + get commentButtonPosition() { + return this.#commentButtonPosition; + } + + get commentButtonWidth() { + return ( + this.#commentButton.getBoundingClientRect().width / + this.parentBoundingClientRect.width + ); + } + + editComment(options) { + const [posX, posY] = + this.#popupPosition || this.commentButtonPosition.map(x => x / 100); + const parentDimensions = this.parentBoundingClientRect; + const { + x: parentX, + y: parentY, + width: parentWidth, + height: parentHeight, + } = parentDimensions; + this.#commentManager.showDialog( + null, + this, + parentX + posX * parentWidth, + parentY + posY * parentHeight, + { ...options, parentDimensions } + ); + } + render() { if (this.#popup) { return; @@ -2612,8 +2743,12 @@ class PopupElement { this.#popup = null; this.#wasVisible = false; this.#pinned = false; - for (const element of this.trigger) { - element.classList.remove("popupTriggerArea"); + this.#commentButton?.remove(); + this.#commentButton = null; + if (this.trigger) { + for (const element of this.trigger) { + element.classList.remove("popupTriggerArea"); + } } } @@ -2665,6 +2800,11 @@ class PopupElement { * Toggle the visibility of the popup. */ #toggle() { + if (this.#commentManager) { + this.#commentManager.toggleCommentPopup(this, /* isSelected = */ false); + return; + } + this.#pinned = !this.#pinned; if (this.#pinned) { this.#show(); @@ -2716,6 +2856,9 @@ class PopupElement { } maybeShow() { + if (this.#commentManager) { + return; + } this.#addEventListeners(); if (!this.#wasVisible) { return; @@ -2728,6 +2871,9 @@ class PopupElement { } get isVisible() { + if (this.#commentManager) { + return false; + } return this.#container.hidden === false; } } @@ -3425,6 +3571,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement { * @property {TextAccessibilityManager} [accessibilityManager] * @property {AnnotationEditorUIManager} [annotationEditorUIManager] * @property {StructTreeLayerBuilder} [structTreeLayer] + * @property {CommentManager} [commentManager] - The comment manager instance. */ /** @@ -3447,6 +3594,7 @@ class AnnotationLayer { page, viewport, structTreeLayer, + commentManager, }) { this.div = div; this.#accessibilityManager = accessibilityManager; @@ -3456,6 +3604,7 @@ class AnnotationLayer { this.viewport = viewport; this.zIndex = 0; this._annotationEditorUIManager = annotationEditorUIManager; + this._commentManager = commentManager || null; if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) { // For testing purposes. diff --git a/src/display/editor/comment.js b/src/display/editor/comment.js index 4845c6479..733247d89 100644 --- a/src/display/editor/comment.js +++ b/src/display/editor/comment.js @@ -104,11 +104,7 @@ class Comment { width: parentWidth, height: parentHeight, } = this.#editor.parent.boundingClientRect; - const OFFSET_UNDER_BUTTON = 2; - return [ - (x - parentX) / parentWidth, - (y + height + OFFSET_UNDER_BUTTON - parentY) / parentHeight, - ]; + return [(x - parentX) / parentWidth, (y + height - parentY) / parentHeight]; } set commentPopupPositionInLayer(pos) { diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index db0d3af29..8f28c67b6 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -1237,7 +1237,10 @@ class AnnotationEditor { } } - setCommentData({ comment, richText }) { + setCommentData({ comment, popupRef, richText }) { + if (!popupRef) { + return; + } this.#comment ||= new Comment(this); this.#comment.setInitialText(comment, richText); } diff --git a/web/annotation_layer_builder.js b/web/annotation_layer_builder.js index b7aa4c720..262d60240 100644 --- a/web/annotation_layer_builder.js +++ b/web/annotation_layer_builder.js @@ -26,6 +26,7 @@ /** @typedef {import("./text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */ // eslint-disable-next-line max-len /** @typedef {import("../src/display/editor/tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */ +/** @typedef {import("./comment_manager.js").CommentManager} CommentManager */ import { AnnotationLayer, @@ -53,6 +54,7 @@ import { PresentationModeState } from "./ui_utils.js"; * @property {TextAccessibilityManager} [accessibilityManager] * @property {AnnotationEditorUIManager} [annotationEditorUIManager] * @property {function} [onAppend] + * @property {CommentManager} [commentManager] */ /** @@ -72,6 +74,8 @@ import { PresentationModeState } from "./ui_utils.js"; class AnnotationLayerBuilder { #annotations = null; + #commentManager = null; + #externalHide = false; #onAppend = null; @@ -91,6 +95,7 @@ class AnnotationLayerBuilder { imageResourcesPath = "", renderForms = true, enableComment = false, + commentManager = null, enableScripting = false, hasJSActionsPromise = null, fieldObjectsPromise = null, @@ -106,6 +111,7 @@ class AnnotationLayerBuilder { this.renderForms = renderForms; this.annotationStorage = annotationStorage; this.enableComment = enableComment; + this.#commentManager = commentManager; this.enableScripting = enableScripting; this._hasJSActionsPromise = hasJSActionsPromise || Promise.resolve(false); this._fieldObjectsPromise = fieldObjectsPromise || Promise.resolve(null); @@ -204,6 +210,7 @@ class AnnotationLayerBuilder { page: this.pdfPage, viewport: viewport.clone({ dontFlip: true }), structTreeLayer, + commentManager: this.#commentManager, }); } diff --git a/web/app_options.js b/web/app_options.js index 8d675df5a..142d40f4a 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -228,7 +228,7 @@ const defaultOptions = { }, enableComment: { /** @type {boolean} */ - value: typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING"), + value: typeof PDFJSDev === "undefined", kind: OptionKind.VIEWER + OptionKind.PREFERENCE, }, enableDetailCanvas: { diff --git a/web/comment_manager.css b/web/comment_manager.css index c9ea7675b..8dffbc714 100644 --- a/web/comment_manager.css +++ b/web/comment_manager.css @@ -496,6 +496,7 @@ gap: 12px; z-index: 100001; /* above selected annotation editor */ pointer-events: auto; + margin-top: 2px; border: 0.5px solid var(--comment-border-color); background: var(--comment-bg-color); diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index 061d93b58..aca1ecdf0 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -22,6 +22,7 @@ /** @typedef {import("./interfaces").IRenderableView} IRenderableView */ // eslint-disable-next-line max-len /** @typedef {import("./pdf_rendering_queue").PDFRenderingQueue} PDFRenderingQueue */ +/** @typedef {import("./comment_manager.js").CommentManager} CommentManager */ import { AbortException, @@ -102,6 +103,7 @@ import { XfaLayerBuilder } from "./xfa_layer_builder.js"; * the necessary layer-properties. * @property {boolean} [enableAutoLinking] - Enable creation of hyperlinks from * text that look like URLs. The default value is `true`. + * @property {CommentManager} [commentManager] - The comment manager instance. */ const DEFAULT_LAYER_PROPERTIES = @@ -136,6 +138,8 @@ class PDFPageView extends BasePDFPageView { #canvasWrapper = null; + #commentManager = null; + #enableAutoLinking = true; #hasRestrictedScaling = false; @@ -197,6 +201,7 @@ class PDFPageView extends BasePDFPageView { this.capCanvasAreaFactor = options.capCanvasAreaFactor ?? AppOptions.get("capCanvasAreaFactor"); this.#enableAutoLinking = options.enableAutoLinking !== false; + this.#commentManager = options.commentManager || null; this.l10n = options.l10n; if (typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) { @@ -1011,6 +1016,7 @@ class PDFPageView extends BasePDFPageView { annotationCanvasMap: this._annotationCanvasMap, accessibilityManager: this._accessibilityManager, annotationEditorUIManager, + commentManager: this.#commentManager, onAppend: annotationLayerDiv => { this.#addLayer(annotationLayerDiv, "annotationLayer"); },