Properly ignore PopupAnnotations with custom trigger-elements
#15360
Conversation
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.
Snuffleupagus
force-pushed
the
Popup-IGNORE_TYPES
branch
from
06e7c85 to
0ecf645
Compare
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/ec97df15ec93020/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/ec97df15ec93020/output.txt Total script time: 2.18 mins Published |
|
/botio test |
From: Bot.io (Linux m4)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/f6a65d7beb831f3/output.txt |
From: Bot.io (Windows)ReceivedCommand cmd_test from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.193.163.58:8877/88837c1d94c6a0a/output.txt |
From: Bot.io (Linux m4)FailedFull output at http://54.241.84.105:8877/f6a65d7beb831f3/output.txt Total script time: 26.43 mins
Image differences available at: http://54.241.84.105:8877/f6a65d7beb831f3/reftest-analyzer.html#web=eq.log |
From: Bot.io (Windows)FailedFull output at http://54.193.163.58:8877/88837c1d94c6a0a/output.txt Total script time: 29.15 mins
|
|
Interesting find; thanks! |
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. theannotation-line.pdffile in the test-suite for one example.By instead moving the types-check into the
PopupAnnotationElementconstructor, 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.rendermethod.[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.