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

Does not work on Bookwalker's web reader on Safari #773

Closed
mxng opened this issue Sep 24, 2021 · 4 comments
Closed

Does not work on Bookwalker's web reader on Safari #773

mxng opened this issue Sep 24, 2021 · 4 comments

Comments

@mxng
Copy link

mxng commented Sep 24, 2021

Here's what it looks like on Firefox when it is working normally:

Screen.Recording.2021-09-24.at.8.51.25.AM-1.mov

And here's what it looks like on Safari:

Screen.Recording.2021-09-24.at.8.50.32.AM-1.mov

This is from bookwalker.jp 's web reader for their light novels, where when you highlight words on the viewer and the popup comes out for you to write any notes or highlights. The words on the popup are usually detectable on Firefox so I wanted to try it on Safari but it doesn't work.

@birtles
Copy link
Member

birtles commented Sep 24, 2021

Hi!

Thanks for reporting this. I suspect this has to do with the fact that Safari runs in a separate mode (activeTab mode) where only the page where the add-on only has access to the page(s) on which you enable it. If that popup is from an iframe on a separate origin then it won't work.

I'd like to investigate but when I open books on bookwalker.jp I don't see the highlight feature. I guess I need to create an account?

@birtles
Copy link
Member

birtles commented Sep 24, 2021

I managed to reproduce this and I think what is happening is as follows:

  • The popup element has class ui-dialog
  • ui-dialog has this rule applied: #viewer, .ui-dialog * { -webkit-user-select: none; user-select: none }
  • If we call document.elementFromPoint(x, y) passing in the mouse position we get back the corresponding <div class="message">文字</div> element which contains the selected/highlighted text.
  • On Firefox, if we call document.caretPositionFromPoint(x, y) we get the corresponding text node for 文字
  • On Safari, however, if we call document.caretRangeFromPoint(x, y) (Safari doesn't implement caretPositionFromPoint) we get back the ui-dialog element

The Safari implementation sort of makes sense. You can't select that text so it won't give you that caret position and it appears to walk up the tree to find the first element whose text you can select.

I suppose we could try something where, if:

  • We end up finding no text
  • We used caretRangeFromPoint
  • At least one of the descendants of the returned element has children with user-select: none (or -webkit-user-select: none)

Temporarily apply a rule to drop the user-select: none from all elements in the document and try the lookup again.

However, the third condition there is hard to check efficiently, and adding rules and forcing a restyle is not very performant.

I'll have to think about this some more.

@birtles
Copy link
Member

birtles commented Oct 1, 2021

Unfortunately Chrome doesn't have the same behaviour as Safari so I can't easily write an automated test for this since we don't run automated tests in Safari at the moment.

Manual test case: https://jsbin.com/takayijibe/edit?html,css,js,output

@birtles
Copy link
Member

birtles commented Oct 1, 2021

What's more, Safari doesn't necessarily return an ancestor of the node it should. In the bookwalker.jp case it appears to return the next sibling (with class ui-widget-overlay).

So I think this needs a different approach where we:

  • Detect failing to get a text node
  • Retry using elementFromPoint
  • If we get a different element back, and it has innerText, and it has (-webkit-)user-select: none
  • Then we retry after clearing the user-select property

In my testing that appears to work.

@birtles birtles closed this as completed in 3ceb27f Oct 1, 2021
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

2 participants