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

Question about highlight annotation and others (#6827 and few other similar PRs) #6835

Closed
xlc opened this issue Jan 4, 2016 · 5 comments
Closed

Comments

@xlc
Copy link
Contributor

xlc commented Jan 4, 2016

It is good to see #6827 and few other PRs implemented few kind of annotations (highlight, underline, strikeout and squiggly). However I failed see how it improves anything?

The rendering is exactly the same (no need to improve). The new PRs only add "interactivity" (cursor: pointer;) which does nothing other than change cursor shape. The pointer cursor is normally used to give user a hint that they can interact with the element, which is not the case here. Clicking the elements doesn't do anything and user no longer able to drag to select/highlight the text anymore (It is still possible, but very hard).

I think we should mark those annotations are implemented but don't do create any HTML elements.
#5252 want's user able to toggle/interact with the highlights, in order to achieve this, we need to disable rendering highlight in canvas (ignore its appearance stream) and render the appearance stream or the equivalent (/QuadPoints) using HTML elements in order to achieve the interactivity.

@xlc xlc changed the title Question about highlight annotation (#6827 and few other similar PRs) Question about highlight annotation and others (#6827 and few other similar PRs) Jan 4, 2016
@automatedbugreportingfacility

The pointer cursor is normally used to give user a hint that they can interact with the element, which is not the case here. Clicking the elements doesn't do anything

This is not exactly correct, the pointer cursor is to indicate that you can click the area to pin the annotation. Otherwise, the annotation is closed when the cursor leaves the annotation area. Yes, I also missed it at first :)

@xlc
Copy link
Contributor Author

xlc commented Jan 5, 2016

@automatedbugreportingfacility pin the highlight annotation? I know you can interact with the text annotation but not highlights.

@timvandermeij
Copy link
Contributor

@xlc Take for example https://github.com/mozilla/pdf.js/blob/master/test/pdfs/annotation-highlight.pdf or https://github.com/mozilla/pdf.js/blob/master/test/pdfs/annotation-squiggly.pdf. If you open these with any PDF reader, so will see a popup when you hover over them or click them. This is because practically any annotation type can have an associated Popup annotation according to the specification. This happens very often. My recent patches add support for this interactivity. Open the files above with PDF.js now and you will notice the popup coming up when you hover over the annotations. Clicking them will pin or unpin them.

We have #6810 for supporting annotations without appearance streams (those are not rendered right now) and #6811 for supporting quad points, so I'm closing this issue as resolved.

@xlc
Copy link
Contributor Author

xlc commented Jan 5, 2016

Thanks for the clarification. However I still want to reopen this issue because for this file, there are no popups attached with the highlight annotation and therefore the click/hover doesn't do anything. The main issue is that you lost the ability to select the highlighted text.

We should avoid render HTMLs for highlight without popup. Also I think most of the highlight annotations doesn't have popups anyway. (You can't create highlight with popup in OS X Preview)

@timvandermeij
Copy link
Contributor

@xlc Thank you. I have opened #6838 to track that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants