Skip to content

Commit 70bdfd7

Browse files
rebornixrebornix
committed
move keybinding validator out of keybindingRegistry
1 parent 3525505 commit 70bdfd7

4 files changed

Lines changed: 91 additions & 60 deletions

File tree

‎src/vs/platform/keybinding/common/keybindingsRegistry.ts‎

Lines changed: 6 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -130,22 +130,18 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry {
130130
const actualKb = KeybindingsRegistryImpl.bindToCurrentPlatform(rule);
131131

132132
if (actualKb && actualKb.primary) {
133-
if (!this._assertBrowserConflicts(actualKb.primary, rule.id)) {
134-
const kk = createKeybinding(actualKb.primary, OS);
135-
if (kk) {
136-
this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, 0, rule.when);
137-
}
133+
const kk = createKeybinding(actualKb.primary, OS);
134+
if (kk) {
135+
this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, 0, rule.when);
138136
}
139137
}
140138

141139
if (actualKb && Array.isArray(actualKb.secondary)) {
142140
for (let i = 0, len = actualKb.secondary.length; i < len; i++) {
143141
const k = actualKb.secondary[i];
144-
if (!this._assertBrowserConflicts(k, rule.id)) {
145-
const kk = createKeybinding(k, OS);
146-
if (kk) {
147-
this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, -i - 1, rule.when);
148-
}
142+
const kk = createKeybinding(k, OS);
143+
if (kk) {
144+
this._registerDefaultKeybinding(kk, rule.id, undefined, rule.weight, -i - 1, rule.when);
149145
}
150146
}
151147
}
@@ -212,54 +208,6 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry {
212208
}
213209
}
214210

215-
private _assertBrowserConflicts(keybinding: number, commandId: string): boolean {
216-
if (!isWeb) {
217-
return false;
218-
}
219-
220-
const firstPart = (keybinding & 0x0000FFFF) >>> 0;
221-
const chordPart = (keybinding & 0xFFFF0000) >>> 16;
222-
const modifiersMask = KeyMod.CtrlCmd | KeyMod.Alt | KeyMod.Shift;
223-
224-
for (let part of [firstPart, chordPart]) {
225-
if ((part & modifiersMask) === 0) {
226-
continue;
227-
}
228-
229-
if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_W) {
230-
console.warn('Ctrl/Cmd+W keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId);
231-
232-
return true;
233-
}
234-
235-
if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_N) {
236-
console.warn('Ctrl/Cmd+N keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId);
237-
238-
return true;
239-
}
240-
241-
if ((part & modifiersMask) === KeyMod.CtrlCmd && (part & 0x000000FF) === KeyCode.KEY_T) {
242-
console.warn('Ctrl/Cmd+T keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId);
243-
244-
return true;
245-
}
246-
247-
if ((part & modifiersMask) === (KeyMod.CtrlCmd | KeyMod.Alt) && ((part & 0x000000FF) === KeyCode.LeftArrow || (part & 0x000000FF) === KeyCode.RightArrow)) {
248-
console.warn('Ctrl/Cmd+Arrow keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId);
249-
250-
return true;
251-
}
252-
253-
if ((part & modifiersMask) === KeyMod.CtrlCmd && ((part & 0x000000FF) >= KeyCode.KEY_0 && (part & 0x000000FF) <= KeyCode.KEY_9)) {
254-
console.warn('Ctrl/Cmd+Num keybindings should not be used by default in web. Offender: ', keybinding, ' for ', commandId);
255-
256-
return true;
257-
}
258-
}
259-
260-
return false;
261-
}
262-
263211
private _registerDefaultKeybinding(keybinding: Keybinding, commandId: string, commandArgs: any, weight1: number, weight2: number, when: ContextKeyExpr | null | undefined): void {
264212
if (OS === OperatingSystem.Windows) {
265213
this._assertNoCtrlAlt(keybinding.parts[0], commandId);

‎src/vs/workbench/services/keybinding/browser/keybindingService.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,10 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
279279
// This might be a removal keybinding item in user settings => accept it
280280
result[resultLen++] = new ResolvedKeybindingItem(undefined, item.command, item.commandArgs, when, isDefault);
281281
} else {
282+
if (this._assertBrowserConflicts(keybinding, item.command)) {
283+
continue;
284+
}
285+
282286
const resolvedKeybindings = this.resolveKeybinding(keybinding);
283287
for (const resolvedKeybinding of resolvedKeybindings) {
284288
result[resultLen++] = new ResolvedKeybindingItem(resolvedKeybinding, item.command, item.commandArgs, when, isDefault);
@@ -308,6 +312,69 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
308312
return result;
309313
}
310314

315+
private _assertBrowserConflicts(kb: Keybinding, commandId: string): boolean {
316+
if (browser.isFullscreen() && (<any>navigator).keyboard) {
317+
return false;
318+
}
319+
320+
for (let part of kb.parts) {
321+
if (!part.metaKey && !part.altKey && !part.ctrlKey && !part.shiftKey) {
322+
continue;
323+
}
324+
325+
const modifiersMask = KeyMod.CtrlCmd | KeyMod.Alt | KeyMod.Shift;
326+
327+
let partModifiersMask = 0;
328+
if (part.metaKey) {
329+
partModifiersMask |= KeyMod.CtrlCmd;
330+
}
331+
332+
if (part.shiftKey) {
333+
partModifiersMask |= KeyMod.Shift;
334+
}
335+
336+
if (part.altKey) {
337+
partModifiersMask |= KeyMod.Alt;
338+
}
339+
340+
if (part.ctrlKey && OS === OperatingSystem.Macintosh) {
341+
partModifiersMask |= KeyMod.WinCtrl;
342+
}
343+
344+
if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_W) {
345+
console.warn('Ctrl/Cmd+W keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId);
346+
347+
return true;
348+
}
349+
350+
if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_N) {
351+
console.warn('Ctrl/Cmd+N keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId);
352+
353+
return true;
354+
}
355+
356+
if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode === KeyCode.KEY_T) {
357+
console.warn('Ctrl/Cmd+T keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId);
358+
359+
return true;
360+
}
361+
362+
if ((partModifiersMask & modifiersMask) === (KeyMod.CtrlCmd | KeyMod.Alt) && (part.keyCode === KeyCode.LeftArrow || part.keyCode === KeyCode.RightArrow)) {
363+
console.warn('Ctrl/Cmd+Arrow keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId);
364+
365+
return true;
366+
}
367+
368+
if ((partModifiersMask & modifiersMask) === KeyMod.CtrlCmd && part.keyCode >= KeyCode.KEY_0 && part.keyCode <= KeyCode.KEY_9) {
369+
console.warn('Ctrl/Cmd+Num keybindings should not be used by default in web. Offender: ', kb.getHashCode(), ' for ', commandId);
370+
371+
return true;
372+
}
373+
}
374+
375+
return false;
376+
}
377+
311378
public resolveKeybinding(kb: Keybinding): ResolvedKeybinding[] {
312379
return this._keyboardMapper.resolveKeybinding(kb);
313380
}

‎src/vs/workbench/services/keybinding/browser/keyboardLayoutService.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { KeyCodeUtils, KeyCode } from 'vs/base/common/keyCodes';
1717
import { IMacLinuxKeyboardMapping, MacLinuxKeyboardMapper } from 'vs/workbench/services/keybinding/common/macLinuxKeyboardMapper';
1818
import { StandardKeyboardEvent } from 'vs/base/browser/keyboardEvent';
1919
import { KeyboardLayoutProvider } from 'vs/workbench/services/keybinding/browser/keyboardLayoutProvider';
20+
import { INavigatorWithKeyboard } from 'vs/workbench/services/keybinding/common/navigatorKeyboard';
2021

2122
export class BrowserKeyboardMapperFactory {
2223
public static readonly INSTANCE = new BrowserKeyboardMapperFactory();
@@ -40,8 +41,8 @@ export class BrowserKeyboardMapperFactory {
4041
this._onKeyboardLayoutChanged();
4142
});
4243

43-
if ((navigator as any).keyboard && (navigator as any).keyboard.addEventListener) {
44-
(navigator as any).keyboard.addEventListener('layoutchange', () => {
44+
if ((<INavigatorWithKeyboard>navigator).keyboard && (<INavigatorWithKeyboard>navigator).keyboard.addEventListener) {
45+
(<INavigatorWithKeyboard>navigator).keyboard.addEventListener!('layoutchange', () => {
4546
// Update user keyboard map settings
4647
this.getBrowserKeyMap().then((keymap: IKeyboardMapping) => {
4748
if (KeyboardLayoutProvider.INSTANCE.isActive(keymap)) {

‎src/vs/workbench/services/keybinding/common/navigatorKeyboard.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*---------------------------------------------------------------------------------------------
2+
* Copyright (c) Microsoft Corporation. All rights reserved.
3+
* Licensed under the MIT License. See License.txt in the project root for license information.
4+
*--------------------------------------------------------------------------------------------*/
5+
6+
export interface IKeyboard {
7+
getLayoutMap(): Promise<Object>;
8+
lock(keyCodes?: string[]): Promise<void>;
9+
unlock(): void;
10+
addEventListener?(type: string, listener: () => void): void;
11+
12+
}
13+
export type INavigatorWithKeyboard = Navigator & {
14+
keyboard: IKeyboard
15+
};

0 commit comments

Comments
 (0)