Skip to content

Commit

Permalink
Fix popup for highlights without popup (follow-up of #12505)
Browse files Browse the repository at this point in the history
 * 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 #12504
  • Loading branch information
calixteman committed Nov 5, 2020
1 parent 8b652d6 commit 47e3fdd
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 19 deletions.
51 changes: 32 additions & 19 deletions src/display/annotation_layer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -790,6 +797,7 @@ class PopupAnnotationElement extends AnnotationElement {
constructor(parameters) {
const isRenderable = !!(parameters.data.title || parameters.data.contents);
super(parameters, isRenderable);
this.popup = true;
}

/**
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down
1 change: 1 addition & 0 deletions test/pdfs/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@
!noembed-identity.pdf
!noembed-identity-2.pdf
!noembed-jis7.pdf
!issue12504.pdf
!noembed-eucjp.pdf
!noembed-sjis.pdf
!vertical.pdf
Expand Down
Binary file added test/pdfs/issue12504.pdf
Binary file not shown.
7 changes: 7 additions & 0 deletions test/test_manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 47e3fdd

Please sign in to comment.