move keybinding validator out of keybindingRegistry

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);

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() && (<any>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);
 	}

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 ((<INavigatorWithKeyboard>navigator).keyboard && (<INavigatorWithKeyboard>navigator).keyboard.addEventListener) {
+			(<INavigatorWithKeyboard>navigator).keyboard.addEventListener!('layoutchange', () => {
 				// Update user keyboard map settings
 				this.getBrowserKeyMap().then((keymap: IKeyboardMapping) => {
 					if (KeyboardLayoutProvider.INSTANCE.isActive(keymap)) {

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

+/*---------------------------------------------------------------------------------------------
+ *  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<Object>;
+	lock(keyCodes?: string[]): Promise<void>;
+	unlock(): void;
+	addEventListener?(type: string, listener: () => void): void;
+
+}
+export type INavigatorWithKeyboard = Navigator & {
+	keyboard: IKeyboard
+};
\ No newline at end of file