If there is no problem I could fix this issue.

I believe there is a good reason we don't do this.
@pmiazga in Sam's absence do you remember what that was?

@Jdlrobson I couldn't see how ignoring the key events due to shift and control keys could be an issue. Anyways, what about making a change and seeing if it really affects anything rather than waiting, in case it really isn't a serious issue ?

There was an issue. We used to do that but removed it. I just have to dig into the history to remember why

In T166610#3335411, @Jdlrobson wrote:

There was an issue. We used to do that but removed it. I just have to dig into the history to remember why

I tried looking into git log src/index.js but in vain. Anyways, if you're that sure then you better close this issue after the reason strikes your mind.

OK, I think I was getting confused with T159490 which doesn't seem to be related to this.

Really this is a product decision, but I think it's such an edge case and I'm not sure it is worth the additional code cost, especially given some users may prefer or not be bothered by this behaviour. What do you think @ovasileva ?

A typical patch would be the one found below. Warning : Browser question is questionable. See https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/key

diff --git a/src/index.js b/src/index.jsowser as it 
index 0ebe48e..181e66f 100644
--- a/src/index.js
+++ b/src/index.js
@@ -169,7 +169,8 @@ mw.requestIdleCallback( function () {
 
                previewLinks
                        .on( 'mouseover keyup', function ( event ) {
-                               boundActions.linkDwell( this, event, gateway, generateToken );
+                               if (event.key !== "Control" && event.key !== "Shift")
+                                       boundActions.linkDwell( this, event, gateway, generateToken );
                        } )
                        .on( 'mouseout blur', function () {
                                boundActions.abandon( this );

@Kaartic If that is the issue then it seems easy to fix, nice. jQuery provides event.which, which should be the keyCode normalized across browsers (see [.keyup()](https://api.jquery.com/keyup/).

Also we need to think about the many keys that can be pressed. for example in OSX Cmd+Shift+[ or ] and Cmd+Shift+→ or ← also moves tabs in every browser.

Related (and when keyup was added) via @phuedx : T158631#3107834

Given that keyup is there for the TAB events when moving through links, maybe it would be better to whitelist TAB if the event is keyup instead of trying to blacklist all possible shortcut keys for moving around tabs in the browser.

Pseudocode:

diff --git a/src/index.js b/src/index.jsowser as it 
index 0ebe48e..181e66f 100644
--- a/src/index.js
+++ b/src/index.js
@@ -169,7 +169,8 @@ mw.requestIdleCallback( function () {
 
                previewLinks
                        .on( 'mouseover keyup', function ( event ) {
+                               // Skip keyups that are not TAB (T166610)
+                               if (event.type === 'keyup' && event.which !== 9) return;
+ 
                                boundActions.linkDwell( this, event, gateway, generateToken );
                        } )
                        .on( 'mouseout blur', function () {
                                boundActions.abandon( this );

Related irc discussion:

joakin
phuedx: so would it be safe to say the only key we want is TAB on keyup?

phuedx
joakin: i think that's right, yes

joakin
because whitelisting that instead of blacklisting all the possible keys of shortcuts to move tabs seems better
ok I'll comment

phuedx
12:29
^
absolutely

12:29
whitelists!

In T166610#3350952, @Jhernandez wrote:

Given that keyup is there for the TAB events when moving through links, maybe it would be better to whitelist TAB if the event is keyup instead of trying to blacklist all possible shortcut keys for moving around tabs in the browser.

I'm surprised by how I missed this comment for so long. Anyways, the idea seems to be nice and a simple test I did with the browser console shows me that the 'Tab' keyup event doesn't occur when I switch back to the old tab using 'Ctrl+Shift+Tab'. Only the keyups for 'Ctrl' and 'Shift' are received.

@ovasileva a solution has been proposed: https://phabricator.wikimedia.org/T166610#3350952 but is this use case worth the associated risk given its an edge case?

I've updated the description with the proposed solution.

Couldn't we just look for blur on the window object ? Or alternatively https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API but i don't think visibility is the right one here actually, because it's about the active window/tab, not even about visibility I think.

In T166610#7561366, @TheDJ wrote:

Couldn't we just look for blur on the window object ? Or alternatively https://developer.mozilla.org/en-US/docs/Web/API/Page_Visibility_API but i don't think visibility is the right one here actually, because it's about the active window/tab, not even about visibility I think.

IMHO that's good idea, we could use the blur/focus events to reset the popups state. The focus most likely it would cause the small flash, eg it could remain visible for a fraction of second.

I am adding this video because I think it's the same or a very similar issue. I do a lot of Cmd-Click to open in background tabs, etc.

What I can't remember is the exact sequence of events that causes this state to occur, but it happens often enough that I can amend this later.

Whenever the preview DIV gets in this state usually the only way to fix it is a full page refresh.

https://mikerichardson.name/tidbits/wikipedia_page_preview_bug.mov

In T166610#7221475, @Jdlrobson wrote:

We can't rely on Ctrl and Shift as indicating a tab change as they may be wired up to different functions on different computers. If someone wishes to investigate this and open a patch by all means we'll consider it.

Or such as in this case where I was not using the keyboard at all to navigate back and forth.

Yeh blur makes sense here (patch is welcome - just not a team priority right now)