Merge pull request #20004 from calixteman/move_popup_dom

Move the popup annotation in the DOM just after the element triggering it

Author: calixteman

src/display/annotation_layer.js

@@ -2057,7 +2057,9 @@ class PopupAnnotationElement extends AnnotationElement {
   }
 
   render() {
-    this.container.classList.add("popupAnnotation");
+    const { container } = this;
+    container.classList.add("popupAnnotation");
+    container.role = "comment";
 
     const popup = (this.popup = new PopupElement({
       container: this.container,
@@ -2210,7 +2212,7 @@ class PopupElement {
     popup.append(header);
 
     if (this.#dateObj) {
-      const modificationDate = document.createElement("span");
+      const modificationDate = document.createElement("time");
       modificationDate.classList.add("popupDate");
       modificationDate.setAttribute(
         "data-l10n-id",
@@ -2220,6 +2222,7 @@ class PopupElement {
         "data-l10n-args",
         JSON.stringify({ dateObj: this.#dateObj.valueOf() })
       );
+      modificationDate.dateTime = this.#dateObj.toISOString();
       header.append(modificationDate);
     }
 
@@ -3142,7 +3145,7 @@ class AnnotationLayer {
     return this.#editableAnnotations.size > 0;
   }
 
-  async #appendElement(element, id) {
+  async #appendElement(element, id, popupElements) {
     const contentElement = element.firstChild || element;
     const annotationId = (contentElement.id = `${AnnotationPrefix}${id}`);
     const ariaAttributes =
@@ -3153,13 +3156,18 @@ class AnnotationLayer {
       }
     }
 
-    this.div.append(element);
-    this.#accessibilityManager?.moveElementInDOM(
-      this.div,
-      element,
-      contentElement,
-      /* isRemovable = */ false
-    );
+    if (popupElements) {
+      // Set the popup just after the first element associated with the popup.
+      popupElements.at(-1).container.after(element);
+    } else {
+      this.div.append(element);
+      this.#accessibilityManager?.moveElementInDOM(
+        this.div,
+        element,
+        contentElement,
+        /* isRemovable = */ false
+      );
+    }
   }
 
   /**
@@ -3227,7 +3235,7 @@ class AnnotationLayer {
       if (data.hidden) {
         rendered.style.visibility = "hidden";
       }
-      await this.#appendElement(rendered, data.id);
+      await this.#appendElement(rendered, data.id, elementParams.elements);
 
       if (element._isEditable) {
         this.#editableAnnotations.set(element.data.id, element);
@@ -3263,7 +3271,7 @@ class AnnotationLayer {
         continue;
       }
       const rendered = element.render();
-      await this.#appendElement(rendered, data.id);
+      await this.#appendElement(rendered, data.id, null);
     }
   }
 

test/integration/annotation_spec.mjs

@@ -36,6 +36,21 @@ describe("Annotation highlight", () => {
       await closePages(pages);
     });
 
+    it("must check the popup position in the DOM", async () => {
+      await Promise.all(
+        pages.map(async ([browserName, page]) => {
+          const areSiblings = await page.evaluate(() => {
+            const highlight = document.querySelector(
+              "[data-annotation-id='19R']"
+            );
+            const popup = document.querySelector("[data-annotation-id='21R']");
+            return highlight.nextElementSibling === popup;
+          });
+          expect(areSiblings).withContext(`In ${browserName}`).toEqual(true);
+        })
+      );
+    });
+
     it("must show a popup on mouseover", async () => {
       await Promise.all(
         pages.map(async ([browserName, page]) => {