From 0ecf6458eec6a45ce7ffa14711ef13d3f21c539f Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 29 Aug 2022 16:12:20 +0200 Subject: [PATCH] Properly ignore PopupAnnotations with custom `trigger`-elements A number of Annotation-types are currently creating their own PopupAnnotations, since they need to use a custom `trigger`-element. However, because of where that check is currently implemented[1] we end up attaching empty/unused containers for those PopupAnnotations to the DOM[2]; see e.g. the `annotation-line.pdf` file in the test-suite for one example. By instead moving the types-check into the `PopupAnnotationElement` constructor, we can completely skip those PopupAnnotations that are being explicitly handled elsewhere. Note that I don't *believe* that this is a new issue, although I've not tried to bisect it, but this likely goes back quite some time (possibly even as far as PR 8228). --- [1] In the `PopupAnnotationElement.render` method. [2] Please note that the actual Popup-element *itself* isn't being attached/rendered here, just its container which by itself serves no purpose as far as I can tell. --- src/display/annotation_layer.js | 40 +++++++++++++++------------------ 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index bf3b3e793..91cc8a1c9 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -1736,35 +1736,31 @@ class ChoiceWidgetAnnotationElement extends WidgetAnnotationElement { } class PopupAnnotationElement extends AnnotationElement { + // Do not render popup annotations for parent elements with these types as + // they create the popups themselves (because of custom trigger divs). + static IGNORE_TYPES = new Set([ + "Line", + "Square", + "Circle", + "PolyLine", + "Polygon", + "Ink", + ]); + constructor(parameters) { - const isRenderable = !!( - parameters.data.titleObj?.str || - parameters.data.contentsObj?.str || - parameters.data.richText?.str - ); + const { data } = parameters; + const isRenderable = + !PopupAnnotationElement.IGNORE_TYPES.has(data.parentType) && + !!(data.titleObj?.str || data.contentsObj?.str || data.richText?.str); super(parameters, { isRenderable }); } render() { - // Do not render popup annotations for parent elements with these types as - // they create the popups themselves (because of custom trigger divs). - const IGNORE_TYPES = [ - "Line", - "Square", - "Circle", - "PolyLine", - "Polygon", - "Ink", - ]; - this.container.className = "popupAnnotation"; - if (IGNORE_TYPES.includes(this.data.parentType)) { - return this.container; - } - - const selector = `[data-annotation-id="${this.data.parentId}"]`; - const parentElements = this.layer.querySelectorAll(selector); + const parentElements = this.layer.querySelectorAll( + `[data-annotation-id="${this.data.parentId}"]` + ); if (parentElements.length === 0) { return this.container; }