From 70bdfd71ca2d8103d74d8af8338c882e929ead34 Mon Sep 17 00:00:00 2001 From: Peng Lyu Date: Mon, 17 Jun 2019 18:01:30 -0700 Subject: [PATCH] move keybinding validator out of keybindingRegistry --- .../keybinding/common/keybindingsRegistry.ts | 64 ++---------------- .../keybinding/browser/keybindingService.ts | 67 +++++++++++++++++++ .../browser/keyboardLayoutService.ts | 5 +- .../keybinding/common/navigatorKeyboard.ts | 15 +++++ 4 files changed, 91 insertions(+), 60 deletions(-) create mode 100644 src/vs/workbench/services/keybinding/common/navigatorKeyboard.ts diff --git a/src/vs/platform/keybinding/common/keybindingsRegistry.ts b/src/vs/platform/keybinding/common/keybindingsRegistry.ts index 2bc7f54bef9..302a3641ec8 100644 --- a/src/vs/platform/keybinding/common/keybindingsRegistry.ts +++ b/src/vs/platform/keybinding/common/keybindingsRegistry.ts @@ -130,22 +130,18 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry { const actualKb = KeybindingsRegistryImpl.bindToCurrentPlatform(rule); if (actualKb && actualKb.primary) { - if (!this._assertBrowserConflicts(actualKb.primary, rule.id)) { - const kk = createKeybinding(actualKb.primary, OS); - if (kk) { - this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, 0, rule.when); - } + const kk = createKeybinding(actualKb.primary, OS); + if (kk) { + this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, 0, rule.when); } } if (actualKb && Array.isArray(actualKb.secondary)) { for (let i = 0, len = actualKb.secondary.length; i < len; i++) { const k = actualKb.secondary[i]; - if (!this._assertBrowserConflicts(k, rule.id)) { - const kk = createKeybinding(k, OS); - if (kk) { - this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, -i - 1, rule.when); - } + const kk = createKeybinding(k, OS); + if (kk) { + this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, -i - 1, rule.when); } } } @@ -212,54 +208,6 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry { } } - private _assertBrowserConflicts(keybinding: number, commandId: string): boolean { - if (!isWeb) { - return false; - } - - const firstPart = (keybinding & 0x0000FFFF) >>> 0; - const chordPart = (keybinding & 0xFFFF0000) >>> 16; - const modifiersMask = KeyMod.CtrlCmd | KeyMod.Alt | KeyMod.Shift; - - for (let part of [firstPart, chordPart]) { - if ((part & modifiersMask) === 0) { - continue; - } - - if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_W) { - console.warn('Ctrl/Cmd+W keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId); - - return true; - } - - if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_N) { - console.warn('Ctrl/Cmd+N keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId); - - return true; - } - - if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_T) { - console.warn('Ctrl/Cmd+T keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId); - - return true; - } - - if ((part & modifiersMask) === (KeyMod.CtrlCmd | KeyMod.Alt) && ((part & 0x000000FF) === KeyCode.LeftArrow || (part & 0x000000FF) === KeyCode.RightArrow)) { - console.warn('Ctrl/Cmd+Arrow keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId); - - return true; - } - - if ((part & modifiersMask) === KeyMod.CtrlCmd && ((part & 0x000000FF) >= KeyCode.KEY_0 && (part & 0x000000FF) <= KeyCode.KEY_9)) { - console.warn('Ctrl/Cmd+Num keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId); - - return true; - } - } - - return false; - } - private _registerDefaultKeybinding(keybinding: Keybinding, commandId: string, commandArgs: any, weight1: number, weight2: number, when: ContextKeyExpr | null | undefined): void { if (OS === OperatingSystem.Windows) { this._assertNoCtrlAlt(keybinding.parts[0], commandId); diff --git a/src/vs/workbench/services/keybinding/browser/keybindingService.ts b/src/vs/workbench/services/keybinding/browser/keybindingService.ts index 2270faefdbb..a956150381c 100644 --- a/src/vs/workbench/services/keybinding/browser/keybindingService.ts +++ b/src/vs/workbench/services/keybinding/browser/keybindingService.ts @@ -279,6 +279,10 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService { // This might be a removal keybinding item in user settings => accept it result[resultLen++] = new ResolvedKeybindingItem(undefined, item.command, item.commandArgs, when, isDefault); } else { + if (this._assertBrowserConflicts(keybinding, item.command)) { + continue; + } + const resolvedKeybindings = this.resolveKeybinding(keybinding); for (const resolvedKeybinding of resolvedKeybindings) { result[resultLen++] = new ResolvedKeybindingItem(resolvedKeybinding, item.command, item.commandArgs, when, isDefault); @@ -308,6 +312,69 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService { return result; } + private _assertBrowserConflicts(kb: Keybinding, commandId: string): boolean { + if (browser.isFullscreen() && (navigator).keyboard) { + return false; + } + + for (let part of kb.parts) { + if (!part.metaKey && !part.altKey && !part.ctrlKey && !part.shiftKey) { + continue; + } + + const modifiersMask = KeyMod.CtrlCmd | KeyMod.Alt | KeyMod.Shift; + + let partModifiersMask = 0; + if (part.metaKey) { + partModifiersMask |= KeyMod.CtrlCmd; + } + + if (part.shiftKey) { + partModifiersMask |= KeyMod.Shift; + } + + if (part.altKey) { + partModifiersMask |= KeyMod.Alt; + } + + if (part.ctrlKey && OS === OperatingSystem.Macintosh) { + partModifiersMask |= KeyMod.WinCtrl; + } + + if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_W) { + console.warn('Ctrl/Cmd+W keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId); + + return true; + } + + if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_N) { + console.warn('Ctrl/Cmd+N keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId); + + return true; + } + + if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_T) { + console.warn('Ctrl/Cmd+T keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId); + + return true; + } + + if ((partModifiersMask & modifiersMask) === (KeyMod.CtrlCmd | KeyMod.Alt) && (part.keyCode === KeyCode.LeftArrow || part.keyCode === KeyCode.RightArrow)) { + console.warn('Ctrl/Cmd+Arrow keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId); + + return true; + } + + if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode >= KeyCode.KEY_0 && part.keyCode <= KeyCode.KEY_9) { + console.warn('Ctrl/Cmd+Num keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId); + + return true; + } + } + + return false; + } + public resolveKeybinding(kb: Keybinding): ResolvedKeybinding[] { return this._keyboardMapper.resolveKeybinding(kb); } diff --git a/src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts b/src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts index b459f0e5a9a..daa12488e96 100644 --- a/src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts +++ b/src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts @@ -17,6 +17,7 @@ import { KeyCodeUtils, KeyCode } from 'vs/base/common/keyCodes'; import { IMacLinuxKeyboardMapping, MacLinuxKeyboardMapper } from 'vs/workbench/services/keybinding/common/macLinuxKeyboardMapper'; import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent'; import { KeyboardLayoutProvider } from 'vs/workbench/services/keybinding/browser/keyboardLayoutProvider'; +import { INavigatorWithKeyboard } from 'vs/workbench/services/keybinding/common/navigatorKeyboard'; export class BrowserKeyboardMapperFactory { public static readonly INSTANCE = new BrowserKeyboardMapperFactory(); @@ -40,8 +41,8 @@ export class BrowserKeyboardMapperFactory { this._onKeyboardLayoutChanged(); }); - if ((navigator as any).keyboard && (navigator as any).keyboard.addEventListener) { - (navigator as any).keyboard.addEventListener('layoutchange', () => { + if ((navigator).keyboard && (navigator).keyboard.addEventListener) { + (navigator).keyboard.addEventListener!('layoutchange', () => { // Update user keyboard map settings this.getBrowserKeyMap().then((keymap: IKeyboardMapping) => { if (KeyboardLayoutProvider.INSTANCE.isActive(keymap)) { diff --git a/src/vs/workbench/services/keybinding/common/navigatorKeyboard.ts b/src/vs/workbench/services/keybinding/common/navigatorKeyboard.ts new file mode 100644 index 00000000000..51e8de9f790 --- /dev/null +++ b/src/vs/workbench/services/keybinding/common/navigatorKeyboard.ts @@ -0,0 +1,15 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +export interface IKeyboard { + getLayoutMap(): Promise; + lock(keyCodes?: string[]): Promise; + unlock(): void; + addEventListener?(type: string, listener: () => void): void; + +} +export type INavigatorWithKeyboard = Navigator & { + keyboard: IKeyboard +}; \ No newline at end of file -- GitLab