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

Remove mousetrap; allow creating shortcuts containing the super key #2543

Merged
merged 2 commits into from
Jan 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@
</div>
<div id="tabs">
<div id="tab-editor" hidden>
<input id="tab-editor-input" class="mousetrap" spellcheck="false" />
<input id="tab-editor-input" spellcheck="false" />
</div>
<div id="tabs-inner" role="tablist" class="has-thin-scrollbar"></div>
</div>
Expand Down Expand Up @@ -216,7 +216,7 @@
hidden
class="theme-background-color theme-text-color"
>
<input id="findinpage-input" class="mousetrap" />
<input id="findinpage-input" />
<span id="findinpage-count" class="inline-text"></span>
<div class="divider"></div>
<button id="findinpage-previous-match">
Expand All @@ -238,7 +238,7 @@
>
<div class="task-search-input-container">
<i class="i carbon:search"></i>
<input type="search" id="task-search-input" class="mousetrap"/>
<input type="search" id="task-search-input"/>
</div>
</div>
<div id="task-area"></div>
Expand Down
123 changes: 59 additions & 64 deletions js/keybindings.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
/*
There are three possible ways that keybindings can be handled.
Shortcuts that appear in the menubar are registered in main.js, and send IPC messages to the window (which are handled by menuRenderer.js)
- If the browser UI is focused, shortcuts are handled by Mousetrap.
- If a BrowserView is focused, shortcuts are handled by the before-input-event listener.
- If the browser UI is focused, a before-input-event is generated in the main process and forwarded to here.
- If a BrowserView is focused, a before-input-event is generated from the webContents and forwarded to here.
*/

const Mousetrap = require('mousetrap')
const keyMapModule = require('util/keyMap.js')

var webviews = require('webviews.js')
Expand All @@ -15,7 +14,6 @@ var settings = require('util/settings/settings.js')
var keyMap = keyMapModule.userKeyMap(settings.get('keyMap'))

var shortcutsList = []
var registeredMousetrapBindings = {}

/*
Determines whether a shortcut can actually run
Expand Down Expand Up @@ -96,18 +94,6 @@ function defineShortcut (keysOrKeyMapName, fn, options = {}) {
fn: shortcutCallback,
keyUp: options.keyUp || false
})
if (!registeredMousetrapBindings[keys + (options.keyUp ? '-keyup' : '')]) {
// mousetrap only allows one listener for each key combination (+keyup variant)
// so register a single listener, and have it call all the other listeners that we have
Mousetrap.bind(keys, function (e, combo) {
shortcutsList.forEach(function (shortcut) {
if (shortcut.combo === combo && (e.type === 'keyup') === shortcut.keyUp) {
shortcut.fn(e, combo)
}
})
}, (options.keyUp ? 'keyup' : null))
registeredMousetrapBindings[keys + (options.keyUp ? '-keyup' : '')] = true
}
})
}

Expand All @@ -117,58 +103,67 @@ navigator.keyboard.getLayoutMap().then(map => {
keyboardMap = map
})

function initialize () {
webviews.bindEvent('before-input-event', function (tabId, input) {
var expectedKeys = 1
// account for additional keys that aren't in the input.key property
if (input.alt && input.key !== 'Alt') {
expectedKeys++
}
if (input.shift && input.key !== 'Shift') {
expectedKeys++
}
if (input.control && input.key !== 'Control') {
expectedKeys++
function beforeInputEventHandler (input) {
var expectedKeys = 1
// account for additional keys that aren't in the input.key property
if (input.alt && input.key !== 'Alt') {
expectedKeys++
}
if (input.shift && input.key !== 'Shift') {
expectedKeys++
}
if (input.control && input.key !== 'Control') {
expectedKeys++
}
if (input.meta && input.key !== 'Meta') {
expectedKeys++
}

shortcutsList.forEach(function (shortcut) {
if ((shortcut.keyUp && input.type !== 'keyUp') || (!shortcut.keyUp && input.type !== 'keyDown')) {
return
}
if (input.meta && input.key !== 'Meta') {
expectedKeys++
var matches = true
var matchedKeys = 0
shortcut.keys.forEach(function (key) {
if (!(
key === input.key.toLowerCase() ||
// we need this check because the alt key can change the typed key, causing input.key to be a special character instead of the base key
// but input.code isn't layout aware, so we need to map it to the correct key for the layout
(keyboardMap && key === keyboardMap.get(input.code)) ||
(key === 'esc' && input.key === 'Escape') ||
(key === 'left' && input.key === 'ArrowLeft') ||
(key === 'right' && input.key === 'ArrowRight') ||
(key === 'up' && input.key === 'ArrowUp') ||
(key === 'down' && input.key === 'ArrowDown') ||
(key === 'alt' && (input.alt || input.key === 'Alt')) ||
(key === 'option' && (input.alt || input.key === 'Alt')) ||
(key === 'shift' && (input.shift || input.key === 'Shift')) ||
(key === 'ctrl' && (input.control || input.key === 'Control')) ||
(key === 'mod' && window.platformType === 'mac' && (input.meta || input.key === 'Meta')) ||
(key === 'mod' && window.platformType !== 'mac' && (input.control || input.key === 'Control')) ||
(key === 'super' && (input.meta || input.key === 'Meta'))
)
) {
matches = false
} else {
matchedKeys++
}
})

if (matches && matchedKeys === expectedKeys) {
shortcut.fn(null, shortcut.combo)
}
})
}

shortcutsList.forEach(function (shortcut) {
if ((shortcut.keyUp && input.type !== 'keyUp') || (!shortcut.keyUp && input.type !== 'keyDown')) {
return
}
var matches = true
var matchedKeys = 0
shortcut.keys.forEach(function (key) {
if (!(
key === input.key.toLowerCase() ||
// we need this check because the alt key can change the typed key, causing input.key to be a special character instead of the base key
// but input.code isn't layout aware, so we need to map it to the correct key for the layout
(keyboardMap && key === keyboardMap.get(input.code)) ||
(key === 'esc' && input.key === 'Escape') ||
(key === 'left' && input.key === 'ArrowLeft') ||
(key === 'right' && input.key === 'ArrowRight') ||
(key === 'up' && input.key === 'ArrowUp') ||
(key === 'down' && input.key === 'ArrowDown') ||
(key === 'alt' && (input.alt || input.key === 'Alt')) ||
(key === 'option' && (input.alt || input.key === 'Alt')) ||
(key === 'shift' && (input.shift || input.key === 'Shift')) ||
(key === 'ctrl' && (input.control || input.key === 'Control')) ||
(key === 'mod' && window.platformType === 'mac' && (input.meta || input.key === 'Meta')) ||
(key === 'mod' && window.platformType !== 'mac' && (input.control || input.key === 'Control'))
)
) {
matches = false
} else {
matchedKeys++
}
})
function initialize () {
webviews.bindEvent('before-input-event', function (tabId, input) {
beforeInputEventHandler(input)
})

if (matches && matchedKeys === expectedKeys) {
shortcut.fn(null, shortcut.combo)
}
})
ipc.on('before-input-event', function (e, input) {
beforeInputEventHandler(input)
})
}

Expand Down
1 change: 0 additions & 1 deletion js/searchbar/bookmarkEditor.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ const bookmarkEditor = {
var newTagInput = document.createElement('input')
newTagInput.className = 'tag-input'
newTagInput.placeholder = l('bookmarksAddTag')
newTagInput.classList.add('mousetrap')
newTagInput.spellcheck = false
tagArea.appendChild(newTagInput)

Expand Down
1 change: 0 additions & 1 deletion js/taskOverlay/taskOverlayBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ var TaskOverlayBuilder = {
nameInputField: function (task, taskIndex) {
var input = document.createElement('input')
input.classList.add('task-name')
input.classList.add('mousetrap')

var taskName = l('defaultTaskName').replace('%n', taskIndex + 1)

Expand Down
4 changes: 4 additions & 0 deletions main/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,6 +317,10 @@ function createWindowWithBounds (bounds, customArgs) {
}
})

newWin.webContents.on('before-input-event', function(e, input) {
sendIPCToWindow(newWin, 'before-input-event', input)
})

newWin.setTouchBar(buildTouchBar())

windows.addWindow(newWin)
Expand Down
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"dragula": "github:minbrowser/dragula",
"electron-squirrel-startup": "^1.0.0",
"expr-eval": "^2.0.2",
"mousetrap": "^1.5.3",
"node-abi": "^3.8.0",
"pdfjs-dist": "4.2.67",
"quick-score": "^0.2.0",
Expand Down
4 changes: 4 additions & 0 deletions pages/settings/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -493,6 +493,9 @@ function formatKeyValue (value) {
value = value.replace(/\bmod\b/g, 'ctrl')
value = value.replace(/\boption\b/g, 'alt')
}
if (navigator.platform === 'Win32') {
value = value.replace(/\bsuper\b/g, 'win')
}
return value
}

Expand All @@ -509,6 +512,7 @@ function parseKeyInput (input) {
} else {
e = e.replace(/\b(control)|(ctrl)\b/g, 'mod')
e = e.replace(/\balt\b/g, 'option')
e = e.replace(/\bwin\b/g, 'super')
}
return e
})
Expand Down