Merge pull request #18889 from calixteman/rm_useless_aria_has_popup

Don't add attribute 'aria-haspopup' when no popup is rendered for an annotation
mozilla · Oct 12, 2024 · e28dd8e · e28dd8e
2 parents c3af342 + 7c43cb2
commit e28dd8e
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 12 deletions.
diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
@@ -285,10 +285,6 @@ class AnnotationElement {
     // use of the z-index.
     style.zIndex = this.parent.zIndex++;
 
-    if (data.popupRef) {
-      container.setAttribute("aria-haspopup", "dialog");
-    }
-
     if (data.alternativeText) {
       container.title = data.alternativeText;
     }
@@ -624,8 +620,7 @@ class AnnotationElement {
    * @memberof AnnotationElement
    */
   _createPopup() {
-    const { container, data } = this;
-    container.setAttribute("aria-haspopup", "dialog");
+    const { data } = this;
 
     const popup = (this.#popupElement = new PopupAnnotationElement({
       data: {
@@ -2091,6 +2086,7 @@ class PopupAnnotationElement extends AnnotationElement {
     const elementIds = [];
     for (const element of this.elements) {
       element.popup = popup;
+      element.container.ariaHasPopup = "dialog";
       elementIds.push(element.data.id);
       element.addHighlightArea();
     }
@@ -2853,15 +2849,13 @@ class InkAnnotationElement extends AnnotationElement {
       polyline.setAttribute("stroke", "transparent");
       polyline.setAttribute("fill", "transparent");
 
-      // Create the popup ourselves so that we can bind it to the polyline
-      // instead of to the entire container (which is the default).
-      if (!popupRef && this.hasPopupData) {
-        this._createPopup();
-      }
-
       svg.append(polyline);
     }
 
+    if (!popupRef && this.hasPopupData) {
+      this._createPopup();
+    }
+
     this.container.append(svg);
     this._editOnDoubleClick();
 

diff --git a/test/integration/annotation_spec.mjs b/test/integration/annotation_spec.mjs
@@ -624,4 +624,34 @@ describe("ResetForm action", () => {
       });
     });
   });
+
+  describe("Annotation with empty popup and aria", () => {
+    describe("issue14438.pdf", () => {
+      let pages;
+
+      beforeAll(async () => {
+        pages = await loadAndWait(
+          "highlights.pdf",
+          "[data-annotation-id='693R']"
+        );
+      });
+
+      afterAll(async () => {
+        await closePages(pages);
+      });
+
+      it("must check that the highlight annotation has no popup and no aria-haspopup attribute", async () => {
+        await Promise.all(
+          pages.map(async ([browserName, page]) => {
+            await page.waitForFunction(
+              // No aria-haspopup attribute,
+              `document.querySelector("[data-annotation-id='693R']").ariaHasPopup === null` +
+                // and no popup.
+                `&& document.querySelector("[data-annotation-id='694R']") === null`
+            );
+          })
+        );
+      });
+    });
+  });
 });