diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 37c03a26a..53753afc3 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -202,7 +202,19 @@ class AnnotationElement { } get hasPopupData() { - return AnnotationElement._hasPopupData(this.data); + return ( + AnnotationElement._hasPopupData(this.data) || + (this.enableComment && !!this.commentText) + ); + } + + get commentData() { + const { data } = this; + const editor = this.annotationStorage?.getEditor(data.id); + if (editor) { + return editor.getData(); + } + return data; } get hasCommentButton() { @@ -210,7 +222,11 @@ class AnnotationElement { } get commentButtonPosition() { - const { quadPoints, rect } = this.data; + const editor = this.annotationStorage?.getEditor(this.data.id); + if (editor) { + return editor.commentButtonPositionInPage; + } + const { quadPoints, inkLists, rect } = this.data; let maxX = -Infinity; let maxY = -Infinity; if (quadPoints?.length >= 8) { @@ -224,6 +240,21 @@ class AnnotationElement { } return [maxX, maxY]; } + if (inkLists?.length >= 1) { + for (const inkList of inkLists) { + for (let i = 0, ii = inkList.length; i < ii; i += 2) { + if (inkList[i + 1] > maxY) { + maxY = inkList[i + 1]; + maxX = inkList[i]; + } else if (inkList[i + 1] === maxY) { + maxX = Math.max(maxX, inkList[i]); + } + } + } + if (maxX !== Infinity) { + return [maxX, maxY]; + } + } if (rect) { return [rect[2], rect[3]]; } @@ -2380,7 +2411,6 @@ class PopupElement { this.#dateObj = PDFDateString.toDateObject(modificationDate); if (commentManager) { - this.#popupAbortController = new AbortController(); this.#renderCommentButton(); } else { this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup()); @@ -2457,7 +2487,7 @@ class PopupElement { button.ariaHasPopup = "dialog"; button.ariaControls = "commentPopup"; - const { signal } = this.#popupAbortController; + const { signal } = (this.#popupAbortController = new AbortController()); button.addEventListener("keydown", this.#boundKeyDown, { signal }); button.addEventListener( "click", @@ -2488,19 +2518,26 @@ class PopupElement { }, { 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; - } + this.#updateColor(); + this.#updateCommentButtonPosition(); parentContainer.after(button); } + #updateCommentButtonPosition() { + this.#renderCommentButton(); + const [x, y] = this.#commentButtonPosition; + const { style } = this.#commentButton; + style.left = `calc(${x}%)`; + style.top = `calc(${y}% - var(--comment-button-dim))`; + } + + #updateColor() { + this.#renderCommentButton(); + this.#commentButton.style.backgroundColor = this.commentButtonColor || ""; + } + get commentButtonColor() { - const { - data: { color, opacity }, - } = this.#firstElement; + const { color, opacity } = this.#firstElement.commentData; if (!color) { return null; } @@ -2509,7 +2546,7 @@ class PopupElement { getData() { const { richText, color, opacity, creationDate, modificationDate } = - this.#firstElement.data; + this.#firstElement.commentData; return { contentsObj: { str: this.comment }, richText, @@ -2743,7 +2780,22 @@ class PopupElement { updateEdited({ rect, popup, deleted }) { if (this.#commentManager) { - this.#commentText = deleted ? null : popup.text; + if (deleted) { + this.remove(); + this.#commentText = null; + } else if (popup) { + if (popup.deleted) { + this.remove(); + } else { + this.#updateColor(); + this.#commentText = popup.text; + } + } + if (rect) { + this.#commentButtonPosition = null; + this.#setCommentButtonPosition(); + this.#updateCommentButtonPosition(); + } return; } if (deleted || popup?.deleted) { @@ -2758,7 +2810,7 @@ class PopupElement { if (rect) { this.#position = null; } - if (popup) { + if (popup && popup.text) { this.#richText = this.#makePopupContent(popup.text); this.#dateObj = PDFDateString.toDateObject(popup.date); this.#contentsObj = null; @@ -3346,31 +3398,6 @@ class InkAnnotationElement extends AnnotationElement { addHighlightArea() { this.container.classList.add("highlightArea"); } - - get commentButtonPosition() { - const { inkLists, rect } = this.data; - if (inkLists?.length >= 1) { - let maxX = -Infinity; - let maxY = -Infinity; - for (const inkList of inkLists) { - for (let i = 0, ii = inkList.length; i < ii; i += 2) { - if (inkList[i + 1] > maxY) { - maxY = inkList[i + 1]; - maxX = inkList[i]; - } else if (inkList[i + 1] === maxY) { - maxX = Math.max(maxX, inkList[i]); - } - } - } - if (maxX !== Infinity) { - return [maxX, maxY]; - } - } - if (rect) { - return [rect[2], rect[3]]; - } - return null; - } } class HighlightAnnotationElement extends AnnotationElement { diff --git a/src/display/annotation_storage.js b/src/display/annotation_storage.js index c1f21beb8..6d6ff8ad8 100644 --- a/src/display/annotation_storage.js +++ b/src/display/annotation_storage.js @@ -268,6 +268,10 @@ class AnnotationStorage { return false; } + getEditor(annotationId) { + return this.#editorsMap?.get(annotationId) || null; + } + /** * @returns {{ids: Set, hash: string}} */ diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 23ab12df7..65b86094c 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -1930,6 +1930,17 @@ class AnnotationEditor { return this._uiManager.direction === "ltr" ? [1, 0] : [0, 0]; } + get commentButtonPositionInPage() { + const { + commentButtonPosition: [posX, posY], + } = this; + const [blX, blY, trX, trY] = this.getPDFRect(); + return [ + AnnotationEditor._round(blX + (trX - blX) * posX), + AnnotationEditor._round(blY + (trY - blY) * (1 - posY)), + ]; + } + get commentButtonColor() { return this._uiManager.makeCommentColor( this.getNonHCMColor(), diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index 595b3cdbd..ca9eca500 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -904,13 +904,13 @@ class FreeTextEditor extends AnnotationEditor { content.append(div); } - const params = { + annotation.updateEdited({ rect: this.getPDFRect(), - }; - params.popup = this.hasEditedComment - ? this.comment - : { text: this.#content }; - annotation.updateEdited(params); + popup: + this._uiManager.hasCommentManager() || this.hasEditedComment + ? this.comment + : { text: this.#content }, + }); return content; } diff --git a/src/display/editor/highlight.js b/src/display/editor/highlight.js index 125184c4b..d40aff479 100644 --- a/src/display/editor/highlight.js +++ b/src/display/editor/highlight.js @@ -172,12 +172,13 @@ class HighlightEditor extends AnnotationEditor { ); this.#focusOutlines = outlinerForOutline.getOutlines(); - // The last point is in the pages coordinate system. - const { firstPoint, lastPoint } = this.#focusOutlines; + const { firstPoint } = this.#highlightOutlines; this.#firstPoint = [ (firstPoint[0] - this.x) / this.width, (firstPoint[1] - this.y) / this.height, ]; + // The last point is in the pages coordinate system. + const { lastPoint } = this.#focusOutlines; this.#lastPoint = [ (lastPoint[0] - this.x) / this.width, (lastPoint[1] - this.y) / this.height, @@ -268,11 +269,12 @@ class HighlightEditor extends AnnotationEditor { } } - const { firstPoint, lastPoint } = this.#focusOutlines; + const { firstPoint } = highlightOutlines; this.#firstPoint = [ (firstPoint[0] - x) / width, (firstPoint[1] - y) / height, ]; + const { lastPoint } = this.#focusOutlines; this.#lastPoint = [(lastPoint[0] - x) / width, (lastPoint[1] - y) / height]; } @@ -1082,13 +1084,10 @@ class HighlightEditor extends AnnotationEditor { annotation.hide(); return null; } - const params = { + annotation.updateEdited({ rect: this.getPDFRect(), - }; - if (this.hasEditedComment) { - params.popup = this.comment; - } - annotation.updateEdited(params); + popup: this.comment, + }); return null; } diff --git a/src/display/editor/ink.js b/src/display/editor/ink.js index ce4cccc9a..ad7414048 100644 --- a/src/display/editor/ink.js +++ b/src/display/editor/ink.js @@ -305,15 +305,12 @@ class InkEditor extends DrawingEditor { return null; } const { points, rect } = this.serializeDraw(/* isForCopying = */ false); - const params = { + annotation.updateEdited({ rect, thickness: this._drawingOptions["stroke-width"], points, - }; - if (this.hasEditedComment) { - params.popup = this.comment; - } - annotation.updateEdited(params); + popup: this.comment, + }); return null; } diff --git a/src/display/editor/stamp.js b/src/display/editor/stamp.js index 6ed7daea4..51c9c10fb 100644 --- a/src/display/editor/stamp.js +++ b/src/display/editor/stamp.js @@ -942,13 +942,10 @@ class StampEditor extends AnnotationEditor { annotation.hide(); return null; } - const params = { + annotation.updateEdited({ rect: this.getPDFRect(), - }; - if (this.hasEditedComment) { - params.popup = this.comment; - } - annotation.updateEdited(params); + popup: this.comment, + }); return null; } diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index 113d7543c..f3f7b37ad 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -15,16 +15,20 @@ import { closePages, + dragAndDrop, getEditorSelector, getRect, getSpanRectFromText, loadAndWait, scrollIntoView, + selectEditor, switchToEditor, waitAndClick, + waitForSerialized, } from "./test_utils.mjs"; const switchToHighlight = switchToEditor.bind(null, "Highlight"); +const switchToStamp = switchToEditor.bind(null, "Stamp"); describe("Comment", () => { describe("Comment edit dialog must be visible in ltr", () => { @@ -131,4 +135,105 @@ describe("Comment", () => { ); }); }); + + describe("Update comment position and color in reading mode", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "comments.pdf", + ".annotationEditorLayer", + "page-fit", + null, + { + enableComment: true, + highlightEditorColors: + "yellow=#FFFF00,green=#00FF00,blue=#0000FF,pink=#FF00FF,red=#FF0000", + } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must set the comment button at the right place", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToStamp(page); + + const stampSelector = getEditorSelector(8); + await selectEditor(page, stampSelector); + await dragAndDrop(page, stampSelector, [[100, 100]]); + await waitForSerialized(page, 1); + const rectCommentButton = await getRect( + page, + `${stampSelector} .annotationCommentButton` + ); + + await switchToStamp(page, /* disable = */ true); + const rectCommentButtonAfter = await getRect( + page, + `#pdfjs_internal_id_713R + .annotationCommentButton` + ); + + expect(Math.abs(rectCommentButtonAfter.x - rectCommentButton.x)) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(1); + expect(Math.abs(rectCommentButtonAfter.y - rectCommentButton.y)) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(1); + expect( + Math.abs(rectCommentButtonAfter.width - rectCommentButton.width) + ) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(1); + expect( + Math.abs(rectCommentButtonAfter.height - rectCommentButton.height) + ) + .withContext(`In ${browserName}`) + .toBeLessThanOrEqual(1); + }) + ); + }); + + it("must set the right color to the comment button", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + const highlightSelector = getEditorSelector(0); + await selectEditor(page, highlightSelector); + const colorButtonSelector = `${highlightSelector} .editToolbar button`; + await page.waitForSelector(`${colorButtonSelector}.colorPicker`); + await page.click(`${colorButtonSelector}.colorPicker`); + await page.waitForSelector(`${colorButtonSelector}[title = "Red"]`); + await page.click(`${colorButtonSelector}[title = "Red"]`); + await page.waitForSelector( + `.page[data-page-number = "1"] svg.highlight[fill = "#FF0000"]` + ); + + const commentButtonColor = await page.evaluate(selector => { + const button = document.querySelector( + `${selector} .annotationCommentButton` + ); + return window.getComputedStyle(button).backgroundColor; + }, highlightSelector); + + await switchToHighlight(page, /* disable = */ true); + + const commentButtonColorAfter = await page.evaluate(() => { + const button = document.querySelector( + "section[data-annotation-id='612R'] + .annotationCommentButton" + ); + return window.getComputedStyle(button).backgroundColor; + }); + + expect(commentButtonColorAfter) + .withContext(`In ${browserName}`) + .toEqual(commentButtonColor); + }) + ); + }); + }); }); diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore index 609bbd87a..6b1ede9ae 100644 --- a/test/pdfs/.gitignore +++ b/test/pdfs/.gitignore @@ -745,3 +745,4 @@ !tracemonkey_annotation_on_page_8.pdf !issue20232.pdf !bug1989304.pdf +!comments.pdf diff --git a/test/pdfs/comments.pdf b/test/pdfs/comments.pdf new file mode 100644 index 000000000..bb0857f02 Binary files /dev/null and b/test/pdfs/comments.pdf differ