Skip to content

Commit 3d63750

Browse files
joaomorenojoaomoreno
committed
prevent recursive splitview mutations
fixes microsoft#35497
1 parent 7d5ae74 commit 3d63750

3 files changed

Lines changed: 67 additions & 3 deletions

File tree

‎src/vs/base/browser/ui/splitview/splitview.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,11 @@ interface ISashDragState {
5454
maxDelta: number;
5555
}
5656

57+
enum State {
58+
Idle,
59+
Busy
60+
}
61+
5762
export class SplitView implements IDisposable {
5863

5964
private orientation: Orientation;
@@ -63,6 +68,7 @@ export class SplitView implements IDisposable {
6368
private viewItems: IViewItem[] = [];
6469
private sashItems: ISashItem[] = [];
6570
private sashDragState: ISashDragState;
71+
private state: State = State.Idle;
6672

6773
get length(): number {
6874
return this.viewItems.length;
@@ -78,6 +84,12 @@ export class SplitView implements IDisposable {
7884
}
7985

8086
addView(view: IView, size: number, index = this.viewItems.length): void {
87+
if (this.state !== State.Idle) {
88+
throw new Error('Cant modify splitview');
89+
}
90+
91+
this.state = State.Busy;
92+
8193
// Add view
8294
const container = dom.$('.split-view-view');
8395

@@ -125,9 +137,16 @@ export class SplitView implements IDisposable {
125137

126138
view.render(container, this.orientation);
127139
this.relayout();
140+
this.state = State.Idle;
128141
}
129142

130143
removeView(index: number): void {
144+
if (this.state !== State.Idle) {
145+
throw new Error('Cant modify splitview');
146+
}
147+
148+
this.state = State.Busy;
149+
131150
if (index < 0 || index >= this.viewItems.length) {
132151
return;
133152
}
@@ -144,9 +163,16 @@ export class SplitView implements IDisposable {
144163
}
145164

146165
this.relayout();
166+
this.state = State.Idle;
147167
}
148168

149169
moveView(from: number, to: number): void {
170+
if (this.state !== State.Idle) {
171+
throw new Error('Cant modify splitview');
172+
}
173+
174+
this.state = State.Busy;
175+
150176
if (from < 0 || from >= this.viewItems.length) {
151177
return;
152178
}
@@ -169,6 +195,7 @@ export class SplitView implements IDisposable {
169195
}
170196

171197
this.layoutViews();
198+
this.state = State.Idle;
172199
}
173200

174201
private relayout(): void {
@@ -221,6 +248,12 @@ export class SplitView implements IDisposable {
221248
}
222249

223250
resizeView(index: number, size: number): void {
251+
if (this.state !== State.Idle) {
252+
throw new Error('Cant modify splitview');
253+
}
254+
255+
this.state = State.Busy;
256+
224257
if (index < 0 || index >= this.viewItems.length) {
225258
return;
226259
}
@@ -248,6 +281,8 @@ export class SplitView implements IDisposable {
248281

249282
this.resize(index - 1, deltaUp);
250283
}
284+
285+
this.state = State.Idle;
251286
}
252287

253288
getViewSize(index: number): number {

‎src/vs/base/test/browser/ui/splitview/splitview.test.ts‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,4 +311,33 @@ suite('Splitview', () => {
311311
view2.dispose();
312312
view1.dispose();
313313
});
314+
315+
test('issue #35497', () => {
316+
const view1 = new TestView(160, Number.POSITIVE_INFINITY);
317+
const view2 = new TestView(66, 66);
318+
319+
const splitview = new SplitView(container);
320+
splitview.layout(986);
321+
322+
splitview.addView(view1, 142, 0);
323+
assert.equal(view1.size, 986, 'first view is stretched');
324+
325+
view2.onDidRender(() => {
326+
assert.throws(() => splitview.resizeView(1, 922));
327+
assert.throws(() => splitview.resizeView(1, 922));
328+
});
329+
330+
splitview.addView(view2, 66, 0);
331+
assert.equal(view2.size, 66, 'second view is fixed');
332+
assert.equal(view1.size, 986 - 66, 'first view is collapsed');
333+
334+
const viewContainers = container.querySelectorAll('.split-view-view');
335+
assert.equal(viewContainers.length, 2, 'there are two view containers');
336+
assert.equal((viewContainers.item(0) as HTMLElement).style.height, '66px', 'second view container is 66px');
337+
assert.equal((viewContainers.item(1) as HTMLElement).style.height, `${986 - 66}px`, 'first view container is 66px');
338+
339+
splitview.dispose();
340+
view2.dispose();
341+
view1.dispose();
342+
});
314343
});

‎src/vs/workbench/parts/scm/electron-browser/scmViewlet.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -851,9 +851,10 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
851851

852852
if (shouldMainPanelBeVisible) {
853853
this.mainPanel = this.instantiationService.createInstance(MainPanel, this);
854-
const selectionChangeDisposable = this.mainPanel.onSelectionChange(this.onSelectionChange, this);
855854
this.addPanel(this.mainPanel, this.mainPanel.minimumSize, 0);
856855

856+
const selectionChangeDisposable = this.mainPanel.onSelectionChange(this.onSelectionChange, this);
857+
857858
this.mainPanelDisposable = toDisposable(() => {
858859
this.removePanel(this.mainPanel);
859860
selectionChangeDisposable.dispose();
@@ -959,7 +960,7 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
959960
}
960961

961962
protected isSingleView(): boolean {
962-
return super.isSingleView() && this.repositories.length === 1;
963+
return super.isSingleView() && this.repositoryPanels.length === 1;
963964
}
964965

965966
hide(repository: ISCMRepository): void {
@@ -970,7 +971,6 @@ export class SCMViewlet extends PanelViewlet implements IViewModel {
970971
this.mainPanel.hide(repository);
971972
}
972973

973-
974974
dispose(): void {
975975
this.disposables = dispose(this.disposables);
976976
this.mainPanelDisposable.dispose();

0 commit comments

Comments
 (0)