Skip to content

Commit 28dc74b

Browse files
authored
Don't pass on CKS change events from parent, when all changed keys are overridden by the child scoped CKS (microsoft#152775)
Fixes microsoft#125109
1 parent d119d65 commit 28dc74b

File tree

2 files changed

+94
-3
lines changed

2 files changed

+94
-3
lines changed

src/vs/platform/contextkey/browser/contextKeyService.ts

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const KEYBINDING_CONTEXT_ATTR = 'data-keybinding-context';
2222
export class Context implements IContext {
2323

2424
protected _parent: Context | null;
25-
protected _value: { [key: string]: any };
25+
protected _value: Record<string, any>;
2626
protected _id: number;
2727

2828
constructor(id: number, parent: Context | null) {
@@ -32,6 +32,10 @@ export class Context implements IContext {
3232
this._value['_contextId'] = id;
3333
}
3434

35+
public get value(): Record<string, any> {
36+
return { ...this._value };
37+
}
38+
3539
public setValue(key: string, value: any): boolean {
3640
// console.log('SET ' + key + ' = ' + value + ' ON ' + this._id);
3741
if (this._value[key] !== value) {
@@ -62,7 +66,7 @@ export class Context implements IContext {
6266
this._parent = parent;
6367
}
6468

65-
public collectAllValues(): { [key: string]: any } {
69+
public collectAllValues(): Record<string, any> {
6670
let result = this._parent ? this._parent.collectAllValues() : Object.create(null);
6771
result = { ...result, ...this._value };
6872
delete result['_contextId'];
@@ -247,6 +251,12 @@ class CompositeContextKeyChangeEvent implements IContextKeyChangeEvent {
247251
}
248252
}
249253

254+
function allEventKeysInContext(event: IContextKeyChangeEvent, context: Record<string, any>): boolean {
255+
return (event instanceof ArrayContextKeyChangeEvent && event.keys.every(key => key in context)) ||
256+
(event instanceof SimpleContextKeyChangeEvent && event.key in context) ||
257+
(event instanceof CompositeContextKeyChangeEvent && event.events.every(e => allEventKeysInContext(e, context)));
258+
}
259+
250260
export abstract class AbstractContextKeyService implements IContextKeyService {
251261
declare _serviceBrand: undefined;
252262

@@ -439,7 +449,14 @@ class ScopedContextKeyService extends AbstractContextKeyService {
439449

440450
private _updateParentChangeListener(): void {
441451
// Forward parent events to this listener. Parent will change.
442-
this._parentChangeListener.value = this._parent.onDidChangeContext(this._onDidChangeContext.fire, this._onDidChangeContext);
452+
this._parentChangeListener.value = this._parent.onDidChangeContext(e => {
453+
const thisContainer = this._parent.getContextValuesContainer(this._myContextId);
454+
const thisContextValues = thisContainer.value;
455+
456+
if (!allEventKeysInContext(e, thisContextValues)) {
457+
this._onDidChangeContext.fire(e);
458+
}
459+
});
443460
}
444461

445462
public dispose(): void {

src/vs/platform/contextkey/test/browser/contextkey.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
* Licensed under the MIT License. See License.txt in the project root for license information.
44
*--------------------------------------------------------------------------------------------*/
55
import * as assert from 'assert';
6+
import { DeferredPromise } from 'vs/base/common/async';
67
import { DisposableStore } from 'vs/base/common/lifecycle';
78
import { URI } from 'vs/base/common/uri';
89
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
@@ -73,4 +74,77 @@ suite('ContextKeyService', () => {
7374
const expr = ContextKeyExpr.in('notebookCellResource', 'jupyter.runByLineCells');
7475
assert.deepStrictEqual(contextKeyService.contextMatchesRules(expr), true);
7576
});
77+
78+
test('suppress update event from parent when one key is overridden by child', () => {
79+
const root = new ContextKeyService(new TestConfigurationService());
80+
const child = root.createScoped(document.createElement('div'));
81+
82+
root.createKey('testA', 1);
83+
child.createKey('testA', 4);
84+
85+
let fired = false;
86+
const event = child.onDidChangeContext(e => fired = true);
87+
root.setContext('testA', 10);
88+
assert.strictEqual(fired, false, 'Should not fire event when overridden key is updated in parent');
89+
event.dispose();
90+
});
91+
92+
test('suppress update event from parent when all keys are overridden by child', () => {
93+
const root = new ContextKeyService(new TestConfigurationService());
94+
const child = root.createScoped(document.createElement('div'));
95+
96+
root.createKey('testA', 1);
97+
root.createKey('testB', 2);
98+
root.createKey('testC', 3);
99+
100+
child.createKey('testA', 4);
101+
child.createKey('testB', 5);
102+
child.createKey('testD', 6);
103+
104+
let fired = false;
105+
const event = child.onDidChangeContext(e => fired = true);
106+
root.bufferChangeEvents(() => {
107+
root.setContext('testA', 10);
108+
root.setContext('testB', 20);
109+
root.setContext('testD', 30);
110+
});
111+
112+
assert.strictEqual(fired, false, 'Should not fire event when overridden key is updated in parent');
113+
event.dispose();
114+
});
115+
116+
test('pass through update event from parent when one key is not overridden by child', () => {
117+
const root = new ContextKeyService(new TestConfigurationService());
118+
const child = root.createScoped(document.createElement('div'));
119+
120+
root.createKey('testA', 1);
121+
root.createKey('testB', 2);
122+
root.createKey('testC', 3);
123+
124+
child.createKey('testA', 4);
125+
child.createKey('testB', 5);
126+
child.createKey('testD', 6);
127+
128+
const def = new DeferredPromise();
129+
child.onDidChangeContext(e => {
130+
try {
131+
assert.ok(e.affectsSome(new Set(['testA'])), 'testA changed');
132+
assert.ok(e.affectsSome(new Set(['testB'])), 'testB changed');
133+
assert.ok(e.affectsSome(new Set(['testC'])), 'testC changed');
134+
} catch (err) {
135+
def.error(err);
136+
return;
137+
}
138+
139+
def.complete(undefined);
140+
});
141+
142+
root.bufferChangeEvents(() => {
143+
root.setContext('testA', 10);
144+
root.setContext('testB', 20);
145+
root.setContext('testC', 30);
146+
});
147+
148+
return def.p;
149+
});
76150
});

0 commit comments

Comments
 (0)