From a00b542f2f58cde6117fa0985a5a2416051b0225 Mon Sep 17 00:00:00 2001
From: Calixte Denizet <calixte.denizet@gmail.com>
Date: Mon, 4 Sep 2023 17:29:50 +0200
Subject: [PATCH] Unconditionally render non-form annotations in the annotation
 layer (bug 1851498)

The goal is to always have something which is focusable to let the user select
it with the keyboard.
It fixes the mentioned bug because, the annotation layer will now have a container
to attach the canvas for annotations having their own canvas.
---
 src/display/annotation_layer.js  | 161 ++++++++-----------------------
 test/pdfs/.gitignore             |   1 +
 test/pdfs/bug1851498.pdf         | Bin 0 -> 5937 bytes
 test/test_manifest.json          |   8 ++
 web/annotation_layer_builder.css |  15 +--
 5 files changed, 51 insertions(+), 134 deletions(-)
 create mode 100755 test/pdfs/bug1851498.pdf

diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js
index 4e6a5ec97..8a8ecb5af 100644
--- a/src/display/annotation_layer.js
+++ b/src/display/annotation_layer.js
@@ -190,6 +190,14 @@ class AnnotationElement {
     }
   }
 
+  static _hasPopupData({ titleObj, contentsObj, richText }) {
+    return !!(titleObj?.str || contentsObj?.str || richText?.str);
+  }
+
+  get hasPopupData() {
+    return AnnotationElement._hasPopupData(this.data);
+  }
+
   /**
    * Create an empty container for the annotation's HTML element.
    *
@@ -956,13 +964,7 @@ class LinkAnnotationElement extends AnnotationElement {
 
 class TextAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable });
+    super(parameters, { isRenderable: true });
   }
 
   render() {
@@ -978,7 +980,7 @@ class TextAnnotationElement extends AnnotationElement {
     image.dataset.l10nId = "text_annotation_type";
     image.dataset.l10nArgs = JSON.stringify({ type: this.data.name });
 
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -1927,12 +1929,7 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement {
 class PopupAnnotationElement extends AnnotationElement {
   constructor(parameters) {
     const { data, elements } = parameters;
-    const isRenderable = !!(
-      data.titleObj?.str ||
-      data.contentsObj?.str ||
-      data.richText?.str
-    );
-    super(parameters, { isRenderable });
+    super(parameters, { isRenderable: AnnotationElement._hasPopupData(data) });
     this.elements = elements;
   }
 
@@ -2044,9 +2041,7 @@ class PopupElement {
       element.addEventListener("click", this.#boundToggle);
       element.addEventListener("mouseenter", this.#boundShow);
       element.addEventListener("mouseleave", this.#boundHide);
-      if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("TESTING")) {
-        element.classList.add("popupTriggerArea");
-      }
+      element.classList.add("popupTriggerArea");
     }
 
     // Attach the event listener to toggle the popup with the keyboard.
@@ -2275,13 +2270,7 @@ class PopupElement {
 
 class FreeTextAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
     this.textContent = parameters.data.textContent;
     this.textPosition = parameters.data.textPosition;
     this.annotationEditorType = AnnotationEditorType.FREETEXT;
@@ -2302,7 +2291,7 @@ class FreeTextAnnotationElement extends AnnotationElement {
       this.container.append(content);
     }
 
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2316,13 +2305,7 @@ class LineAnnotationElement extends AnnotationElement {
   #line = null;
 
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
   }
 
   render() {
@@ -2357,7 +2340,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) {
+    if (!data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2377,13 +2360,7 @@ class SquareAnnotationElement extends AnnotationElement {
   #square = null;
 
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
   }
 
   render() {
@@ -2420,7 +2397,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) {
+    if (!data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2440,13 +2417,7 @@ class CircleAnnotationElement extends AnnotationElement {
   #circle = null;
 
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
   }
 
   render() {
@@ -2484,7 +2455,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) {
+    if (!data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2504,13 +2475,7 @@ class PolylineAnnotationElement extends AnnotationElement {
   #polyline = null;
 
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
 
     this.containerClassName = "polylineAnnotation";
     this.svgElementName = "svg:polyline";
@@ -2557,8 +2522,8 @@ 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 (!data.popupRef) {
-      this._createPopup(polyline, data);
+    if (!data.popupRef && this.hasPopupData) {
+      this._createPopup();
     }
 
     return this.container;
@@ -2585,19 +2550,13 @@ class PolygonAnnotationElement extends PolylineAnnotationElement {
 
 class CaretAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
   }
 
   render() {
     this.container.classList.add("caretAnnotation");
 
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
     return this.container;
@@ -2608,13 +2567,7 @@ class InkAnnotationElement extends AnnotationElement {
   #polylines = [];
 
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
 
     this.containerClassName = "inkAnnotation";
 
@@ -2661,8 +2614,8 @@ 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).
-      if (!data.popupRef) {
-        this._createPopup(polyline, data);
+      if (!data.popupRef && this.hasPopupData) {
+        this._createPopup();
       }
 
       svg.append(polyline);
@@ -2683,21 +2636,15 @@ class InkAnnotationElement extends AnnotationElement {
 
 class HighlightAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
     super(parameters, {
-      isRenderable,
+      isRenderable: true,
       ignoreBorder: true,
       createQuadrilaterals: true,
     });
   }
 
   render() {
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2708,21 +2655,15 @@ class HighlightAnnotationElement extends AnnotationElement {
 
 class UnderlineAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
     super(parameters, {
-      isRenderable,
+      isRenderable: true,
       ignoreBorder: true,
       createQuadrilaterals: true,
     });
   }
 
   render() {
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2733,21 +2674,15 @@ class UnderlineAnnotationElement extends AnnotationElement {
 
 class SquigglyAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
     super(parameters, {
-      isRenderable,
+      isRenderable: true,
       ignoreBorder: true,
       createQuadrilaterals: true,
     });
   }
 
   render() {
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2758,21 +2693,15 @@ class SquigglyAnnotationElement extends AnnotationElement {
 
 class StrikeOutAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
     super(parameters, {
-      isRenderable,
+      isRenderable: true,
       ignoreBorder: true,
       createQuadrilaterals: true,
     });
   }
 
   render() {
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
 
@@ -2783,19 +2712,13 @@ class StrikeOutAnnotationElement extends AnnotationElement {
 
 class StampAnnotationElement extends AnnotationElement {
   constructor(parameters) {
-    const isRenderable = !!(
-      parameters.data.popupRef ||
-      parameters.data.titleObj?.str ||
-      parameters.data.contentsObj?.str ||
-      parameters.data.richText?.str
-    );
-    super(parameters, { isRenderable, ignoreBorder: true });
+    super(parameters, { isRenderable: true, ignoreBorder: true });
   }
 
   render() {
     this.container.classList.add("stampAnnotation");
 
-    if (!this.data.popupRef) {
+    if (!this.data.popupRef && this.hasPopupData) {
       this._createPopup();
     }
     return this.container;
@@ -2847,15 +2770,13 @@ class FileAttachmentAnnotationElement extends AnnotationElement {
         }
       }
     }
-    trigger.classList.add("popupTriggerArea");
     trigger.addEventListener("dblclick", this._download.bind(this));
     this.#trigger = trigger;
 
-    if (
-      !data.popupRef &&
-      (data.titleObj?.str || data.contentsObj?.str || data.richText)
-    ) {
+    if (!data.popupRef && this.hasPopupData) {
       this._createPopup();
+    } else {
+      trigger.classList.add("popupTriggerArea");
     }
 
     this.container.append(trigger);
diff --git a/test/pdfs/.gitignore b/test/pdfs/.gitignore
index 43c66b754..e33e13501 100644
--- a/test/pdfs/.gitignore
+++ b/test/pdfs/.gitignore
@@ -611,3 +611,4 @@
 !widget_hidden_print.pdf
 !empty_protected.pdf
 !tagged_stamp.pdf
+!bug1851498.pdf
diff --git a/test/pdfs/bug1851498.pdf b/test/pdfs/bug1851498.pdf
new file mode 100755
index 0000000000000000000000000000000000000000..1140fd507a2f13da50d151c0f3f0d336fdb44b0c
GIT binary patch
literal 5937
zcmeHLe{3699Z%M->uwDy+Em6Qq=&n*gvNgF-7ot*$E{<>YD(O=$w3OCBKOOwTl>zO
zFO6Fb(IO^Rv`*UuVnzL9s-Q4Uu(a?ORDz9F(9*6-DIi2CF!m4orwC07P`2@XXP<pd
z-HA$!zjBhF@80|V{Jifw`}^5c%;lprCiiW6{V$h()yGka2y*njeW?^4E8CXlmJeDc
zqIe963QzDjq5>X4aaqJWU{*z<OyhVPnnt2PVU6xir~541WF}D83w($jA34i#OslB5
z)jD|0I+eMqT}J{gx|UfsJjX?pPp5IdT=6Uy=PR0LHC6CvZaO6rB1l4B$&6$rB_k#h
z8Jbb}Y$iWEoE0RZq*HlW5ELa(Np@HnQAC=E^CP^H$z}OrLh>1s-i=3X(`rmZ;u5<r
zIfxR7Zvu~5_N=!D34{f+pCy{W6OQ9SME?3DAcLrk)dMvP@0zemcxsv=SOW;^o@;5<
zK5ox7T{qXy=ra?#KJzzC|HsnNBZn6sLO<>KAi3*I9|uUaL|d5EJ<atRu2t&eNFTRp
z)5v(fj|*@F=%fyG-7rD%co4T3hs}~Ho<xFtQ$RB_GpkShaUClD{LsC-9!xylc(S-|
z>b^I~+lRSpt2WQB7Z3marQX#K_MG{{-D|o;=x5)mkb>oDrsioZYd~y#&Z|Jp)RF8z
z7PVQpSNwY)q%4S5oY_u=P@$7f)^%#bPGg$H0Dkz$l!q~{z_=S1pc=H>u0r`(w%77B
zVbz^E*RY_Tpl~J}4@w4+<}R^_Lf$*hyI`3wyLH%WF~F1Z2`G7?X=q&EJLy%!u=10n
zNT$dsb)mQ*ern;=;{0p);^v*?%KR?}KOXDd{^9=Bf4%YWE5|<i^}ApA*E`?dq#Qf&
z!rA@%xAr}?^?T^pp;u3+&+ibPIDO&S<Lc2vufC-|e)__d6K|=7h0lCrPu5s?EN4FO
z3;h!tA3t;X`0-tD&wOF&-%mgG=%a^vR`p!{*&~Zx_K9staFAP-%oBnq=XB36*uq#d
zm0%A$$)vV4YMQan@{n%LmhGX)^&kHvg39JlWJ)TKLM?0UDQ`RIT9XHNsK&v4hGIsx
zq`6ch*{Ig4P!Xt6t=RQsV<@7*d%#M<J)6c6<coOwh9ViZfOZv%DC@WuQere}5JF+Q
zW4uPpgsBW7p6~)D0;Xb=(j--qq8uEANSd9Alxvof6S;iLA3P03_IO?`iShjWd~9Bb
zIqoc`ilSh`W1f$KM6`au_OwRSu5WA_3GC#oy5W{<UfHn`v#aUOoHrDSv=1Ni-Ab%l
zYdf;*F+a(e;Z$)$t6>@=xSNt`v~_B8Zp9DHG_X~%s!+Ri(4$@T0MkODt=iRHwc1s)
z7Q*8uJy-yD0&WAqFaSTvf@79T2Xb&=(hU44k)u>riINm(QWQ25K1oOlFO%#5<lD%?
zix1w>U2X76*hWZlnT?<%yIWZnN$VBl%PzCydvILUaVqI%#bpfF(KQbhT?h5cVpK1C
z1Be%5G{zGe7n{k2)j}3cBUy6Xs^+D&TCGwxG*&AZz)a!JQb^w4xn#~U=GYF8=F)R>
zWiwgQO9@lg<fut4BMLv7R#c;w6txUdCsg4zg~)ywg+f^O7Q3T1{8ZS671&iMC5pty
zqh^U%QISv~su10XY9(1Th-sOUs0VgKx*@xeNfK}|=M2mJZ{fO0r)4XJ+4%{br{-bx
zXW_KMn*z6A75)-CS?}xBN%DPkv*P5Az+HiB1+F_n;Ese>s%r(VJ3`=&gjcHTe+C!V
zX*Qq(9*WFEE77Hy3H6D$ZPnVHny56n3D)M^+NhmYNio!(Exk52@RTgB;ntje^DF9x
zZ@qcX!zaJ9;rG8|9q^x|`(El2Oo+a?Z7@=&ZKUrN4KEg(;l#EV2G3o%R39B2{dMgW
z5C6YqB8b9*D2u1B$Ydf;iogBxchBR?wXt*K+do+wi=7|eKDcvj|M{sW7KyM}I6rt~
zQ6>CugewA1#Q5TsrHkV)>nD!TtGVZ|eeLoa>gz{;_=8nzNzeLo<a1rJ3NZ`aNg46~
zOUm3r#E9MK7oeb)(JzdF3ezt=cYQ2>a?850bx$rGT7CV~>tFm(?z5}5pSke?lM?dE
z+LK@0`lV-jfsnbh_MVS)fh9!9P%5`6nSi<AQYnO6)CHPm#1ZfojJ4Q}+N8zu`7m99
zaGS3{jIe;`7Gr_@%n)Xr1;QEIcDy>8rg3mF0Vy)-2-<s&GOmT)ut0{jx|Mf;3c=f~
z%6`j(CM7V_oNjgh%}I*zhz;~x*`7sss%&TMdO4T~Iba?G2Mp|1FesoB5YD@nrCJSe
z>$gtO7O`#&_2-hTjU);Yp*ew;WPoLBc#wsn#1cqS;xPg7k`&_=fK7}g0JK~N_50B?
z8cr>N?^#gGBN4K5T+?!!Xc7?ZAeAWsc!-^|1cN6HR~sKT6NI0K{>@JTQg1`SCIggq
zjq7_^gm@UpruU39yloI41D%Dya0gN_hysd0^V~8s1ycl36m8}n^^=DrS}Y?KMcDji
z-EyDj0%@jWRJjgbJg^yxtcVF-<bko2VqyX&1U?~C7!ZxeBm$Q}BpR3E2`PbqUYSBR
zH>`31C45vPh?3#kGA$E8AE_qlhXg7PnsVHVIe^r?+=$BU?9h~LR2i!mpz0H$Jb_YW
zJgEZMHp_%@qHLGilqm}HtLy=YSGJvuIA(*)hBzA_6GRn60ie+wn(p7aYVx1oKl{yX
zUw!D=^Eb{&_YR=`-i;%(M_#;m{hg~ndTzhUuO9%}DTXtT*x_rlw;fn}dV1e|xA*=8
geBb{v3=c%u@Mjn{#Cv*fO|pITIY~~$n`A8aA0rx(4*&oF

literal 0
HcmV?d00001

diff --git a/test/test_manifest.json b/test/test_manifest.json
index 1692017b4..a401d84ef 100644
--- a/test/test_manifest.json
+++ b/test/test_manifest.json
@@ -8126,5 +8126,13 @@
       "md5": "af8abe281721f92a0d46646969f061de",
       "link": true,
       "type": "other"
+   },
+   {
+      "id": "bug1851498",
+      "file": "pdfs/bug1851498.pdf",
+      "md5": "787f092f449016a649c6c9343042429a",
+      "rounds": 1,
+      "type": "eq",
+      "annotations": true
    }
 ]
diff --git a/web/annotation_layer_builder.css b/web/annotation_layer_builder.css
index 4a5b4f1f1..ecdb2cff5 100644
--- a/web/annotation_layer_builder.css
+++ b/web/annotation_layer_builder.css
@@ -337,20 +337,7 @@
   font-size: calc(9px * var(--scale-factor));
 }
 
-.annotationLayer .highlightAnnotation,
-.annotationLayer .underlineAnnotation,
-.annotationLayer .squigglyAnnotation,
-.annotationLayer .strikeoutAnnotation,
-.annotationLayer .freeTextAnnotation,
-.annotationLayer .lineAnnotation svg line,
-.annotationLayer .squareAnnotation svg rect,
-.annotationLayer .circleAnnotation svg ellipse,
-.annotationLayer .polylineAnnotation svg polyline,
-.annotationLayer .polygonAnnotation svg polygon,
-.annotationLayer .caretAnnotation,
-.annotationLayer .inkAnnotation svg polyline,
-.annotationLayer .stampAnnotation,
-.annotationLayer .fileAttachmentAnnotation {
+.annotationLayer .popupTriggerArea {
   cursor: pointer;
 }