[Annotation] Use the annotations rect in order to fix the order in the DOM (bug 1987914)

It's just a partial fix for bug 1987914 but the time spent to add the annotations in the DOM
is divided by 5.

src/display/annotation_layer.js

@@ -184,6 +184,7 @@ class AnnotationElement {
     this.hasJSActions = parameters.hasJSActions;
     this._fieldObjects = parameters.fieldObjects;
     this.parent = parameters.parent;
+    this.hasOwnCommentButton = false;
 
     if (isRenderable) {
       this.contentElement = this.container =
@@ -747,7 +748,7 @@ class AnnotationElement {
       contentsObj = data.contentsObj;
       modificationDate = data.modificationDate;
     }
-    const popup = (this.#popupElement = new PopupAnnotationElement({
+    this.#popupElement = new PopupAnnotationElement({
       data: {
         color: data.color,
         titleObj: data.titleObj,
@@ -763,10 +764,7 @@ class AnnotationElement {
       linkService: this.linkService,
       parent: this.parent,
       elements: [this],
-    }));
+    });
-    if (!this.parent._commentManager) {
-      this.parent.div.append(popup.render());
-    }
   }
 
   get hasPopupElement() {
@@ -916,7 +914,6 @@ class EditorAnnotationElement extends AnnotationElement {
       return;
     }
     this._createPopup(editor.comment);
-    this.extraPopupElement.popup.renderCommentButton();
   }
 
   get hasCommentButton() {
@@ -943,6 +940,7 @@ class EditorAnnotationElement extends AnnotationElement {
   }
 
   remove() {
+    this.parent.removeAnnotation(this.data.id);
     this.container.remove();
     this.container = null;
     this.removePopup();
@@ -1274,6 +1272,7 @@ class TextAnnotationElement extends AnnotationElement {
     );
 
     if (!this.data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -2493,9 +2492,7 @@ class PopupElement {
     // The elements that will trigger the popup.
     this.trigger = elements.flatMap(e => e.getElementsToTriggerPopup());
 
-    if (commentManager) {
+    if (!commentManager) {
-      this.renderCommentButton();
-    } else {
       this.#addEventListeners();
 
       this.#container.hidden = true;
@@ -2550,6 +2547,9 @@ class PopupElement {
 
   renderCommentButton() {
     if (this.#commentButton) {
+      if (!this.#commentButton.parentNode) {
+        this.#firstElement.container.after(this.#commentButton);
+      }
       return;
     }
 
@@ -2562,7 +2562,7 @@ class PopupElement {
     }
 
     const { signal } = (this.#popupAbortController = new AbortController());
-    const hasOwnButton = !!this.#firstElement.extraPopupElement;
+    const hasOwnButton = this.#firstElement.hasOwnCommentButton;
     const togglePopup = () => {
       this.#commentManager.toggleCommentPopup(
         this,
@@ -2623,7 +2623,9 @@ class PopupElement {
       // button position can't be changed.
       return;
     }
+    if (!this.#commentButton) {
       this.renderCommentButton();
+    }
     const [x, y] = this.#commentButtonPosition;
     const { style } = this.#commentButton;
     style.left = `calc(${x}%)`;
@@ -2634,7 +2636,9 @@ class PopupElement {
     if (this.#firstElement.extraPopupElement) {
       return;
     }
+    if (!this.#commentButton) {
       this.renderCommentButton();
+    }
     this.#commentButton.style.backgroundColor = this.commentButtonColor || "";
   }
 
@@ -3085,6 +3089,7 @@ class FreeTextAnnotationElement extends AnnotationElement {
     }
 
     if (!this.data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3133,6 +3138,7 @@ class LineAnnotationElement extends AnnotationElement {
     // Create the popup ourselves so that we can bind it to the line instead
     // of to the entire container (which is the default).
     if (!data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3189,6 +3195,7 @@ class SquareAnnotationElement extends AnnotationElement {
     // Create the popup ourselves so that we can bind it to the square instead
     // of to the entire container (which is the default).
     if (!data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3246,6 +3253,7 @@ class CircleAnnotationElement extends AnnotationElement {
     // Create the popup ourselves so that we can bind it to the circle instead
     // of to the entire container (which is the default).
     if (!data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3319,6 +3327,7 @@ class PolylineAnnotationElement extends AnnotationElement {
     // 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.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3353,6 +3362,7 @@ class CaretAnnotationElement extends AnnotationElement {
     this.container.classList.add("caretAnnotation");
 
     if (!this.data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
     return this.container;
@@ -3447,6 +3457,7 @@ class InkAnnotationElement extends AnnotationElement {
     }
 
     if (!popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3503,6 +3514,7 @@ class HighlightAnnotationElement extends AnnotationElement {
       data: { overlaidText, popupRef },
     } = this;
     if (!popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3534,6 +3546,7 @@ class UnderlineAnnotationElement extends AnnotationElement {
       data: { overlaidText, popupRef },
     } = this;
     if (!popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3564,6 +3577,7 @@ class SquigglyAnnotationElement extends AnnotationElement {
       data: { overlaidText, popupRef },
     } = this;
     if (!popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3594,6 +3608,7 @@ class StrikeOutAnnotationElement extends AnnotationElement {
       data: { overlaidText, popupRef },
     } = this;
     if (!popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
 
@@ -3621,6 +3636,7 @@ class StampAnnotationElement extends AnnotationElement {
     this.container.setAttribute("role", "img");
 
     if (!this.data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     }
     this._editOnDoubleClick();
@@ -3684,6 +3700,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
     });
 
     if (!data.popupRef && this.hasPopupData) {
+      this.hasOwnCommentButton = true;
       this._createPopup();
     } else {
       trigger.classList.add("popupTriggerArea");
@@ -3748,6 +3765,10 @@ class AnnotationLayer {
 
   #linkService = null;
 
+  #elements = [];
+
+  #hasAriaAttributesFromStructTree = false;
+
   constructor({
     div,
     accessibilityManager,
@@ -3789,44 +3810,6 @@ class AnnotationLayer {
     return this.#editableAnnotations.size > 0;
   }
 
-  async #appendElement(element, id, popupElements) {
-    const { contentElement, container } = element;
-    const annotationId = (contentElement.id = `${AnnotationPrefix}${id}`);
-    const ariaAttributes =
-      await this.#structTreeLayer?.getAriaAttributes(annotationId);
-    if (ariaAttributes) {
-      for (const [key, value] of ariaAttributes) {
-        contentElement.setAttribute(key, value);
-      }
-    }
-
-    if (popupElements) {
-      // Set the popup just after the first element associated with the popup.
-      popupElements.at(-1).container.after(container);
-    } else {
-      this.#moveElementInDOM(container, contentElement);
-    }
-  }
-
-  #moveElementInDOM(container, contentElement) {
-    this.div.append(container);
-    this.#accessibilityManager?.moveElementInDOM(
-      this.div,
-      container,
-      contentElement,
-      /* isRemovable = */ false,
-      /* filter = */ node => node.nodeName === "SECTION",
-      /* inserter = */ (prevNode, node) => {
-        if (prevNode.nextElementSibling.nodeName === "BUTTON") {
-          // In case we have a comment button, insert after the button.
-          prevNode.nextElementSibling.after(node);
-        } else {
-          prevNode.after(node);
-        }
-      }
-    );
-  }
-
   /**
    * Render a new annotation layer with all annotation elements.
    *
@@ -3839,6 +3822,7 @@ class AnnotationLayer {
     setLayerDimensions(layer, this.viewport);
 
     const popupToElements = new Map();
+    const popupAnnotations = [];
     const elementParams = {
       data: null,
       layer,
@@ -3871,6 +3855,10 @@ class AnnotationLayer {
           // Ignore popup annotations without a corresponding annotation.
           continue;
         }
+        if (!this._commentManager) {
+          popupAnnotations.push(data);
+          continue;
+        }
         elementParams.elements = elements;
       }
       elementParams.data = data;
@@ -3880,7 +3868,10 @@ class AnnotationLayer {
         continue;
       }
 
-      if (!isPopupAnnotation && data.popupRef) {
+      if (!isPopupAnnotation) {
+        this.#elements.push(element);
+
+        if (data.popupRef) {
           const elements = popupToElements.get(data.popupRef);
           if (!elements) {
             popupToElements.set(data.popupRef, [element]);
@@ -3888,13 +3879,12 @@ class AnnotationLayer {
             elements.push(element);
           }
         }
+      }
 
       const rendered = element.render();
       if (data.hidden) {
         rendered.style.visibility = "hidden";
       }
-      await this.#appendElement(element, data.id, elementParams.elements);
-      element.extraPopupElement?.popup?.renderCommentButton();
 
       if (element._isEditable) {
         this.#editableAnnotations.set(element.data.id, element);
@@ -3902,9 +3892,124 @@ class AnnotationLayer {
       }
     }
 
+    await this.#addElementsToDOM();
+
+    for (const data of popupAnnotations) {
+      const elements = (elementParams.elements = popupToElements.get(data.id));
+      elementParams.data = data;
+      const element = AnnotationElementFactory.create(elementParams);
+      if (!element.isRenderable) {
+        continue;
+      }
+      const rendered = element.render();
+      element.contentElement.id = `${AnnotationPrefix}${data.id}`;
+      if (data.hidden) {
+        rendered.style.visibility = "hidden";
+      }
+      elements.at(-1).container.after(rendered);
+    }
+
     this.#setAnnotationCanvasMap();
   }
 
+  async #addElementsToDOM() {
+    if (this.#elements.length === 0) {
+      return;
+    }
+    // Clear the existing annotations in order to make sure comment buttons
+    // don't have a parent any longer.
+    this.div.replaceChildren();
+
+    const promises = [];
+    if (!this.#hasAriaAttributesFromStructTree) {
+      this.#hasAriaAttributesFromStructTree = true;
+      for (const {
+        contentElement,
+        data: { id },
+      } of this.#elements) {
+        const annotationId = (contentElement.id = `${AnnotationPrefix}${id}`);
+        promises.push(
+          this.#structTreeLayer
+            ?.getAriaAttributes(annotationId)
+            .then(ariaAttributes => {
+              if (ariaAttributes) {
+                for (const [key, value] of ariaAttributes) {
+                  contentElement.setAttribute(key, value);
+                }
+              }
+            })
+        );
+      }
+    }
+    this.#elements.sort(
+      (
+        {
+          data: {
+            rect: [a0, a1, a2, a3],
+          },
+        },
+        {
+          data: {
+            rect: [b0, b1, b2, b3],
+          },
+        }
+      ) => {
+        // We are in page coordinates, which has the origin at the
+        // bottom left.
+        if (a0 === a2 && a1 === a3) {
+          return +1;
+        }
+
+        if (b0 === b2 && b1 === b3) {
+          return -1;
+        }
+
+        const top1 = a3;
+        const bot1 = a1;
+        const mid1 = (a1 + a3) / 2;
+
+        const top2 = b3;
+        const bot2 = b1;
+        const mid2 = (b1 + b3) / 2;
+
+        if (mid1 >= top2 && mid2 <= bot1) {
+          return -1;
+        }
+
+        if (mid2 >= top1 && mid1 <= bot2) {
+          return +1;
+        }
+
+        const centerX1 = (a0 + a2) / 2;
+        const centerX2 = (b0 + b2) / 2;
+
+        return centerX1 - centerX2;
+      }
+    );
+
+    const fragment = document.createDocumentFragment();
+    for (const element of this.#elements) {
+      fragment.append(element.container);
+      if (this._commentManager) {
+        (
+          element.extraPopupElement?.popup || element.popup
+        )?.renderCommentButton();
+      } else if (element.extraPopupElement) {
+        fragment.append(element.extraPopupElement.render());
+      }
+    }
+    this.div.append(fragment);
+    await Promise.all(promises);
+    if (this.#accessibilityManager) {
+      for (const element of this.#elements) {
+        this.#accessibilityManager.addPointerInTextLayer(
+          element.contentElement,
+          /* isRemovable = */ false
+        );
+      }
+    }
+  }
+
   /**
    * Add link annotations to the annotation layer.
    *
@@ -3930,8 +4035,10 @@ class AnnotationLayer {
         continue;
       }
       element.render();
-      await this.#appendElement(element, data.id, null);
+      element.contentElement.id = `${AnnotationPrefix}${data.id}`;
+      this.#elements.push(element);
     }
+    await this.#addElementsToDOM();
   }
 
   /**
@@ -4015,30 +4122,36 @@ class AnnotationLayer {
       linkService: this.#linkService,
       annotationStorage: this.#annotationStorage,
     });
-    const rendered = element.render();
+    element.render();
-    rendered.id = `${AnnotationPrefix}${id}`;
+    element.contentElement.id = `${AnnotationPrefix}${id}`;
-    this.#moveElementInDOM(rendered, rendered);
     element.createOrUpdatePopup();
+    this.#elements.push(element);
     return element;
   }
 
-  togglePointerEvents(enabled = false) {
+  removeAnnotation(id) {
-    this.div.classList.toggle("disabled", !enabled);
+    const index = this.#elements.findIndex(el => el.data.id === id);
+    if (index < 0) {
+      return;
+    }
+    const [element] = this.#elements.splice(index, 1);
+    this.#accessibilityManager?.removePointerInTextLayer(
+      element.contentElement
+    );
   }
 
   updateFakeAnnotations(editors) {
     if (editors.length === 0) {
       return;
     }
-    // In order to ensure that the annotations are correctly moved in the DOM
-    // we need to make sure that this has been laid out.
-    window.requestAnimationFrame(() =>
-      setTimeout(() => {
     for (const editor of editors) {
       editor.updateFakeAnnotationElement(this);
     }
-      }, 10)
+    this.#addElementsToDOM();
-    );
+  }
+
+  togglePointerEvents(enabled = false) {
+    this.div.classList.toggle("disabled", !enabled);
   }
 
   /**

test/integration/accessibility_spec.mjs

@@ -181,7 +181,10 @@ describe("accessibility", () => {
     let pages;
 
     beforeEach(async () => {
-      pages = await loadAndWait("tagged_stamp.pdf", ".annotationLayer");
+      pages = await loadAndWait(
+        "tagged_stamp.pdf",
+        ".annotationLayer #pdfjs_internal_id_21R"
+      );
     });
 
     afterEach(async () => {

web/text_accessibility.js

@@ -222,14 +222,7 @@ class TextAccessibilityManager {
    * @param {HTMLDivElement} element
    * @returns {string|null} The id in the struct tree if any.
    */
-  moveElementInDOM(
+  moveElementInDOM(container, element, contentElement, isRemovable) {
-    container,
-    element,
-    contentElement,
-    isRemovable,
-    filter,
-    inserter
-  ) {
     const id = this.addPointerInTextLayer(contentElement, isRemovable);
 
     if (!container.hasChildNodes()) {
@@ -238,7 +231,7 @@ class TextAccessibilityManager {
     }
 
     const children = Array.from(container.childNodes).filter(
-      node => node !== element && (!filter || filter(node))
+      node => node !== element
     );
 
     if (children.length === 0) {
@@ -253,8 +246,6 @@ class TextAccessibilityManager {
 
     if (index === 0) {
       children[0].before(element);
-    } else if (inserter) {
-      inserter(children[index - 1], element);
     } else {
       children[index - 1].after(element);
     }