From d9f67bd8ee5a145522a8e4f987f31d289d4e5810 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Wed, 21 Jan 2026 13:37:20 +0100 Subject: [PATCH 1/4] Bug 1999154 - Add the ability to undo comment deletion --- l10n/en-US/viewer.ftl | 1 + src/display/editor/tools.js | 16 +++ test/integration/comment_spec.mjs | 155 ++++++++++++++++++++++++++++++ web/comment_manager.js | 10 +- web/editor_undo_bar.js | 1 + 5 files changed, 181 insertions(+), 2 deletions(-) diff --git a/l10n/en-US/viewer.ftl b/l10n/en-US/viewer.ftl index 3b3760605..9c6636de7 100644 --- a/l10n/en-US/viewer.ftl +++ b/l10n/en-US/viewer.ftl @@ -561,6 +561,7 @@ pdfjs-editor-undo-bar-message-freetext = Text removed pdfjs-editor-undo-bar-message-ink = Drawing removed pdfjs-editor-undo-bar-message-stamp = Image removed pdfjs-editor-undo-bar-message-signature = Signature removed +pdfjs-editor-undo-bar-message-comment = Comment removed # Variables: # $count (Number) - the number of removed annotations. pdfjs-editor-undo-bar-message-multiple = diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index 84d2f23c2..feafa2003 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -1177,6 +1177,22 @@ class AnnotationEditorUIManager { this.#commentManager?.removeComments([editor.uid]); } + /** + * Delete a comment from an editor with undo support. + * @param {AnnotationEditor} editor - The editor whose comment to delete. + * @param {string} savedComment - The comment text to save for undo. + */ + deleteComment(editor, savedComment) { + const undo = () => { + editor.comment = savedComment; + }; + const cmd = () => { + this._editorUndoBar?.show(undo, "comment"); + editor.comment = null; + }; + this.addCommands({ cmd, undo, mustExec: true }); + } + toggleComment(editor, isSelected, visibility = undefined) { this.#commentManager?.toggleCommentPopup(editor, isSelected, visibility); } diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index fc97906ac..36bebb3b6 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -965,4 +965,159 @@ describe("Comment", () => { ); }); }); + + describe("Undo deletion popup for comments (bug 1999154)", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "tracemonkey.pdf", + ".annotationEditorLayer", + "page-fit", + null, + { enableComment: true } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must check that deleting a comment can be undone using the undo button", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + await highlightSpan(page, 1, "Abstract"); + const editorSelector = getEditorSelector(0); + const comment = "Test comment for undo"; + await editComment(page, editorSelector, comment); + + // Stay in highlight mode - don't disable it + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + + await page.waitForSelector("#commentPopup", { visible: true }); + await waitAndClick(page, "button.commentPopupDelete"); + + await page.waitForSelector("#editorUndoBar", { visible: true }); + await page.waitForSelector("#editorUndoBarUndoButton", { + visible: true, + }); + await page.click("#editorUndoBarUndoButton"); + + // Check that the comment is restored by hovering to show the popup + await page.hover(`${editorSelector} .annotationCommentButton`); + await page.waitForSelector("#commentPopup", { visible: true }); + const popupText = await page.evaluate( + () => + document.querySelector("#commentPopup .commentPopupText") + ?.textContent + ); + expect(popupText).withContext(`In ${browserName}`).toEqual(comment); + }) + ); + }); + + it("must check that the undo deletion popup displays 'Comment removed' message", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + await highlightSpan(page, 1, "Abstract"); + const editorSelector = getEditorSelector(0); + await editComment(page, editorSelector, "Test comment"); + + // Stay in highlight mode - don't disable it + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + + await page.waitForSelector("#commentPopup", { visible: true }); + await waitAndClick(page, "button.commentPopupDelete"); + + await page.waitForFunction(() => { + const messageElement = document.querySelector( + "#editorUndoBarMessage" + ); + return messageElement && messageElement.textContent.trim() !== ""; + }); + const message = await page.waitForSelector("#editorUndoBarMessage"); + const messageText = await page.evaluate( + el => el.textContent, + message + ); + expect(messageText) + .withContext(`In ${browserName}`) + .toContain("Comment removed"); + }) + ); + }); + + it("must check that the undo bar closes when clicking the close button", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + await highlightSpan(page, 1, "Abstract"); + const editorSelector = getEditorSelector(0); + await editComment(page, editorSelector, "Test comment"); + + // Stay in highlight mode - don't disable it + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + + await page.waitForSelector("#commentPopup", { visible: true }); + await waitAndClick(page, "button.commentPopupDelete"); + + await page.waitForSelector("#editorUndoBar", { visible: true }); + await waitAndClick(page, "#editorUndoBarCloseButton"); + await page.waitForSelector("#editorUndoBar", { hidden: true }); + }) + ); + }); + + it("must check that deleting a comment can be undone using Ctrl+Z", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + await highlightSpan(page, 1, "Abstract"); + const editorSelector = getEditorSelector(0); + const comment = "Test comment for Ctrl+Z undo"; + await editComment(page, editorSelector, comment); + + // Stay in highlight mode - don't disable it + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + + await page.waitForSelector("#commentPopup", { visible: true }); + await waitAndClick(page, "button.commentPopupDelete"); + + await page.waitForSelector("#editorUndoBar", { visible: true }); + + // Use Ctrl+Z to undo + await kbModifierDown(page); + await page.keyboard.press("z"); + await kbModifierUp(page); + + // The undo bar should be hidden after undo + await page.waitForSelector("#editorUndoBar", { hidden: true }); + + // Check that the comment is restored by hovering to show the popup + await page.hover(`${editorSelector} .annotationCommentButton`); + await page.waitForSelector("#commentPopup", { visible: true }); + const popupText = await page.evaluate( + () => + document.querySelector("#commentPopup .commentPopupText") + ?.textContent + ); + expect(popupText).withContext(`In ${browserName}`).toEqual(comment); + }) + ); + }); + }); }); diff --git a/web/comment_manager.js b/web/comment_manager.js index 0b7bb268d..438d85f78 100644 --- a/web/comment_manager.js +++ b/web/comment_manager.js @@ -982,9 +982,15 @@ class CommentPopup { }, }, }); - this.#editor.comment = null; - this.#editor.focus(); + const savedComment = this.#editor.comment?.text; + const editor = this.#editor; this.destroy(); + if (savedComment) { + editor._uiManager.deleteComment(editor, savedComment); + } else { + editor.comment = null; + } + editor.focus(); }); del.addEventListener("contextmenu", noContextMenu); buttons.append(edit, del); diff --git a/web/editor_undo_bar.js b/web/editor_undo_bar.js index 096547062..1cb3ed90a 100644 --- a/web/editor_undo_bar.js +++ b/web/editor_undo_bar.js @@ -40,6 +40,7 @@ class EditorUndoBar { stamp: "pdfjs-editor-undo-bar-message-stamp", ink: "pdfjs-editor-undo-bar-message-ink", signature: "pdfjs-editor-undo-bar-message-signature", + comment: "pdfjs-editor-undo-bar-message-comment", _multiple: "pdfjs-editor-undo-bar-message-multiple", }); From 84d15dc453aaad06787abfacbdc7a6f0d1daaeea Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Wed, 21 Jan 2026 17:06:13 +0100 Subject: [PATCH 2/4] Restore date too --- src/display/editor/comment.js | 14 ++++++++++++++ src/display/editor/editor.js | 9 +++++++-- src/display/editor/tools.js | 6 +++--- test/integration/comment_spec.mjs | 18 ++++++++++++++++++ web/comment_manager.js | 6 +++--- 5 files changed, 45 insertions(+), 8 deletions(-) diff --git a/src/display/editor/comment.js b/src/display/editor/comment.js index 86bf52ca7..5f73e3e5b 100644 --- a/src/display/editor/comment.js +++ b/src/display/editor/comment.js @@ -314,6 +314,20 @@ class Comment { this.#deleted = false; } + /** + * Restore the comment data (used for undo). + * @param {Object} data - The comment data to restore. + * @param {string} data.text - The comment text. + * @param {string|null} data.richText - The rich text content. + * @param {Date|null} data.date - The original date. + */ + restoreData({ text, richText, date }) { + this.#text = text; + this.#richText = richText; + this.#date = date; + this.#deleted = false; + } + setInitialText(text, richText = null) { this.#initialText = text; this.data = text; diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index db014c7e9..e330a4a94 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -1220,9 +1220,14 @@ class AnnotationEditor { }; } - set comment(text) { + set comment(value) { this.#comment ||= new Comment(this); - this.#comment.data = text; + if (typeof value === "object" && value !== null) { + // Restore full comment data (used for undo). + this.#comment.restoreData(value); + } else { + this.#comment.data = value; + } if (this.hasComment) { this.removeCommentButtonFromToolbar(); this.addStandaloneCommentButton(); diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index feafa2003..7767c024f 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -1180,11 +1180,11 @@ class AnnotationEditorUIManager { /** * Delete a comment from an editor with undo support. * @param {AnnotationEditor} editor - The editor whose comment to delete. - * @param {string} savedComment - The comment text to save for undo. + * @param {Object} savedData - The comment data to save for undo. */ - deleteComment(editor, savedComment) { + deleteComment(editor, savedData) { const undo = () => { - editor.comment = savedComment; + editor.comment = savedData; }; const cmd = () => { this._editorUndoBar?.show(undo, "comment"); diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index 36bebb3b6..2e4971c37 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -999,6 +999,14 @@ describe("Comment", () => { ); await page.waitForSelector("#commentPopup", { visible: true }); + + // Capture the date before deletion + const dateBefore = await page.evaluate( + () => + document.querySelector("#commentPopup .commentPopupTime") + ?.textContent + ); + await waitAndClick(page, "button.commentPopupDelete"); await page.waitForSelector("#editorUndoBar", { visible: true }); @@ -1016,6 +1024,16 @@ describe("Comment", () => { ?.textContent ); expect(popupText).withContext(`In ${browserName}`).toEqual(comment); + + // Check that the date is preserved + const dateAfter = await page.evaluate( + () => + document.querySelector("#commentPopup .commentPopupTime") + ?.textContent + ); + expect(dateAfter) + .withContext(`In ${browserName}`) + .toEqual(dateBefore); }) ); }); diff --git a/web/comment_manager.js b/web/comment_manager.js index 438d85f78..9da5b24fe 100644 --- a/web/comment_manager.js +++ b/web/comment_manager.js @@ -982,11 +982,11 @@ class CommentPopup { }, }, }); - const savedComment = this.#editor.comment?.text; const editor = this.#editor; + const savedData = editor.comment; this.destroy(); - if (savedComment) { - editor._uiManager.deleteComment(editor, savedComment); + if (savedData?.text) { + editor._uiManager.deleteComment(editor, savedData); } else { editor.comment = null; } From bdc9323b15a7702334c41b1c179806b1e919c521 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 22 Jan 2026 13:12:13 +0100 Subject: [PATCH 3/4] Hide comment popup after redo action --- src/display/editor/tools.js | 1 + test/integration/comment_spec.mjs | 40 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index 7767c024f..3f3b3c1da 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -1188,6 +1188,7 @@ class AnnotationEditorUIManager { }; const cmd = () => { this._editorUndoBar?.show(undo, "comment"); + this.toggleComment(/* editor = */ null); editor.comment = null; }; this.addCommands({ cmd, undo, mustExec: true }); diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index 2e4971c37..8fd5cad57 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -24,6 +24,8 @@ import { highlightSpan, kbModifierDown, kbModifierUp, + kbRedo, + kbUndo, loadAndWait, scrollIntoView, selectEditor, @@ -1137,5 +1139,43 @@ describe("Comment", () => { }) ); }); + + it("must check that the comment popup is hidden after redo", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + await highlightSpan(page, 1, "Abstract"); + const editorSelector = getEditorSelector(0); + const comment = "Test comment for redo"; + await editComment(page, editorSelector, comment); + + // Show the popup by clicking the comment button + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + await page.waitForSelector("#commentPopup", { visible: true }); + + // Delete the comment + await waitAndClick(page, "button.commentPopupDelete"); + await page.waitForSelector("#editorUndoBar", { visible: true }); + + // Undo the deletion + await kbUndo(page); + await page.waitForSelector("#editorUndoBar", { hidden: true }); + + // Show the popup again by clicking the comment button + await waitAndClick( + page, + `${editorSelector} .annotationCommentButton` + ); + await page.waitForSelector("#commentPopup", { visible: true }); + + // Redo the deletion - popup should be hidden + await kbRedo(page); + await page.waitForSelector("#commentPopup", { hidden: true }); + }) + ); + }); }); }); From bfdcaadf7c8c85e566d0536e90df70071f312cd7 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 22 Jan 2026 13:12:38 +0100 Subject: [PATCH 4/4] Use kbUndo when possible --- test/integration/comment_spec.mjs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/integration/comment_spec.mjs b/test/integration/comment_spec.mjs index 8fd5cad57..cad7f144c 100644 --- a/test/integration/comment_spec.mjs +++ b/test/integration/comment_spec.mjs @@ -1120,9 +1120,7 @@ describe("Comment", () => { await page.waitForSelector("#editorUndoBar", { visible: true }); // Use Ctrl+Z to undo - await kbModifierDown(page); - await page.keyboard.press("z"); - await kbModifierUp(page); + await kbUndo(page); // The undo bar should be hidden after undo await page.waitForSelector("#editorUndoBar", { hidden: true });