Prepare KeybindingService for dispatching based on code

src/vs/base/browser/keyboardEvent.ts

@@ -174,11 +174,12 @@ export interface IKeyboardEvent {
 	readonly altKey: boolean;
 	readonly metaKey: boolean;
 	readonly keyCode: KeyCode;
+	readonly code: string;
 
 	/**
 	 * @internal
 	 */
-	toRuntimeKeybinding(): SimpleKeybinding;
+	toKeybinding(): SimpleKeybinding;
 	equals(keybinding: number): boolean;
 
 	preventDefault(): void;
@@ -200,6 +201,7 @@ export class StandardKeyboardEvent implements IKeyboardEvent {
 	public readonly altKey: boolean;
 	public readonly metaKey: boolean;
 	public readonly keyCode: KeyCode;
+	public readonly code: string;
 
 	private _asKeybinding: number;
 	private _asRuntimeKeybinding: SimpleKeybinding;
@@ -215,6 +217,7 @@ export class StandardKeyboardEvent implements IKeyboardEvent {
 		this.altKey = e.altKey;
 		this.metaKey = e.metaKey;
 		this.keyCode = extractKeyCode(e);
+		this.code = e.code;
 
 		// console.info(e.type + ": keyCode: " + e.keyCode + ", which: " + e.which + ", charCode: " + e.charCode + ", detail: " + e.detail + " ====> " + this.keyCode + ' -- ' + KeyCode[this.keyCode]);
 
@@ -241,7 +244,7 @@ export class StandardKeyboardEvent implements IKeyboardEvent {
 		}
 	}
 
-	public toRuntimeKeybinding(): SimpleKeybinding {
+	public toKeybinding(): SimpleKeybinding {
 		return this._asRuntimeKeybinding;
 	}
 

src/vs/base/parts/tree/browser/treeDefaults.ts

@@ -264,7 +264,7 @@ export class DefaultController implements _.IController {
 	}
 
 	private onKey(bindings: KeybindingDispatcher, tree: _.ITree, event: IKeyboardEvent): boolean {
-		var handler = bindings.dispatch(event.toRuntimeKeybinding());
+		var handler = bindings.dispatch(event.toKeybinding());
 		if (handler) {
 			if (handler(tree, event)) {
 				event.preventDefault();

src/vs/editor/browser/standalone/simpleServices.ts

@@ -14,7 +14,7 @@ import { ICommandService, ICommand, ICommandEvent, ICommandHandler, CommandsRegi
 import { AbstractKeybindingService } from 'vs/platform/keybinding/common/abstractKeybindingService';
 import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
 import { KeybindingResolver } from 'vs/platform/keybinding/common/keybindingResolver';
-import { IKeybindingEvent, KeybindingSource } from 'vs/platform/keybinding/common/keybinding';
+import { IKeybindingEvent, KeybindingSource, IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
 import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 import { IConfirmation, IMessageService } from 'vs/platform/message/common/message';
 import * as editorCommon from 'vs/editor/common/editorCommon';
@@ -32,7 +32,7 @@ import { KeybindingsRegistry, IKeybindingItem } from 'vs/platform/keybinding/com
 import { MenuId, IMenu, IMenuService } from 'vs/platform/actions/common/actions';
 import { Menu } from 'vs/platform/actions/common/menu';
 import { ITelemetryService, ITelemetryExperiments, ITelemetryInfo } from 'vs/platform/telemetry/common/telemetry';
-import { ResolvedKeybinding, Keybinding, createKeybinding } from 'vs/base/common/keyCodes';
+import { ResolvedKeybinding, Keybinding, createKeybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
 import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { OS } from 'vs/base/common/platform';
 
@@ -323,7 +323,7 @@ export class StandaloneKeybindingService extends AbstractKeybindingService {
 
 		this.toDispose.push(dom.addDisposableListener(domNode, dom.EventType.KEY_DOWN, (e: KeyboardEvent) => {
 			let keyEvent = new StandardKeyboardEvent(e);
-			let shouldPreventDefault = this._dispatch(keyEvent.toRuntimeKeybinding(), keyEvent.target);
+			let shouldPreventDefault = this._dispatch(keyEvent, keyEvent.target);
 			if (shouldPreventDefault) {
 				keyEvent.preventDefault();
 			}
@@ -387,7 +387,7 @@ export class StandaloneKeybindingService extends AbstractKeybindingService {
 			const item = items[i];
 			const when = (item.when ? item.when.normalize() : null);
 			const keybinding = item.keybinding;
-			const resolvedKeybinding = (keybinding ? this._createResolvedKeybinding(keybinding) : null);
+			const resolvedKeybinding = (keybinding ? this.resolveKeybinding(keybinding) : null);
 
 			result[resultLen++] = new ResolvedKeybindingItem(resolvedKeybinding, item.command, item.commandArgs, when, isDefault);
 		}
@@ -395,10 +395,20 @@ export class StandaloneKeybindingService extends AbstractKeybindingService {
 		return result;
 	}
 
-	protected _createResolvedKeybinding(kb: Keybinding): ResolvedKeybinding {
+	public resolveKeybinding(kb: Keybinding): ResolvedKeybinding {
 		return new USLayoutResolvedKeybinding(kb, OS);
 	}
 
+	public resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding {
+		let keybinding = new SimpleKeybinding(
+			keyboardEvent.ctrlKey,
+			keyboardEvent.shiftKey,
+			keyboardEvent.altKey,
+			keyboardEvent.metaKey,
+			keyboardEvent.keyCode
+		);
+		return this.resolveKeybinding(keybinding);
+	}
 }
 
 export class SimpleConfigurationService implements IConfigurationService {

src/vs/editor/contrib/suggest/test/common/suggestModel.test.ts

@@ -17,12 +17,12 @@ import { MockCodeEditor, MockScopeLocation } from 'vs/editor/test/common/mocks/m
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
 import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
-import { MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { ITelemetryService } from 'vs/platform/telemetry/common/telemetry';
 import { NullTelemetryService } from 'vs/platform/telemetry/common/telemetryUtils';
 
 function createMockEditor(model: Model): MockCodeEditor {
-	const contextKeyService = new MockKeybindingService();
+	const contextKeyService = new MockContextKeyService();
 	const telemetryService = NullTelemetryService;
 	const instantiationService = new InstantiationService(new ServiceCollection(
 		[IContextKeyService, contextKeyService],

src/vs/editor/test/browser/standalone/simpleServices.test.ts

@@ -9,17 +9,14 @@ import { ContextKeyService } from 'vs/platform/contextkey/browser/contextKeyServ
 import { SimpleConfigurationService, SimpleMessageService, StandaloneKeybindingService, StandaloneCommandService } from 'vs/editor/browser/standalone/simpleServices';
 import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
-import { KeyCode, SimpleKeybinding } from 'vs/base/common/keyCodes';
-import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
+import { KeyCode } from 'vs/base/common/keyCodes';
+import { IKeyboardEvent } from "vs/platform/keybinding/common/keybinding";
 
 suite('StandaloneKeybindingService', () => {
 
 	class TestStandaloneKeybindingService extends StandaloneKeybindingService {
-		public dispatch(e: IKeyboardEvent): void {
-			let shouldPreventDefault = super._dispatch(e.toRuntimeKeybinding(), e.target);
-			if (shouldPreventDefault) {
-				e.preventDefault();
-			}
+		public testDispatch(e: IKeyboardEvent): void {
+			super._dispatch(e, null);
 		}
 	}
 
@@ -45,9 +42,13 @@ suite('StandaloneKeybindingService', () => {
 			commandInvoked = true;
 		}, null);
 
-		keybindingService.dispatch(<any>{
-			toRuntimeKeybinding: () => new SimpleKeybinding(false, false, false, false, KeyCode.F9),
-			preventDefault: () => { }
+		keybindingService.testDispatch({
+			ctrlKey: false,
+			shiftKey: false,
+			altKey: false,
+			metaKey: false,
+			keyCode: KeyCode.F9,
+			code: null
 		});
 
 		assert.ok(commandInvoked, 'command invoked');

src/vs/editor/test/common/mocks/mockCodeEditor.ts

@@ -8,7 +8,7 @@ import { EventEmitter, IEventEmitter } from 'vs/base/common/eventEmitter';
 import { ServiceCollection } from 'vs/platform/instantiation/common/serviceCollection';
 import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService';
 import { IContextKeyService, IContextKeyServiceTarget } from 'vs/platform/contextkey/common/contextkey';
-import { MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { CommonCodeEditor } from 'vs/editor/common/commonCodeEditor';
 import { CommonEditorConfiguration } from 'vs/editor/common/config/commonEditorConfig';
 import { Cursor } from 'vs/editor/common/controller/cursor';
@@ -87,7 +87,7 @@ export function withMockCodeEditor(text: string[], options: editorCommon.ICodeEd
 
 export function mockCodeEditor(text: string[], options: editorCommon.ICodeEditorWidgetCreationOptions): CommonCodeEditor {
 
-	let contextKeyService = new MockKeybindingService();
+	let contextKeyService = new MockContextKeyService();
 
 	let services = new ServiceCollection();
 	services.set(IContextKeyService, contextKeyService);

src/vs/monaco.d.ts

@@ -380,6 +380,7 @@ declare module monaco {
         readonly altKey: boolean;
         readonly metaKey: boolean;
         readonly keyCode: KeyCode;
+        readonly code: string;
         equals(keybinding: number): boolean;
         preventDefault(): void;
         stopPropagation(): void;

src/vs/platform/actions/test/common/menuService.test.ts

@@ -9,7 +9,7 @@ import { MenuRegistry, MenuId } from 'vs/platform/actions/common/actions';
 import { MenuService } from 'vs/platform/actions/common/menuService';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import { NullCommandService } from 'vs/platform/commands/common/commands';
-import { MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { AbstractExtensionService, ActivatedExtension } from 'vs/platform/extensions/common/abstractExtensionService';
 
 // --- service instances
@@ -26,7 +26,7 @@ const extensionService = new class extends AbstractExtensionService<ActivatedExt
 	}
 }(true);
 
-const contextKeyService = new class extends MockKeybindingService {
+const contextKeyService = new class extends MockContextKeyService {
 	contextMatchesRules() {
 		return true;
 	}

src/vs/platform/keybinding/common/abstractKeybindingService.ts

@@ -5,12 +5,12 @@
 'use strict';
 
 import * as nls from 'vs/nls';
-import { ResolvedKeybinding, Keybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
+import { ResolvedKeybinding, Keybinding } from 'vs/base/common/keyCodes';
 import { IDisposable, dispose } from 'vs/base/common/lifecycle';
 import Severity from 'vs/base/common/severity';
 import { ICommandService } from 'vs/platform/commands/common/commands';
 import { KeybindingResolver, IResolveResult } from 'vs/platform/keybinding/common/keybindingResolver';
-import { IKeybindingEvent, IKeybindingService, IKeybindingItem2, KeybindingSource } from 'vs/platform/keybinding/common/keybinding';
+import { IKeybindingEvent, IKeybindingService, IKeybindingItem2, KeybindingSource, IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
 import { IContextKeyService, IContextKeyServiceTarget } from 'vs/platform/contextkey/common/contextkey';
 import { IStatusbarService } from 'vs/platform/statusbar/common/statusbar';
 import { IMessageService } from 'vs/platform/message/common/message';
@@ -56,16 +56,13 @@ export abstract class AbstractKeybindingService implements IKeybindingService {
 		this.toDispose = dispose(this.toDispose);
 	}
 
-	protected abstract _getResolver(): KeybindingResolver;
-	protected abstract _createResolvedKeybinding(kb: Keybinding): ResolvedKeybinding;
-
 	get onDidUpdateKeybindings(): Event<IKeybindingEvent> {
 		return this._onDidUpdateKeybindings ? this._onDidUpdateKeybindings.event : Event.None; // Sinon stubbing walks properties on prototype
 	}
 
-	public resolveKeybinding(keybinding: Keybinding): ResolvedKeybinding {
-		return this._createResolvedKeybinding(keybinding);
-	}
+	protected abstract _getResolver(): KeybindingResolver;
+	public abstract resolveKeybinding(keybinding: Keybinding): ResolvedKeybinding;
+	public abstract resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding;
 
 	public getDefaultKeybindings(): string {
 		return '';
@@ -96,32 +93,40 @@ export abstract class AbstractKeybindingService implements IKeybindingService {
 		return result.resolvedKeybinding;
 	}
 
-	public resolve(keybinding: SimpleKeybinding, target: IContextKeyServiceTarget): IResolveResult {
-		if (keybinding.isModifierKey()) {
+	public softDispatch(e: IKeyboardEvent, target: IContextKeyServiceTarget): IResolveResult {
+		const keybinding = this.resolveKeyboardEvent(e);
+		if (keybinding.isChord()) {
+			console.warn('Unexpected keyboard event mapped to a chord');
+			return null;
+		}
+		const [firstPart,] = keybinding.getDispatchParts();
+		if (firstPart === null) {
+			// cannot be dispatched, probably only modifier keys
 			return null;
 		}
 
 		const contextValue = this._contextKeyService.getContextValue(target);
 		const currentChord = this._currentChord ? this._currentChord.keypress : null;
-		const resolvedKeybinding = this._createResolvedKeybinding(keybinding);
-		const [firstPart,] = resolvedKeybinding.getDispatchParts();
-		// We know for a fact the chordPart is null since we're using a single keypress
 		return this._getResolver().resolve(contextValue, currentChord, firstPart);
 	}
 
-	protected _dispatch(keybinding: SimpleKeybinding, target: IContextKeyServiceTarget): boolean {
-		// Check modifier key here and cancel early, it's also checked in resolve as the function
-		// is used externally.
+	protected _dispatch(e: IKeyboardEvent, target: IContextKeyServiceTarget): boolean {
 		let shouldPreventDefault = false;
-		if (keybinding.isModifierKey()) {
+
+		const keybinding = this.resolveKeyboardEvent(e);
+		if (keybinding.isChord()) {
+			console.warn('Unexpected keyboard event mapped to a chord');
+			return null;
+		}
+		const [firstPart,] = keybinding.getDispatchParts();
+		if (firstPart === null) {
+			// cannot be dispatched, probably only modifier keys
 			return shouldPreventDefault;
 		}
 
 		const contextValue = this._contextKeyService.getContextValue(target);
 		const currentChord = this._currentChord ? this._currentChord.keypress : null;
-		const resolvedKeybinding = this._createResolvedKeybinding(keybinding);
-		const [firstPart,] = resolvedKeybinding.getDispatchParts();
-		const keypressLabel = resolvedKeybinding.getLabel();
+		const keypressLabel = keybinding.getLabel();
 		const resolveResult = this._getResolver().resolve(contextValue, currentChord, firstPart);
 
 		if (resolveResult && resolveResult.enterChord) {

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

@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import { ResolvedKeybinding, Keybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
+import { ResolvedKeybinding, Keybinding, KeyCode } from 'vs/base/common/keyCodes';
 import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
 import { ContextKeyExpr, IContextKeyServiceTarget } from 'vs/platform/contextkey/common/contextkey';
 import { IResolveResult } from 'vs/platform/keybinding/common/keybindingResolver';
@@ -34,6 +34,15 @@ export interface IKeybindingEvent {
 	keybindings?: IUserFriendlyKeybinding[];
 }
 
+export interface IKeyboardEvent {
+	readonly ctrlKey: boolean;
+	readonly shiftKey: boolean;
+	readonly altKey: boolean;
+	readonly metaKey: boolean;
+	readonly keyCode: KeyCode;
+	readonly code: string;
+}
+
 export let IKeybindingService = createDecorator<IKeybindingService>('keybindingService');
 
 export interface IKeybindingService {
@@ -43,9 +52,12 @@ export interface IKeybindingService {
 
 	resolveKeybinding(keybinding: Keybinding): ResolvedKeybinding;
 
-	getDefaultKeybindings(): string;
+	resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding;
 
-	getKeybindings(): IKeybindingItem2[];
+	/**
+	 * Resolve and dispatch `keyboardEvent`, but do not invoke the command or change inner state.
+	 */
+	softDispatch(keyboardEvent: IKeyboardEvent, target: IContextKeyServiceTarget): IResolveResult;
 
 	/**
 	 * Look up keybindings for a command.
@@ -59,8 +71,10 @@ export interface IKeybindingService {
 	 */
 	lookupKeybinding(commandId: string): ResolvedKeybinding;
 
-	customKeybindingsCount(): number;
+	getDefaultKeybindings(): string;
 
-	resolve(keybinding: SimpleKeybinding, target: IContextKeyServiceTarget): IResolveResult;
+	getKeybindings(): IKeybindingItem2[];
+
+	customKeybindingsCount(): number;
 }
 

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

@@ -157,17 +157,17 @@ class KeybindingsRegistryImpl implements IKeybindingsRegistry {
 		}
 	}
 
-	private registerDefaultKeybinding(keybinding: number, commandId: string, weight1: number, weight2: number, when: ContextKeyExpr): void {
-		const runtimeKeybinding = createKeybinding(keybinding, OS);
+	private registerDefaultKeybinding(kb: number, commandId: string, weight1: number, weight2: number, when: ContextKeyExpr): void {
+		const keybinding = createKeybinding(kb, OS);
 		if (OS === OperatingSystem.Windows) {
-			if (runtimeKeybinding.type === KeybindingType.Chord) {
-				this._assertNoCtrlAlt(runtimeKeybinding.firstPart, commandId);
+			if (keybinding.type === KeybindingType.Chord) {
+				this._assertNoCtrlAlt(keybinding.firstPart, commandId);
 			} else {
-				this._assertNoCtrlAlt(runtimeKeybinding, commandId);
+				this._assertNoCtrlAlt(keybinding, commandId);
 			}
 		}
 		this._keybindings.push({
-			keybinding: runtimeKeybinding,
+			keybinding: keybinding,
 			command: commandId,
 			commandArgs: null,
 			when: when,

src/vs/platform/keybinding/common/usLayoutResolvedKeybinding.ts

@@ -149,6 +149,9 @@ export class USLayoutResolvedKeybinding extends ResolvedKeybinding {
 	}
 
 	public static getDispatchStr(keybinding: SimpleKeybinding): string {
+		if (keybinding.isModifierKey()) {
+			return null;
+		}
 		let result = '';
 
 		if (keybinding.ctrlKey) {

src/vs/platform/keybinding/test/common/abstractKeybindingService.test.ts

@@ -5,7 +5,7 @@
 'use strict';
 
 import * as assert from 'assert';
-import { ResolvedKeybinding, KeyCode, KeyMod, KeyChord, Keybinding, createKeybinding, createSimpleKeybinding } from 'vs/base/common/keyCodes';
+import { ResolvedKeybinding, KeyCode, KeyMod, KeyChord, Keybinding, createKeybinding, createSimpleKeybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
 import { AbstractKeybindingService } from 'vs/platform/keybinding/common/abstractKeybindingService';
 import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
 import { IDisposable } from 'vs/base/common/lifecycle';
@@ -18,6 +18,7 @@ import { IMessageService } from 'vs/platform/message/common/message';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { ResolvedKeybindingItem } from 'vs/platform/keybinding/common/resolvedKeybindingItem';
 import { OS } from 'vs/base/common/platform';
+import { IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
 
 suite('AbstractKeybindingService', () => {
 
@@ -39,12 +40,31 @@ suite('AbstractKeybindingService', () => {
 			return this._resolver;
 		}
 
-		protected _createResolvedKeybinding(kb: Keybinding): ResolvedKeybinding {
+		public resolveKeybinding(kb: Keybinding): ResolvedKeybinding {
 			return new USLayoutResolvedKeybinding(kb, OS);
 		}
 
-		public dispatch(keybinding: number): boolean {
-			return this._dispatch(createSimpleKeybinding(keybinding, OS), null);
+		public resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding {
+			let keybinding = new SimpleKeybinding(
+				keyboardEvent.ctrlKey,
+				keyboardEvent.shiftKey,
+				keyboardEvent.altKey,
+				keyboardEvent.metaKey,
+				keyboardEvent.keyCode
+			);
+			return this.resolveKeybinding(keybinding);
+		}
+
+		public testDispatch(kb: number): boolean {
+			const keybinding = createSimpleKeybinding(kb, OS);
+			return this._dispatch({
+				ctrlKey: keybinding.ctrlKey,
+				shiftKey: keybinding.shiftKey,
+				altKey: keybinding.altKey,
+				metaKey: keybinding.metaKey,
+				keyCode: keybinding.keyCode,
+				code: null
+			}, null);
 		}
 	}
 
@@ -153,7 +173,7 @@ suite('AbstractKeybindingService', () => {
 		]);
 
 		// send Ctrl/Cmd + K
-		let shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		let shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, []);
 		assert.deepEqual(showMessageCalls, []);
@@ -167,7 +187,7 @@ suite('AbstractKeybindingService', () => {
 		statusMessageCallsDisposed = [];
 
 		// send backspace
-		shouldPreventDefault = kbService.dispatch(KeyCode.Backspace);
+		shouldPreventDefault = kbService.testDispatch(KeyCode.Backspace);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, []);
 		assert.deepEqual(showMessageCalls, []);
@@ -183,7 +203,7 @@ suite('AbstractKeybindingService', () => {
 		statusMessageCallsDisposed = [];
 
 		// send backspace
-		shouldPreventDefault = kbService.dispatch(KeyCode.Backspace);
+		shouldPreventDefault = kbService.testDispatch(KeyCode.Backspace);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'simpleCommand',
@@ -200,7 +220,7 @@ suite('AbstractKeybindingService', () => {
 		kbService.dispose();
 	});
 
-	test('issue #16833: Keybinding service should not dispatch on modifier keys', () => {
+	test('issue #16833: Keybinding service should not testDispatch on modifier keys', () => {
 
 		let kbService = createTestKeybindingService([
 			kbItem(KeyCode.Ctrl, 'nope'),
@@ -215,7 +235,7 @@ suite('AbstractKeybindingService', () => {
 		]);
 
 		function assertIsIgnored(keybinding: number): void {
-			let shouldPreventDefault = kbService.dispatch(keybinding);
+			let shouldPreventDefault = kbService.testDispatch(keybinding);
 			assert.equal(shouldPreventDefault, false);
 			assert.deepEqual(executeCommandCalls, []);
 			assert.deepEqual(showMessageCalls, []);
@@ -252,7 +272,7 @@ suite('AbstractKeybindingService', () => {
 		currentContextValue = {
 			key1: true
 		};
-		let shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		let shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'simpleCommand',
@@ -268,7 +288,7 @@ suite('AbstractKeybindingService', () => {
 
 		// send Ctrl/Cmd + K
 		currentContextValue = {};
-		shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, []);
 		assert.deepEqual(showMessageCalls, []);
@@ -283,7 +303,7 @@ suite('AbstractKeybindingService', () => {
 
 		// send Ctrl/Cmd + X
 		currentContextValue = {};
-		shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_X);
+		shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_X);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'chordCommand',
@@ -312,7 +332,7 @@ suite('AbstractKeybindingService', () => {
 
 		// send Ctrl/Cmd + K
 		currentContextValue = {};
-		let shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		let shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'simpleCommand',
@@ -330,7 +350,7 @@ suite('AbstractKeybindingService', () => {
 		currentContextValue = {
 			key1: true
 		};
-		shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, true);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'simpleCommand',
@@ -348,7 +368,7 @@ suite('AbstractKeybindingService', () => {
 		currentContextValue = {
 			key1: true
 		};
-		shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_X);
+		shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_X);
 		assert.equal(shouldPreventDefault, false);
 		assert.deepEqual(executeCommandCalls, []);
 		assert.deepEqual(showMessageCalls, []);
@@ -370,7 +390,7 @@ suite('AbstractKeybindingService', () => {
 
 		// send Ctrl/Cmd + K
 		currentContextValue = {};
-		let shouldPreventDefault = kbService.dispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
+		let shouldPreventDefault = kbService.testDispatch(KeyMod.CtrlCmd | KeyCode.KEY_K);
 		assert.equal(shouldPreventDefault, false);
 		assert.deepEqual(executeCommandCalls, [{
 			commandId: 'simpleCommand',

src/vs/platform/keybinding/test/common/mockKeybindingService.ts

@@ -6,7 +6,7 @@
 
 import { ResolvedKeybinding, Keybinding, SimpleKeybinding } from 'vs/base/common/keyCodes';
 import Event from 'vs/base/common/event';
-import { IKeybindingService, IKeybindingEvent, IKeybindingItem2 } from 'vs/platform/keybinding/common/keybinding';
+import { IKeybindingService, IKeybindingEvent, IKeybindingItem2, IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
 import { IContextKey, IContextKeyService, IContextKeyServiceTarget, ContextKeyExpr } from 'vs/platform/contextkey/common/contextkey';
 import { IResolveResult } from 'vs/platform/keybinding/common/keybindingResolver';
 import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayoutResolvedKeybinding';
@@ -36,7 +36,7 @@ class MockKeybindingContextKey<T> implements IContextKey<T> {
 	}
 }
 
-export class MockKeybindingService implements IContextKeyService {
+export class MockContextKeyService implements IContextKeyService {
 	public _serviceBrand: any;
 
 	public dispose(): void { }
@@ -61,7 +61,7 @@ export class MockKeybindingService implements IContextKeyService {
 	}
 }
 
-export class MockKeybindingService2 implements IKeybindingService {
+export class MockKeybindingService implements IKeybindingService {
 	public _serviceBrand: any;
 
 	public get onDidUpdateKeybindings(): Event<IKeybindingEvent> {
@@ -80,6 +80,17 @@ export class MockKeybindingService2 implements IKeybindingService {
 		return new USLayoutResolvedKeybinding(keybinding, OS);
 	}
 
+	public resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding {
+		let keybinding = new SimpleKeybinding(
+			keyboardEvent.ctrlKey,
+			keyboardEvent.shiftKey,
+			keyboardEvent.altKey,
+			keyboardEvent.metaKey,
+			keyboardEvent.keyCode
+		);
+		return this.resolveKeybinding(keybinding);
+	}
+
 	public lookupKeybindings(commandId: string): ResolvedKeybinding[] {
 		return [];
 	}
@@ -92,7 +103,7 @@ export class MockKeybindingService2 implements IKeybindingService {
 		return 0;
 	}
 
-	public resolve(keybinding: SimpleKeybinding, target: IContextKeyServiceTarget): IResolveResult {
+	public softDispatch(keybinding: IKeyboardEvent, target: IContextKeyServiceTarget): IResolveResult {
 		return null;
 	}
 }

src/vs/workbench/parts/preferences/browser/keybindingWidgets.ts

@@ -58,7 +58,7 @@ class KeybindingInputWidget extends Widget {
 	}
 
 	private printKeybinding(keyboardEvent: IKeyboardEvent): void {
-		const keybinding = this.keybindingService.resolveKeybinding(keyboardEvent.toRuntimeKeybinding());
+		const keybinding = this.keybindingService.resolveKeyboardEvent(keyboardEvent);
 		this.inputBox.value = keybinding.getUserSettingsLabel().toLowerCase();
 		this.inputBox.inputElement.title = 'keyCode: ' + keyboardEvent.browserEvent.keyCode;
 		this._onKeybinding.fire(keybinding);

src/vs/workbench/parts/terminal/electron-browser/terminalInstance.ts

@@ -227,8 +227,7 @@ export class TerminalInstance implements ITerminalInstance {
 			// Skip processing by xterm.js of keyboard events that resolve to commands described
 			// within commandsToSkipShell
 			const standardKeyboardEvent = new StandardKeyboardEvent(event);
-			const keybinding = standardKeyboardEvent.toRuntimeKeybinding();
-			const resolveResult = this._keybindingService.resolve(keybinding, standardKeyboardEvent.target);
+			const resolveResult = this._keybindingService.softDispatch(standardKeyboardEvent, standardKeyboardEvent.target);
 			if (resolveResult && this._skipTerminalCommands.some(k => k === resolveResult.commandId)) {
 				event.preventDefault();
 				return false;

src/vs/workbench/parts/terminal/test/electron-browser/terminalInstance.test.ts

@@ -15,7 +15,7 @@ import { TerminalInstance } from 'vs/workbench/parts/terminal/electron-browser/t
 import { IShellLaunchConfig } from 'vs/workbench/parts/terminal/common/terminal';
 import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
 import { TestMessageService, TestContextService } from 'vs/workbench/test/workbenchTestServices';
-import { MockKeybindingService, MockKeybindingService2 } from 'vs/platform/keybinding/test/common/mockKeybindingService';
+import { MockContextKeyService, MockKeybindingService } from 'vs/platform/keybinding/test/common/mockKeybindingService';
 import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
 import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 
@@ -81,8 +81,8 @@ suite('Workbench - TerminalInstance', () => {
 		let configHelper: { config: { cwd: string } };
 
 		setup(() => {
-			let contextKeyService = new MockKeybindingService();
-			let keybindingService = new MockKeybindingService2();
+			let contextKeyService = new MockContextKeyService();
+			let keybindingService = new MockKeybindingService();
 			let terminalFocusContextKey = contextKeyService.createKey('test', false);
 			instantiationService = new TestInstantiationService();
 			instantiationService.stub(IMessageService, new TestMessageService());

src/vs/workbench/services/keybinding/electron-browser/keybindingService.ts

@@ -18,7 +18,7 @@ import { USLayoutResolvedKeybinding } from 'vs/platform/keybinding/common/usLayo
 import { IStatusbarService } from 'vs/platform/statusbar/common/statusbar';
 import { KeybindingResolver } from 'vs/platform/keybinding/common/keybindingResolver';
 import { ICommandService } from 'vs/platform/commands/common/commands';
-import { IKeybindingEvent, IUserFriendlyKeybinding, KeybindingSource } from 'vs/platform/keybinding/common/keybinding';
+import { IKeybindingEvent, IUserFriendlyKeybinding, KeybindingSource, IKeyboardEvent } from 'vs/platform/keybinding/common/keybinding';
 import { ContextKeyExpr, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
 import { IKeybindingRule, IKeybindingItem, KeybindingsRegistry } from 'vs/platform/keybinding/common/keybindingsRegistry';
 import { Registry } from 'vs/platform/platform';
@@ -229,6 +229,9 @@ export class FancyResolvedKeybinding extends ResolvedKeybinding {
 	}
 
 	private _getDispatchPart(keybinding: SimpleKeybinding): string {
+		if (keybinding.isModifierKey()) {
+			return null;
+		}
 		let result = '';
 
 		if (keybinding.ctrlKey) {
@@ -291,7 +294,7 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
 
 		this.toDispose.push(dom.addDisposableListener(windowElement, dom.EventType.KEY_DOWN, (e: KeyboardEvent) => {
 			let keyEvent = new StandardKeyboardEvent(e);
-			let shouldPreventDefault = this._dispatch(keyEvent.toRuntimeKeybinding(), keyEvent.target);
+			let shouldPreventDefault = this._dispatch(keyEvent, keyEvent.target);
 			if (shouldPreventDefault) {
 				keyEvent.preventDefault();
 			}
@@ -339,7 +342,7 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
 			const item = items[i];
 			const when = (item.when ? item.when.normalize() : null);
 			const keybinding = item.keybinding;
-			const resolvedKeybinding = (keybinding ? this._createResolvedKeybinding(keybinding) : null);
+			const resolvedKeybinding = (keybinding ? this.resolveKeybinding(keybinding) : null);
 
 			result[resultLen++] = new ResolvedKeybindingItem(resolvedKeybinding, item.command, item.commandArgs, when, isDefault);
 		}
@@ -360,10 +363,21 @@ export class WorkbenchKeybindingService extends AbstractKeybindingService {
 		return extraUserKeybindings.map((k, i) => KeybindingIO.readKeybindingItem(k, i, OS));
 	}
 
-	protected _createResolvedKeybinding(kb: Keybinding): ResolvedKeybinding {
+	public resolveKeybinding(kb: Keybinding): ResolvedKeybinding {
 		return new FancyResolvedKeybinding(kb);
 	}
 
+	public resolveKeyboardEvent(keyboardEvent: IKeyboardEvent): ResolvedKeybinding {
+		let keybinding = new SimpleKeybinding(
+			keyboardEvent.ctrlKey,
+			keyboardEvent.shiftKey,
+			keyboardEvent.altKey,
+			keyboardEvent.metaKey,
+			keyboardEvent.keyCode
+		);
+		return this.resolveKeybinding(keybinding);
+	}
+
 	private _handleKeybindingsExtensionPointUser(isBuiltin: boolean, keybindings: ContributedKeyBinding | ContributedKeyBinding[], collector: ExtensionMessageCollector): boolean {
 		if (isContributedKeyBindingsArray(keybindings)) {
 			let commandAdded = false;