Merge pull request #20374 from calixteman/bug1995028

[Editor] FreeText annotations aren't supposed to have an attached popup so disable commenting for them (bug 1995028)

Author: timvandermeij

src/display/editor/editor.js

@@ -188,6 +188,7 @@ class AnnotationEditor {
     this.annotationElementId = parameters.annotationElementId || null;
     this.creationDate = parameters.creationDate || new Date();
     this.modificationDate = parameters.modificationDate || null;
+    this.canAddComment = true;
 
     const {
       rotation,
@@ -1173,7 +1174,7 @@ class AnnotationEditor {
   }
 
   addCommentButton() {
-    return (this.#comment ||= new Comment(this));
+    return this.canAddComment ? (this.#comment ||= new Comment(this)) : null;
   }
 
   addStandaloneCommentButton() {
@@ -1991,7 +1992,7 @@ class AnnotationEditor {
   }
 
   setCommentButtonStates(options) {
-    this.#comment.setCommentButtonStates(options);
+    this.#comment?.setCommentButtonStates(options);
   }
 
   /**

src/display/editor/freetext.js

@@ -135,6 +135,7 @@ class FreeTextEditor extends AnnotationEditor {
     if (!this.annotationElementId) {
       this._uiManager.a11yAlert("pdfjs-editor-freetext-added-alert");
     }
+    this.canAddComment = false;
   }
 
   /** @inheritdoc */

src/display/editor/toolbar.js

@@ -211,24 +211,35 @@ class EditorToolbar {
   async addButton(name, tool) {
     switch (name) {
       case "colorPicker":
-        this.addColorPicker(tool);
+        if (tool) {
+          this.addColorPicker(tool);
+        }
         break;
       case "altText":
-        await this.addAltText(tool);
+        if (tool) {
+          await this.addAltText(tool);
+        }
         break;
       case "editSignature":
-        await this.addEditSignatureButton(tool);
+        if (tool) {
+          await this.addEditSignatureButton(tool);
+        }
         break;
       case "delete":
         this.addDeleteButton();
         break;
       case "comment":
-        this.addComment(tool);
+        if (tool) {
+          this.addComment(tool);
+        }
         break;
     }
   }
 
   async addButtonBefore(name, tool, beforeSelector) {
+    if (!tool && name === "comment") {
+      return;
+    }
     const beforeElement = this.#buttons.querySelector(beforeSelector);
     if (!beforeElement) {
       return;

test/integration/comment_spec.mjs

@@ -27,11 +27,13 @@ import {
   switchToEditor,
   waitAndClick,
   waitForSerialized,
+  waitForTimeout,
 } from "./test_utils.mjs";
 
 const switchToHighlight = switchToEditor.bind(null, "Highlight");
 const switchToStamp = switchToEditor.bind(null, "Stamp");
 const switchToComment = switchToEditor.bind(null, "Comment");
+const switchToFreeText = switchToEditor.bind(null, "FreeText");
 
 const highlightSpan = async (page, pageIndex, text) => {
   const rect = await getSpanRectFromText(page, pageIndex, text);
@@ -838,4 +840,55 @@ describe("Comment", () => {
       );
     });
   });
+
+  describe("FreeText annotation doesn't have a popup (bug 1995028)", () => {
+    let pages;
+
+    beforeEach(async () => {
+      pages = await loadAndWait(
+        "empty.pdf",
+        ".annotationEditorLayer",
+        "page-fit",
+        null,
+        { enableComment: true }
+      );
+    });
+
+    afterEach(async () => {
+      await closePages(pages);
+    });
+
+    it("must check that comment button isn't in the annotation toolbar", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          await switchToFreeText(page);
+
+          const rect = await getRect(page, ".annotationEditorLayer");
+          const editorSelector = getEditorSelector(0);
+          const data = "Hello PDF.js World !!";
+          await page.mouse.click(rect.x + 100, rect.y + 100);
+          await page.waitForSelector(editorSelector, { visible: true });
+          await page.type(`${editorSelector} .internal`, data);
+          await page.keyboard.press("Escape");
+
+          await page.waitForSelector(`${editorSelector} .editToolbar`, {
+            visible: true,
+          });
+
+          // We want to be sure that the comment button isn't rendered.
+          // eslint-disable-next-line no-restricted-syntax
+          await waitForTimeout(100);
+
+          const hasCommentButton = await page.evaluate(
+            selector =>
+              !!document.querySelector(
+                `${selector} .editToolbar button.comment`
+              ),
+            editorSelector
+          );
+          expect(hasCommentButton).withContext(`In ${browserName}`).toBe(false);
+        })
+      );
+    });
+  });
 });