Fix: loadFullSizeImageImageTimeout not clearing when mouse leaves image #1516
Conversation
LiliaDoe
commented
Dec 31, 2024
•
LiliaDoe
commented
- This fixes what Fix: images showing when mouse is off image link #1515 was trying to fix
- setLoadImage used to set loadHoveredImage within callback so timeout can be set outside of callback. This makes clearTimeout use the correct timeout ID
- There is probably a smarter way of fixing this, but this works
js/hoverzoom.js
Outdated
| } else if (src) { | ||
| chrome.runtime.sendMessage({action:'isImageBanned', url:src}, function (result) { | ||
| setLoadImage(false); | ||
| if (!result) { | ||
| loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay); | ||
| setLoadImage(true); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // if the action key has been pressed over an image, no delay is applied | ||
| const delay = actionKeyDown || explicitCall ? 0 : (isVideoLink(srcDetails.url) ? options.displayDelayVideo : options.displayDelay); | ||
| if (loadHoveredImage) loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay); |
There was a problem hiding this comment.
I'm not quite sure I understand how this works. Isn't chrome.runtime.sendMessage asynchronous? I mean it returns immediately and at some later point invokes a call back function. Which means line 1096 could check loadHoveredImage value before the callback has opportunity to set it.
There was a problem hiding this comment.
That is true, and that is also why the bug was happening, i think. The timeout id being set by setTimeout wasn't always being cleared by clearTimeout due to it being async. Maybe I'm misunderstanding why the bug was happening, though. I'm sure there is a better way of solving this
There was a problem hiding this comment.
I think it's better to temporarily remove that entire message sending block until a proper solution is found. Right now the value of loadHoveredImage would be reused from the previous image (because the current image would only set it after it was already checked) which is not correct.
There was a problem hiding this comment.
If it's temporary, I could comment it out for now like
/*
old code
*/There was a problem hiding this comment.
I'd imagine @GrosPoulet would want to make it work at some point so commenting out seems fine to me.
There was a problem hiding this comment.
@extesy @LiliaDoe Happy New Year 🎉
I uncommented L1076-1088 and i can not reproduce the issue on Reddit or any other website. I have a 1 second delay before picture display.
My code:
`
// Temporarily removing until a better fix is found: sendMessage is async so it can't be used to set local variables
if (audioSrc) {
chrome.runtime.sendMessage({action:'isImageBanned', url:audioSrc}, function (result) {
if (!result) {
loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay);
}
});
} else if (src) {
chrome.runtime.sendMessage({action:'isImageBanned', url:src}, function (result) {
if (!result) {
loadFullSizeImageTimeout = setTimeout(loadFullSizeImage, delay);
}
});
}
loading = true;`
There was a problem hiding this comment.
@GrosPoulet I posted an easy repro steps in #1515 (comment) - please take a look. Anyhow, the code as you wrote it couldn't possibly work due to async nature of sendMessage() function.
|
LiliaDoe
deleted the
Fix-loadFullSizeImageImageTimeout-not-clearing-when-mouse-leaves-image
branch
- when an image (or video & audio track) is banned: - url of banned image is stored in background page local storage - a message is sent to all tabs with updated list of banned urls - removal of async call to background page: check for banned image is now performed synchronously in each tab
