Skip to content
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

Triggering on shift-cmd-b #1362

Closed
jmound opened this issue Dec 19, 2014 · 23 comments
Closed

Triggering on shift-cmd-b #1362

jmound opened this issue Dec 19, 2014 · 23 comments

Comments

@jmound
Copy link

jmound commented Dec 19, 2014

Shift-cmd-b is the keystroke for "always show bookmarks bar", it also triggers the Vomnibox / "open bookmark" via the 'b'

I think the Vomnibox shouldn't be triggered on that keystroke. If there is a way to disable that keystroke via Vimium's options > excluded URLs and keys, let me know.

OS: OSX 10.10.1
Chrome: 39.0.2171.95
Vimium: 1.49

@smblott-github
Copy link
Collaborator

Shift-cmd-b is the keystroke for "always show bookmarks bar", it also triggers the Vomnibox / "open bookmark" via the 'b'

Don't completely understand this, but you can unmap (or remap) any keys you like via Custom key mappings on the options page:

snapshot

This doesn't sound like something for which Excluded URLs and keys is appropriate. (But, perhaps I'm misunderstanding you.)

@maximbaz
Copy link

I think @jmound means that when he triggers browser native shortcut ctrl+shift+b, which toggles the bookmark bar, not only Chrome catches that, but vimium also reacts by executing the Vomnibar.activateBookmarks command.

What's interesting through, is that I cannot reproduce that on both Windows and Kubuntu, with the latest code from master. For me, only Chrome reacts, vimium does not.

@jmound
Copy link
Author

jmound commented Dec 19, 2014

Exactly what @z0rch said (although I haven't looked on other OSes besides OSX).

@smblott-github
Copy link
Collaborator

Exactly what @z0rch said ...

Not seeing this on Linux. (Edit: Debian.)

@jmound
Copy link
Author

jmound commented Dec 19, 2014

@smblott-github It definitely may be OS-specific. Would screenshots or anything else like that help?

@deiga
Copy link
Contributor

deiga commented Dec 22, 2014

Yes, this happens on OS X and would be fairly annoying if a person were to show/hide the bookmark bar a lot.

@rmetzler
Copy link

Yeah, it's annoying to see the Vomnibox whenever I press cmd+shift+b. I guess the fix is similar to this story https://medium.com/medium-eng/the-curious-case-of-disappearing-polish-s-fa398313d4df#9108

@mrmr1993
Copy link
Contributor

I remember reading somewhere that the command key isn't a modifier in javascript; its only effect is firing keyup/keydown events. Can anybody confirm this (or point to how to detect it being held during other events)?

@rmetzler
Copy link

command key is signaled via event.metaKey in JavaScript. Vimium seems to use it here:

if (((event.metaKey || event.ctrlKey || event.altKey) && event.keyCode > 31) || (
# TODO(philc): some events don't have a keyidentifier. How is that possible?
event.keyIdentifier && event.keyIdentifier.slice(0, 2) != "U+"))
keyChar = KeyboardUtils.getKeyChar(event)
# Again, ignore just modifiers. Maybe this should replace the keyCode>31 condition.
if (keyChar != "")
modifiers = []
if (event.shiftKey)
keyChar = keyChar.toUpperCase()
if (event.metaKey)
modifiers.push("m")
if (event.ctrlKey)
modifiers.push("c")
if (event.altKey)
modifiers.push("a")

@mrmr1993
Copy link
Contributor

Can somebody on OS X check what shows up in the background page console when you press cmd-b and cmd-shift-b on a page? That would give some insight into what is happening.

To show the background page console:

  • Go to chrome://extensions
  • check checkbox next to developer mode
  • click "inspect background page" under Vimium
  • press esc to show the console if it's not already visible

@rmetzler
Copy link

main.js:720 checking keyQueue: [ <m-B> ]
main.js:722 new KeyQueue: 
main.js:720 checking keyQueue: [ b ]
main.js:722 new KeyQueue: 
main.js:717 clearing keyQueue

@mrmr1993
Copy link
Contributor

Is that just pressing cmd-shift-b? If it is, I suspect we need to start checking for event.meta in the keyup handler, and (for now) just bailing if it's true.

This will still have issues (eg. #1411), but I don't think it would make any situations any worse then they already are.

@rmetzler
Copy link

Yes, this is only cmd-shift-b and I also tried to verify if the second b is coming from the keyup event or not. It seems like it does all come from keydown as I press and hold all the keys for more than a second, but the [b] comes up in the log almost immediately.

@rmetzler
Copy link

I also tried to type other shortcuts to find key combinations with similar problems, but so far haven't identified any other cmd+shift+char combination triggering similar behavior.

Then I put a breakpoint in the extension and looked at the call stack for handleKeyDown. This seems to go the proper extension messaging route as far as I can tell without having any experience in writing browser extensions.

Too bad, I can't see the vimium content_scripts in the background page. :-(

@mrmr1993
Copy link
Contributor

The b should be from the keypress handler, setting a breakpoint there will show us sending the message for it (and, notably, ignoring event.meta).

@rmetzler
Copy link

yes, I can confirm. The additional b comes from onKeypress in vimium_frontend.coffee.
I'm not sure how to fix it properly. Do you want me to just add another check for metaKey or should I refactor the modifier related code from onKeydown into KeyboardUtils and reuse this?

@mrmr1993
Copy link
Contributor

Several other questions need answering, so we know we're not breaking things (and maybe even some fixes for other issues may arise):

  • Is event.shiftKey true for all keydown events where shift was held (eg. typing B)?
  • Are event.shiftKey and event.metaKey both true for a printing character that uses shift and command (eg. cmd-shift-4)?
  • (BONUS: Do we get any extra information to indicate when the keypress event is the result of a key combo (eg. the 'accent key' and then e for é on some European keyboards).)

Basically, I would like to know whether we can read the keys pressed, the character generated, both, or some unhelpful combination of parts from both. This will inform us of whether we can/how we should handle this (and related) case(s).

@deiga
Copy link
Contributor

deiga commented Jan 11, 2016

Is this still an issue? For me it doesn't occur anymore

@smblott-github
Copy link
Collaborator

@deiga... Are you saying you experienced this issue previously, but at some point it resolved itself? In which case, we'd close this issue, yes?

@deiga
Copy link
Contributor

deiga commented Jan 12, 2016

@smblott-github Yeah, I was able to repro the issue earlier. But now I can't get it to bug out anymore. I'd kinda wait for @jmound to tell us if he still experiences the issue before closing

@mrmr1993
Copy link
Contributor

The issue seemed to be that cmd-shift-b was leaking a b keypress event in OS X. If this no longer happens then the issue should be resolved.

@jmound
Copy link
Author

jmound commented Jan 14, 2016

The issue is resolved. I tested with OSX 10.11.1, Chrome 47.0, Vimium 1.53, cmd-shift-bjust hides/shows the bookmarks bar and doesn't trigger the Vomnibox.

@jmound jmound closed this as completed Jan 14, 2016
@tmetn
Copy link

tmetn commented Aug 6, 2018

I could not figure it out how to map CMD key, what is the correct format?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants
@deiga @rmetzler @jmound @maximbaz @mrmr1993 @smblott-github @tmetn and others