Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix highlight annotation regressions #12576

Closed
timvandermeij opened this issue Nov 4, 2020 · 0 comments · Fixed by #12585
Closed

Fix highlight annotation regressions #12576

timvandermeij opened this issue Nov 4, 2020 · 0 comments · Fixed by #12585

Comments

@timvandermeij
Copy link
Contributor

timvandermeij commented Nov 4, 2020

Unfortunately there was an oversight in PR #12505 where some regressions were missed:

  • The annotation-highlight-without-appearance, issue7014 and issue9552 reference tests no longer show the popup, and I can confirm that it also doesn't render in the viewer anymore. They most likely share the same root cause.
    • I have debugged this and found that, at least in the annotation-highlight-without-appearance case, the _createPopup line in HighlightAnnotationElement is triggered. The popup is in fact added to the container, but the annotation also has quadrilaterals and in that case those are returned/rendered and not the container. The problem is therefore limited to annotations without an explicit popup annotation, otherwise it works because that logic queries all quadrilateral elements. Therefore, the solution here seems to be to change _createPopup to also take the quadrilaterals into account, similar to the other code.
  • The quadrilaterals don't have the proper class name for highlight annotations anymore, causing them not to have a cursor pointer on hover.
    • I have found the cause of this to be on line 1379 in src/display/annotation_layer.js where the class name is only set on the container and not on all quadrilaterals (because we only had the container before).

Once this is fixed, please add the PDF file from #12504 to the test suite as well to ensure that that case is also still working.

/cc @calixteman

@timvandermeij timvandermeij changed the title Fix annotation-highlight-without-appearance reference test regression Fix reference test regressions Nov 4, 2020
@timvandermeij timvandermeij changed the title Fix reference test regressions Fix highlight annotation regressions Nov 4, 2020
@calixteman calixteman self-assigned this Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants