Don't mark as read if middle click is outside of article link#8553
Don't mark as read if middle click is outside of article link#8553Inverle wants to merge 1 commit intoFreshRSS:edgefrom
Conversation
| el.parentElement.click(); // Normal click, just toggle article. | ||
| } | ||
| } else if (ev.which == 2 && !ev.ctrlKey) { // Simple middle click: same behaviour as CTRL+click | ||
| } else if (ev.type === 'auxclick' && ev.which == 2 && !ev.ctrlKey) { // Simple middle click: same behaviour as CTRL+click |
There was a problem hiding this comment.
ev.which is deprecated in favor of ev.button
But what concretely is auxclick supposed to do here besides prefilter out the first button?
There was a problem hiding this comment.
ev.which is deprecated in favor of ev.button
Yes, I didn't change it for now though
But what concretely is auxclick supposed to do here besides prefilter out the first button?
So that this doesn't happen:
If you activate auto scroll outside of the article link and then deactivate it on the article link, it will mark the article as read even though the article wasn't opened by the browser.
Here is how auxclick works instead:
There was a problem hiding this comment.
Sure, that's mouseup vs click. But I meant why auxclick. :-)
MDN suggests there may be some advantages to it regarding opening popups.
Starting in Firefox 68, the auxclick event is used to trigger the new tab on middle-click action; previously, this had been done with the click event. Apps can prevent middle-click from opening new tabs (or middle-click to paste, if that feature is enabled) by intercepting auxclick on links, and auxclick event handlers can now open popups without triggering the popup blocker.
But I'm not overly fond of "new in 2024" if that's the only reason to use auxclick instead of click.
There was a problem hiding this comment.
click doesn't work on middle click, only auxclick does.
Chrome and Firefox had support for it for a long time already, Safari added auxclick support in 2024: https://caniuse.com/auxclick
There was a problem hiding this comment.
I see. I suppose that'll do for now, but in that case I find this whole stream.onmouseup = stream.onauxclick = … concept confusing to read due to the inapplicability of some if branches and I'd rather repeat two or three lines of code for clarity.
More important, I think the lines below might trigger twice for the same reason you added the if (ev.type === 'auxclick'? (This ties in with the clarity thing. ;-)
There was a problem hiding this comment.
All good here, or what is the status?
There was a problem hiding this comment.
As I wrote previously, the if (context.auto_mark_site) { on line 1527 would fire first on mouseup and once again on auxclick (directly after mouseup), wouldn't it? So it should include if ev.type === 'auxclick' check (or the whole function should be refactored into two separate functions but never mind that).
Additionally I imagine it's cheaper to start with if (context.auto_mark_site && ev.type === 'auxclick' && ev.which == 3) { thus skipping the .closest() stuff, but that aside.
Alkarex
added
javascript
3 participants
Closes #6451
https://developer.mozilla.org/en-US/docs/Web/API/Element/auxclick_event