Merge pull request #20312 from calixteman/bug1991029

[Annotation] In reading mode with new commment stuff enabled, use the comment popup for annotations without a popup but with some contents (bug 1991029)

Author: calixteman

src/display/annotation_layer.js

@@ -718,9 +718,6 @@ class AnnotationElement {
    * @memberof AnnotationElement
    */
   _createPopup(popupData = null) {
-    if (this.parent._commentManager) {
-      return;
-    }
     const { data } = this;
 
     let contentsObj, modificationDate;
@@ -750,13 +747,19 @@ class AnnotationElement {
       parent: this.parent,
       elements: [this],
     }));
-    this.parent.div.append(popup.render());
+    if (!this.parent._commentManager) {
+      this.parent.div.append(popup.render());
+    }
   }
 
   get hasPopupElement() {
     return !!(this.#popupElement || this.popup || this.data.popupRef);
   }
 
+  get extraPopupElement() {
+    return this.#popupElement;
+  }
+
   /**
    * Render the annotation's HTML element(s).
    *
@@ -2410,10 +2413,12 @@ class PopupElement {
     // consistent with other viewers such as Adobe Acrobat.
     this.#dateObj = PDFDateString.toDateObject(modificationDate);
 
+    // The elements that will trigger the popup.
+    this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
+
     if (commentManager) {
-      this.#renderCommentButton();
+      this.renderCommentButton();
     } else {
-      this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
       this.#addEventListeners();
 
       this.#container.hidden = true;
@@ -2443,8 +2448,8 @@ class PopupElement {
     // Attach the event listeners to the trigger element.
     for (const element of this.trigger) {
       element.addEventListener("click", this.#boundToggle, { signal });
-      element.addEventListener("mouseenter", this.#boundShow, { signal });
-      element.addEventListener("mouseleave", this.#boundHide, { signal });
+      element.addEventListener("pointerenter", this.#boundShow, { signal });
+      element.addEventListener("pointerleave", this.#boundHide, { signal });
       element.classList.add("popupTriggerArea");
     }
 
@@ -2466,7 +2471,7 @@ class PopupElement {
     );
   }
 
-  #renderCommentButton() {
+  renderCommentButton() {
     if (this.#commentButton) {
       return;
     }
@@ -2479,61 +2484,78 @@ class PopupElement {
       return;
     }
 
-    const button = (this.#commentButton = document.createElement("button"));
-    button.className = "annotationCommentButton";
-    const parentContainer = this.#firstElement.container;
-    button.style.zIndex = parentContainer.style.zIndex + 1;
-    button.tabIndex = 0;
-    button.ariaHasPopup = "dialog";
-    button.ariaControls = "commentPopup";
-    button.setAttribute("data-l10n-id", "pdfjs-show-comment-button");
-
     const { signal } = (this.#popupAbortController = new AbortController());
-    button.addEventListener("keydown", this.#boundKeyDown, { signal });
-    button.addEventListener(
-      "click",
-      () => {
-        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 }
-    );
-    this.#updateColor();
-    this.#updateCommentButtonPosition();
-    parentContainer.after(button);
+    const hasOwnButton = !!this.#firstElement.extraPopupElement;
+    const togglePopup = () => {
+      this.#commentManager.toggleCommentPopup(
+        this,
+        /* isSelected = */ true,
+        /* visibility = */ undefined,
+        /* isEditable = */ !hasOwnButton
+      );
+    };
+    const showPopup = () => {
+      this.#commentManager.toggleCommentPopup(
+        this,
+        /* isSelected = */ false,
+        /* visibility = */ true,
+        /* isEditable = */ !hasOwnButton
+      );
+    };
+    const hidePopup = () => {
+      this.#commentManager.toggleCommentPopup(
+        this,
+        /* isSelected = */ false,
+        /* visibility = */ false
+      );
+    };
+
+    if (!hasOwnButton) {
+      const button = (this.#commentButton = document.createElement("button"));
+      button.className = "annotationCommentButton";
+      const parentContainer = this.#firstElement.container;
+      button.style.zIndex = parentContainer.style.zIndex + 1;
+      button.tabIndex = 0;
+      button.ariaHasPopup = "dialog";
+      button.ariaControls = "commentPopup";
+      button.setAttribute("data-l10n-id", "pdfjs-show-comment-button");
+      this.#updateColor();
+      this.#updateCommentButtonPosition();
+      button.addEventListener("keydown", this.#boundKeyDown, { signal });
+      button.addEventListener("click", togglePopup, { signal });
+      button.addEventListener("pointerenter", showPopup, { signal });
+      button.addEventListener("pointerleave", hidePopup, { signal });
+      parentContainer.after(button);
+    } else {
+      this.#commentButton = this.#firstElement.container;
+      for (const element of this.trigger) {
+        element.ariaHasPopup = "dialog";
+        element.ariaControls = "commentPopup";
+        element.addEventListener("keydown", this.#boundKeyDown, { signal });
+        element.addEventListener("click", togglePopup, { signal });
+        element.addEventListener("pointerenter", showPopup, { signal });
+        element.addEventListener("pointerleave", hidePopup, { signal });
+        element.classList.add("popupTriggerArea");
+      }
+    }
   }
 
   #updateCommentButtonPosition() {
-    this.#renderCommentButton();
+    if (this.#firstElement.extraPopupElement) {
+      return;
+    }
+    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();
+    if (this.#firstElement.extraPopupElement) {
+      return;
+    }
+    this.renderCommentButton();
     this.#commentButton.style.backgroundColor = this.commentButtonColor || "";
   }
 
@@ -3792,6 +3814,7 @@ class AnnotationLayer {
         rendered.style.visibility = "hidden";
       }
       await this.#appendElement(rendered, data.id, elementParams.elements);
+      element.extraPopupElement?.popup?.renderCommentButton();
 
       if (element._isEditable) {
         this.#editableAnnotations.set(element.data.id, element);

test/integration/annotation_spec.mjs

@@ -810,4 +810,72 @@ a dynamic compiler for JavaScript based on our`);
       });
     });
   });
+
+  describe("Annotation without popup and enableComment set to true", () => {
+    describe("annotation-text-without-popup.pdf", () => {
+      let pages;
+
+      beforeEach(async () => {
+        pages = await loadAndWait(
+          "annotation-text-without-popup.pdf",
+          getAnnotationSelector("4R"),
+          "page-fit",
+          null,
+          { enableComment: true }
+        );
+      });
+
+      afterEach(async () => {
+        await closePages(pages);
+      });
+
+      it("must check that the popup is shown", async () => {
+        await Promise.all(
+          pages.map(async ([browserName, page]) => {
+            const rect = await getRect(page, getAnnotationSelector("4R"));
+
+            // Hover the annotation, the popup should be visible.
+            let promisePopup = page.waitForSelector("#commentPopup", {
+              visible: true,
+            });
+            await page.mouse.move(
+              rect.x + rect.width / 2,
+              rect.y + rect.height / 2
+            );
+            await promisePopup;
+
+            // Move the mouse away, the popup should be hidden.
+            promisePopup = page.waitForSelector("#commentPopup", {
+              visible: false,
+            });
+            await page.mouse.move(
+              rect.x - rect.width / 2,
+              rect.y - rect.height / 2
+            );
+            await promisePopup;
+
+            // Click the annotation, the popup should be visible.
+            promisePopup = page.waitForSelector("#commentPopup", {
+              visible: true,
+            });
+            await page.mouse.click(
+              rect.x + rect.width / 2,
+              rect.y + rect.height / 2
+            );
+            await promisePopup;
+
+            // Click again, the popup should be hidden.
+            promisePopup = page.waitForSelector("#commentPopup", {
+              visible: false,
+            });
+            await page.mouse.click(
+              rect.x + rect.width / 2,
+              rect.y + rect.height / 2
+            );
+            await promisePopup;
+          })
+        );
+      });
+    });
+  });
 });

web/comment_manager.js

@@ -92,11 +92,11 @@ class CommentManager {
     this.#sidebar.updateComment(annotation);
   }
 
-  toggleCommentPopup(editor, isSelected, visibility) {
+  toggleCommentPopup(editor, isSelected, visibility, isEditable) {
     if (isSelected) {
       this.selectComment(editor.uid);
     }
-    this.#popup.toggle(editor, isSelected, visibility);
+    this.#popup.toggle(editor, isSelected, visibility, isEditable);
   }
 
   destroyPopup() {
@@ -880,6 +880,8 @@ class CommentDialog {
 }
 
 class CommentPopup {
+  #buttonsContainer = null;
+
   #commentDialog;
 
   #dateFormat;
@@ -954,7 +956,7 @@ class CommentPopup {
     const time = (this.#time = document.createElement("time"));
     time.className = "commentPopupTime";
 
-    const buttons = document.createElement("div");
+    const buttons = (this.#buttonsContainer = document.createElement("div"));
     buttons.className = "commentPopupButtons";
     const edit = document.createElement("button");
     edit.classList.add("commentPopupEdit", "toolbarButton");
@@ -1089,7 +1091,7 @@ class CommentPopup {
     this.sidebar.selectComment(null);
   }
 
-  toggle(editor, isSelected, visibility = undefined) {
+  toggle(editor, isSelected, visibility = undefined, isEditable = true) {
     if (!editor) {
       this.destroy();
       return;
@@ -1119,6 +1121,7 @@ class CommentPopup {
     }
 
     const container = this.#createPopup();
+    this.#buttonsContainer.classList.toggle("hidden", !isEditable);
     container.classList.toggle("hidden", false);
     container.classList.toggle("selected", isSelected);
     this.#selected = isSelected;