Fix popup for highlights without popup (follow-up of mozilla#12505)

 * remove 1st param of _createPopup (almost useless for a method)
 * prepend popup div to avoid to have them on top of some highlights (and so "disable" partially mouse events)
 * add a ref test for issue mozilla#12504

Author: calixteman

src/display/annotation_layer.js

@@ -262,7 +262,9 @@ class AnnotationElement {
         quadPoint[1].y,
       ];
       this.data.rect = rect;
-      quadrilaterals.push(this._createContainer(ignoreBorder));
+      const quad = this._createContainer(ignoreBorder);
+      quad.className = "highlightAnnotation";
+      quadrilaterals.push(quad);
     }
     this.data.rect = savedRect;
     return quadrilaterals;
@@ -274,12 +276,17 @@ class AnnotationElement {
    * are of a type that works with popups (such as Highlight annotations).
    *
    * @private
-   * @param {HTMLSectionElement} container
    * @param {HTMLDivElement|HTMLImageElement|null} trigger
    * @param {Object} data
    * @memberof AnnotationElement
    */
-  _createPopup(container, trigger, data) {
+  _createPopup(trigger, data) {
+    let container = this.container;
+    if (this.quadrilaterals) {
+      trigger = trigger || this.quadrilaterals;
+      container = this.quadrilaterals[0];
+    }
+
     // If no trigger element is specified, create it.
     if (!trigger) {
       trigger = document.createElement("div");
@@ -433,7 +440,7 @@ class TextAnnotationElement extends AnnotationElement {
     image.dataset.l10nArgs = JSON.stringify({ type: this.data.name });
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, image, this.data);
+      this._createPopup(image, this.data);
     }
 
     this.container.appendChild(image);
@@ -790,6 +797,7 @@ class PopupAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const isRenderable = !!(parameters.data.title || parameters.data.contents);
     super(parameters, isRenderable);
+    this.popup = true;
   }
 
   /**
@@ -885,7 +893,6 @@ class PopupElement {
     // disappear too. In that special case, hiding the wrapper suffices.
     this.hideElement = this.hideWrapper ? wrapper : this.container;
     this.hideElement.setAttribute("hidden", true);
-
     const popup = document.createElement("div");
     popup.className = "popup";
 
@@ -1027,7 +1034,7 @@ class FreeTextAnnotationElement extends AnnotationElement {
     this.container.className = "freeTextAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1078,7 +1085,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).
-    this._createPopup(this.container, line, data);
+    this._createPopup(line, data);
 
     return this.container;
   }
@@ -1132,7 +1139,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).
-    this._createPopup(this.container, square, data);
+    this._createPopup(square, data);
 
     return this.container;
   }
@@ -1186,7 +1193,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).
-    this._createPopup(this.container, circle, data);
+    this._createPopup(circle, data);
 
     return this.container;
   }
@@ -1248,7 +1255,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).
-    this._createPopup(this.container, polyline, data);
+    this._createPopup(polyline, data);
 
     return this.container;
   }
@@ -1285,7 +1292,7 @@ class CaretAnnotationElement extends AnnotationElement {
     this.container.className = "caretAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1347,7 +1354,7 @@ class InkAnnotationElement 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).
-      this._createPopup(this.container, polyline, data);
+      this._createPopup(polyline, data);
 
       svg.appendChild(polyline);
     }
@@ -1379,9 +1386,10 @@ class HighlightAnnotationElement extends AnnotationElement {
     this.container.className = "highlightAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.quadrilaterals || this.container;
+    // return this.container;
   }
 }
 
@@ -1406,7 +1414,7 @@ class UnderlineAnnotationElement extends AnnotationElement {
     this.container.className = "underlineAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1433,7 +1441,7 @@ class SquigglyAnnotationElement extends AnnotationElement {
     this.container.className = "squigglyAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1460,7 +1468,7 @@ class StrikeOutAnnotationElement extends AnnotationElement {
     this.container.className = "strikeoutAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1487,7 +1495,7 @@ class StampAnnotationElement extends AnnotationElement {
     this.container.className = "stampAnnotation";
 
     if (!this.data.hasPopup) {
-      this._createPopup(this.container, null, this.data);
+      this._createPopup(null, this.data);
     }
     return this.container;
   }
@@ -1528,7 +1536,7 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
     trigger.addEventListener("dblclick", this._download.bind(this));
 
     if (!this.data.hasPopup && (this.data.title || this.data.contents)) {
-      this._createPopup(this.container, trigger, this.data);
+      this._createPopup(trigger, this.data);
     }
 
     this.container.appendChild(trigger);
@@ -1615,7 +1623,12 @@ class AnnotationLayer {
             parameters.div.appendChild(renderedElement);
           }
         } else {
-          parameters.div.appendChild(rendered);
+          if (element.popup) {
+            // popup div mustn't be on top of an annotation
+            parameters.div.prepend(rendered);
+          } else {
+            parameters.div.appendChild(rendered);
+          }
         }
       }
     }

test/pdfs/.gitignore

@@ -290,6 +290,7 @@
 !noembed-identity.pdf
 !noembed-identity-2.pdf
 !noembed-jis7.pdf
+!issue12504.pdf
 !noembed-eucjp.pdf
 !noembed-sjis.pdf
 !vertical.pdf

test/pdfs/issue12504.pdf

@@ -1427,6 +1427,13 @@
        "type": "eq",
        "about": "Type3 fonts with image resources; both pages need to be tested, otherwise the bug won't manifest."
     },
+    {  "id": "issue12504",
+       "file": "pdfs/issue12504.pdf",
+       "md5": "04fcc87f3e7e9e925e3ef83cf0bf49f4",
+       "rounds": 1,
+       "type": "eq",
+       "annotations": true
+    },
     {  "id": "close-path-bug",
        "file": "pdfs/close-path-bug.pdf",
        "md5": "48dd17ef58393857d2d038d33699cac5",