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

Support compound keys (<a-f>, <c-[>, etc.) in passkeys #1368

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 23 additions & 4 deletions content_scripts/vimium_frontend.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,17 @@ keyPort = null
# are passed through to the underlying page.
isEnabledForUrl = true
passKeys = null
splitPassKeys = []
keyQueue = null
# The user's operating system.
currentCompletionKeys = null
validFirstKeys = null

# Keys are either literal characters, or "named" - for example <a-b> (alt+b), <left> (left arrow) or <f12>
# This regular expression captures two groups: the first is a named key, the second is the remainder of
# the string.
namedKeyRegex = /^(<(?:[amc]-.|(?:[amc]-)?[a-z0-9]{2,5})>)$/

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to ignore case for named keys, i.e. treat <A-b> in the same way as <a-b>.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is pinched from the background page, so (for now, at least) I think it's best leave as is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if it matters here, but case in keys matters since <A-b> and <a-b> are two different combinations and matching without case, would create weird ambiguity errors

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@deiga are they different? I think <A-b> and <a-b> are same and both mean Alt+b, while <a-B> and <A-B> both mean Alt+Shift+b.

Update: I was talking about the conceptual idea in vim, not about the current implementation in vimium.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment we don't support capitalised modifiers for mappings. This regex is copied from there, for consistency.

If this is a bug, we can open a separate issue/PR and discuss it there.


# The types in <input type="..."> that we consider for focusInput command. Right now this is recalculated in
# each content script. Alternatively we could calculate it once in the background page and use a request to
# fetch it each time.
Expand Down Expand Up @@ -149,7 +155,7 @@ installListener = (element, event, callback) ->
installedListeners = false
initializeWhenEnabled = (newPassKeys) ->
isEnabledForUrl = true
passKeys = newPassKeys
setPassKeys newPassKeys
if (!installedListeners)
# Key event handlers fire on window before they do on document. Prefer window for key events so the page
# can't set handlers to grab the keys before us.
Expand All @@ -165,7 +171,7 @@ initializeWhenEnabled = (newPassKeys) ->
setState = (request) ->
initializeWhenEnabled(request.passKeys) if request.enabled
isEnabledForUrl = request.enabled
passKeys = request.passKeys
setPassKeys request.passKeys

#
# The backend needs to know which frame has focus.
Expand Down Expand Up @@ -345,8 +351,19 @@ extend window,
# Decide whether this keyChar should be passed to the underlying page.
# Keystrokes are *never* considered passKeys if the keyQueue is not empty. So, for example, if 't' is a
# passKey, then 'gt' and '99t' will neverthless be handled by vimium.
isPassKey = ( keyChar ) ->
return !keyQueue and passKeys and 0 <= passKeys.indexOf(keyChar)
isPassKey = (keyChar) ->
not keyQueue and splitPassKeys and 0 <= splitPassKeys.indexOf(keyChar)

setPassKeys = (newPassKeys) ->
passKeys = newPassKeys
passKeyGroups = passKeys.split " "
splitPassKeys = []
for passKeyGroup in passKeyGroups
namedKeyMatch = passKeyGroup.match namedKeyRegex
if namedKeyMatch # The current space delimited group is a single named key.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny: if namedKeyRegex.test passKeyGroup would be clearer, as you don't really use matches anyway

splitPassKeys.push passKeyGroup # Push this to as a single key
else
splitPassKeys = splitPassKeys.concat passKeyGroup.split "" # Pass all the characters as seperate keys.

# Track which keydown events we have handled, so that we can subsequently suppress the corresponding keyup
# event.
Expand Down Expand Up @@ -473,6 +490,8 @@ onKeydown = (event) ->

else if (!isInsertMode() && !findMode)
if (keyChar)
if isPassKey keyChar
return undefined
if (currentCompletionKeys.indexOf(keyChar) != -1 or isValidFirstKey(keyChar))
DomUtils.suppressEvent event
KeydownEvents.push event
Expand Down
4 changes: 4 additions & 0 deletions pages/options.html
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,10 @@
<br/><br/>
If "Keys" is left empty, then vimium is wholly disabled.
Otherwise, just the listed keys are disabled (they are passed through).
<br/><br/>
To enter complex keys such as
<pre>&lt;c-[&gt;, &lt;a-f&gt;, &lt;c-a-A&gt;</pre>
simply seperate them from other passkeys with a space.
</div>
</div>
<div>
Expand Down