Skip to content
This repository has been archived by the owner on Feb 25, 2023. It is now read-only.

[bug] Yomichan popover not closing when webpage blocks registering events #111

Closed
seanblue opened this issue Jan 7, 2018 · 7 comments · Fixed by #2101
Closed

[bug] Yomichan popover not closing when webpage blocks registering events #111

seanblue opened this issue Jan 7, 2018 · 7 comments · Fixed by #2101
Labels

Comments

@seanblue
Copy link

seanblue commented Jan 7, 2018

This is a strange issue, probably due to bad website design. Ideally websites wouldn't block the registering of events, but if reasonable Yomichan should still work anyway.

It seems like this bug occurs on Discourse websites (but only when logged in) and I would assume other websites on occasion, though I haven't encountered any others.

The issue is that when you have the popover open and click away, it doesn't close. Here was a diagnosis done by someone: https://community.wanikani.com/t/yomichan-not-closing-popover-in-forums/24961/6

According to that poster, theoretically changing this line (and I assume others) from window to document would fix the issue. Though of course I have no idea if that has other side effects.

Let me know if you think fixing this is unreasonable and I'll open an issue with Discourse.

@seanblue seanblue changed the title Yomichan popover not closing when webpage blocks registering events [bug] Yomichan popover not closing when webpage blocks registering events Jan 7, 2018
@mdboop
Copy link

mdboop commented Jan 8, 2018

Thanks @seanblue for posting this! I was just looking into this some more, because I'm still not sure what the best course of action is, and I wanted to find some more documentation for how to address it.

@FooSoft, one option would be to use the capture option and set it to true.

So the change would be something like this:

window.addEventListener('mousedown', this.onMouseDown.bind(this), { capture: true });

Documentation on this option from MDN.

@seanblue
Copy link
Author

@FooSoft Any thoughts on this?

@seanblue
Copy link
Author

@FooSoft I reached out to Discourse and they are resolving the issue on their end. But not surprisingly, this issue happens on other websites as well. The latest website I found with this issue is goodreads.com. Based on this, please consider the suggestion made by @mdboop or another change so that closing the popover works on all websites.

@FooSoft
Copy link
Owner

FooSoft commented Jan 28, 2018

Sorry for slow reply -- I've been busy dealing with some stuff IRL.

@mdboop what you are suggesting is probably the best way to go (possibly for other event handlers as well). Fighting with scripts already on the page is always fun.

@mdboop
Copy link

mdboop commented Jan 29, 2018

@FooSoft, no worries; thanks for replying. I think a change from window.addEventListener('mousedown', this.onMouseDown.bind(this) to document.addEventListener('mousedown', this.onMouseDown.bind(this) should be all that's needed.

I'd be happy to open a PR with this change, but if you think we should investigate a bit further, I'd be happy to take another look at this to confirm this is the best way to resolve the issue.

@FooSoft
Copy link
Owner

FooSoft commented Feb 1, 2018

@mdboop to me it seems like the only reason the issue is resolved by switching to document from window is that the page(s) in question just happen to block events on window. I don't see a reason that they could not also block document as well. I think the useCapture option may be a more reliable solution to this problem since it actually changes event processing behavior. Any thoughts?

@seanblue
Copy link
Author

Have you decided whether or not to make any changes to make the extension less likely to interfere with other website's code?

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

Successfully merging a pull request may close this issue.

4 participants