FooSoft / yomichan

Japanese pop-up dictionary extension for Chrome and Firefox.
https://foosoft.net/projects/yomichan
Other
1.06k stars 213 forks source link

Popup stays closed after scanning content while hide-override is active #530

Closed toasted-nutbread closed 3 years ago

toasted-nutbread commented 4 years ago

From: https://github.com/FooSoft/yomichan/pull/520#pullrequestreview-408653478

Found an unrelated bug with the screenshot marker and (I assume) iframe popups when testing. If you manage to scan text where the popup is temporarily hidden because of taking a screenshot, the popup will stay closed even after the override is removed.

Probably relevant code in source.js:


static shouldEnter(node) {
switch (node.nodeName.toUpperCase()) {
case 'RT':
case 'SCRIPT':
case 'STYLE':
return false;
}
    const style = window.getComputedStyle(node);
    return !(
        style.visibility === 'hidden' ||
        style.display === 'none' ||
        parseFloat(style.fontSize) === 0
    );
}
siikamiika commented 4 years ago

Were you able to reproduce this? I can start looking deeper into the issue now.

toasted-nutbread commented 4 years ago

Didn't test yet; just made into an issue so it's not lost.

siikamiika commented 4 years ago

This seems to be caused by the visibility style as expected, but it was a bit unexpected to me that document.elementFromPoint and document.elementsFromPoint don't return hidden elements.

I see some approaches, for example using transparency instead of visibility to hide the popup or blocking popup hiding until the override ends. Not sure what would be the most correct way.

toasted-nutbread commented 3 years ago

Tested this again after forcing the delay to be longer. The problem seems to no longer exist, so I will close the issue.