-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tab close and left-right nav hotkeys sometimes fail to send hold key release to other tabs #15
Comments
Could this also happen when using left-right nav, if the hold key is released between switching tabs? I haven't experienced it, so I'm guessing not -- but I'm not sure what's different between left-right nav and this bug. My best guess would be that the keyup event gets triggered on the tab to be closed (so not on the new tab moved to), but the tab gets closed before it actually handles the event. Whereas with left-right nav, the keyup event must either land on the original tab or the tab just navigated to. |
This might be a better fix: explicitly switch the active tab to another tab before we close the intended tab. Though if it's just a matter of timing, it could still be possible for the keyup to get triggered on the tab to close, switch to the new tab, and then the old tab gets closed before it executes the keyup handler. |
Another idea. In |
Darn, I tried it but apparently there is a security restriction (I should've paid more heed to mentions of this I saw in MDN documentation) not allowing you to |
After thinking a bit, there's an edge case this solution can't quite handle: when the tab is the last one in a window (with other windows existing; if it was the global last tab, we don't have to worry about the hold key getting "stuck"). In this case, we don't have another tab to switch to (if we switched to a tab in another window, that would disagree with Chrome's default behavior and be really unexpected). At the same time, I've come to realize that my original proposed solution isn't good either, because I do in fact often use left-right nav and tab close hotkeys in close succession without releasing the hold key (e.g. moving over to close a bunch of old tabs, or closing a tab and then moving left to a different one than the default tab active after closing). I'm out of good solutions. As a side note, I tried just broadcasting a hold key release whenever we close a tab. I figured that the repeated keydown events generated by holding the hold key down would relatively quickly set |
Okay, with more careful examination, I've found that what I brought up in this comment is true. You can miss the hold key release when using left-right nav if you release it quickly "between" tabs. We need a solution that generalizes to both of these. Updating issue title and complexity. |
With some more tinkering, I've figured out the behavior of a held down key and why my idea two comments up didn't work. The difference between what I was trying to do in KeepTabs and the test I did is that I used the mouse to switch tabs. When key A is held, keydown events for it continue to get fired rapidly, but if another key B is pressed, they stop getting fired (even if A is still held down). (You can easily observe this with two alphabet keys. Try holding 'a' and then press 's'. You'll see that the repeated a's stop getting typed.) Thus when we pressed [ or ] or ; to do left-right nav or close a tab, the new active tab didn't receive repeated keydown events for the hold key because another key being pressed had interrupted the repeated events. |
New discovery: The hold key behaves perfectly fine when there are one or two tabs when you use the hotkeys in question. When the hotkeys are pressed when there are three tabs or more, they consistently make the hold key stay active when it's not supposed to. |
I don't think that the reason why the hold key is misbehaving is that the release event is being triggered "in between tabs" since the problem still occurs when I release the hold key after waiting for the tab switch to occur. |
I find it extremely hard to believe that the bug has any direct relation to the number of tabs you have open. Note that it occurs very infrequently to begin with; the timing has to be just right -- in particular with left-right navigation (it happens more with tab close, I find). So any such pattern sounds purely coincidental, as I can't imagine any reason why this behavior would depend on the number of open tabs itself. Though, I have noticed that it can occur more when switching between many tabs or to an older tab; it's probably related to Chrome's memory limiting functionality that takes old tabs out of memory when they've been unused a while -- probably a tab that is being loaded back into memory takes longer to be ready to listen for key events, creating a larger window of opportunity for the hold key release to be missed. As for the latter comment, can you provide minimal steps to reproduce? I've never seen this occur, and I've tinkered with this a lot. |
ReiterationOn my Macbook, this issue occurs very frequently. I agree that this has most likely to do with Chrome's memory management. I reaffirm that the problem is not that the keyup event is being missed "in between tabs" as the issue happens whether I release the hold key quickly or after a pause. ObservationsFurthermore, I have observed that, in the instances where this issue occurs, no keyup event is being registered at all when I release the hold key. During my testing, the hold key was set to the escape key. This test was done by printing console logs whenever the content script detected a keyup event (Note that you can't listen for the keyup events from the webpage itself because the extension prevents the event from getting to the webpage). Possible recourseIf this issue is specific to the escape keyHowever, I have noticed that the problem disappears when I change the hold key to be my control key. This issue may be a problem specific to the escape key. If this is the case, we may want to consider removing the escape key from the list of possible hold keys. Macs can easily use the alt or control key as the hold key, and windows and linux can use the alt key (they can also map any of the possible hold keys to their caps lock key). ElseEven if this issue still occurs with the other modifier keys, there is still reason to consider removing the escape key from the list of possible hold keys. The escape key is not a true modifier key like control or alt (we've just been hacking it into one). Javascript actually tells us which (true) modifier keys were being held down when a click event or a keyboard event occurs (the code for this looks like
Benefits
NOTE: These changes only impact the built-in hotkeys and have only minimal impact on regular hotkeys!
However, this does not actually cause any problem because, instead of keeping track of when the hold key is being held down, the extension will keep track of a similar abstraction: when the extension is listening for a non-built-in hotkey to be typed in. The visual overlay would reflect this change out-of-the-box. If we go with this approach, then the visual overlay wouldn't represent that the hold key is being pressed down. Rather, it would represent that the extension is now listening for a non-built-in hotkey to be entered. I say "non-built-in" hotkey because the extension will always execute a built-in hotkey when the hold key is being held down since it reads whether the hold key is being held down straight from the event object. It does not rely on the state of the extension where as "non-built-in" hotkeys do. Side EffectsThis is related to #1 . Using only the true modifier keys can make it a lot easier to write clean code for all the hold keys. Also, if we remove the Escape from our list of possible hold keys, then #13 is not a problem any more because escape is the only one of the possible hold keys that had any function that occurred when you pressed it alone. #35 would also no longer be a problem since keys pressed with modifier keys don't trigger any listeners. |
After further investigation, I feel I've gotten to a much better understanding. I at least know why we observe a difference in behavior between escape, control, and Cmd as hold keys on Mac, as well as confirming that Mac and Ubuntu Chrome behaviors differ (even using the same version of Chrome). From now on, polished long-form investigation should be added to the wiki page I just started (investigation in the history section; updated conclusions in the current hypothesis). I still have some TODOs to finish filling in tomorrow. |
In conclusion, Roger's proposed solution is the best way to go. We just need to determine which hold key choices offer the best combination of 1) convenient location (or convenient method for remapping to convenient location) and 2) lack of interference with browser and OS shortcuts (or ways to remap them to remove interference). To this end, I've created a wiki page for collecting this information. |
We immediately send the tab close hotkey to the background script (like the left-right navigation, we don't wait for keyup on the hold key). In some cases, if the user releases the hold key right in-between when the tab closes and Chrome switches to the adjacent tab, the keyup won't be handled either on the closed tab or on the new active one, causing no hold key released message to be broadcast and all tabs to be stuck in the holding = true state until the hold key is deliberately pressed and released to fix it. The solution I'm thinking of right now isn't perfect, it would be: don't treat the tab close hotkey like left-right nav; i.e. don't send it until the hold key is released. There's an additional decision whether to treat it exactly like a regular user-specified hotkey, or to treat it as special so that if a user enters ; anywhere in the hotkey sequence, we count it as a close tab. Pro: Fixes the bug; also, makes closing a tab a slightly more deliberate and cancellable (if we treat it like a regular hotkey requiring exact match) action, which may be good. Con: Prevents rapid closing of multiple tabs in the same spirit as rapid left-right nav. This doesn't seem like a super common use case, but it does exist.
The text was updated successfully, but these errors were encountered: