Skip to content

Commit b91c21b

Browse files
committed
protect list and webview from revert.
1 parent 5d9e565 commit b91c21b

7 files changed

Lines changed: 66 additions & 44 deletions

File tree

src/vs/vscode.proposed.d.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1693,7 +1693,7 @@ declare module 'vscode' {
16931693
}): Promise<void>;
16941694
saveNotebook(document: NotebookDocument, cancellation: CancellationToken): Promise<void>;
16951695
saveNotebookAs(targetResource: Uri, document: NotebookDocument, cancellation: CancellationToken): Promise<void>;
1696-
readonly onDidChangeNotebook: Event<NotebookDocumentContentChangeEvent>;
1696+
readonly onDidChangeNotebook: Event<NotebookDocumentContentChangeEvent | NotebookDocumentEditEvent>;
16971697
backupNotebook(document: NotebookDocument, context: NotebookDocumentBackupContext, cancellation: CancellationToken): Promise<NotebookDocumentBackup>;
16981698

16991699
kernel?: NotebookKernel;

src/vs/workbench/contrib/notebook/browser/notebookBrowser.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,7 @@ export interface INotebookEditor extends IEditor {
356356
}
357357

358358
export interface INotebookCellList {
359+
isDisposed: boolean
359360
readonly contextKeyService: IContextKeyService;
360361
elementAt(position: number): ICellViewModel | undefined;
361362
elementHeight(element: ICellViewModel): number;

src/vs/workbench/contrib/notebook/browser/notebookEditor.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,10 @@ export class NotebookEditor extends BaseEditor {
188188

189189
private _saveEditorViewState(input: IEditorInput | undefined): void {
190190
if (this.group && this._widget.value && input instanceof NotebookEditorInput) {
191+
if (this._widget.value.isDisposed) {
192+
return;
193+
}
194+
191195
const state = this._widget.value.getEditorViewState();
192196
this._editorMemento.saveEditorState(this.group, input.resource, state);
193197
}

src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -685,14 +685,13 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditor
685685
const focus = this._list.getFocus()[0];
686686
if (typeof focus === 'number') {
687687
const element = this._notebookViewModel!.viewCells[focus];
688-
const itemDOM = this._list?.domElementOfElement(element!);
689-
let editorFocused = false;
690-
if (document.activeElement && itemDOM && itemDOM.contains(document.activeElement)) {
691-
editorFocused = true;
692-
}
688+
if (element) {
689+
const itemDOM = this._list?.domElementOfElement(element);
690+
let editorFocused = !!(document.activeElement && itemDOM && itemDOM.contains(document.activeElement));
693691

694-
state.editorFocused = editorFocused;
695-
state.focus = focus;
692+
state.editorFocused = editorFocused;
693+
state.focus = focus;
694+
}
696695
}
697696
}
698697

src/vs/workbench/contrib/notebook/browser/notebookEditorWidgetService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ class NotebookEditorWidgetService implements INotebookEditorWidgetService {
9292

9393
private _disposeWidget(widget: NotebookEditorWidget): void {
9494
widget.onWillHide();
95-
widget.getDomNode().remove();
95+
const domNode = widget.getDomNode();
9696
widget.dispose();
97+
domNode.remove();
9798
}
9899

99100
private _freeWidget(input: NotebookEditorInput, source: IEditorGroup, target: IEditorGroup): void {

src/vs/workbench/contrib/notebook/browser/view/notebookCellList.ts

Lines changed: 48 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,12 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
4545
private _hiddenRangeIds: string[] = [];
4646
private hiddenRangesPrefixSum: PrefixSumComputer | null = null;
4747

48+
private _isDisposed = false;
49+
50+
get isDisposed() {
51+
return this._isDisposed;
52+
}
53+
4854
constructor(
4955
private listUser: string,
5056
container: HTMLElement,
@@ -162,43 +168,28 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
162168
attachViewModel(model: NotebookViewModel) {
163169
this._viewModel = model;
164170
this._viewModelStore.add(model.onDidChangeViewCells((e) => {
165-
const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[];
166-
const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges);
167-
168-
const oldVisibleViewCells: CellViewModel[] = [];
169-
const oldViewCellMapping = new Set<string>();
170-
for (let i = 0; i < this.length; i++) {
171-
oldVisibleViewCells.push(this.element(i));
172-
oldViewCellMapping.add(this.element(i).uri.toString());
173-
}
174-
175-
const viewDiffs = diff<CellViewModel>(oldVisibleViewCells, newVisibleViewCells, a => {
176-
return oldViewCellMapping.has(a.uri.toString());
177-
});
171+
DOM.scheduleAtNextAnimationFrame(() => {
172+
if (this._isDisposed) {
173+
return;
174+
}
178175

179-
if (e.synchronous) {
180-
viewDiffs.reverse().forEach((diff) => {
181-
// remove output in the webview
182-
const hideOutputs: IProcessedOutput[] = [];
183-
const deletedOutputs: IProcessedOutput[] = [];
184-
185-
for (let i = diff.start; i < diff.start + diff.deleteCount; i++) {
186-
const cell = this.element(i);
187-
if (this._viewModel!.hasCell(cell.handle)) {
188-
hideOutputs.push(...cell?.model.outputs);
189-
} else {
190-
deletedOutputs.push(...cell?.model.outputs);
191-
}
192-
}
176+
const currentRanges = this._hiddenRangeIds.map(id => this._viewModel!.getTrackedRange(id)).filter(range => range !== null) as ICellRange[];
177+
const newVisibleViewCells: CellViewModel[] = getVisibleCells(this._viewModel!.viewCells as CellViewModel[], currentRanges);
193178

194-
this.splice2(diff.start, diff.deleteCount, diff.toInsert);
179+
const oldVisibleViewCells: CellViewModel[] = [];
180+
const oldViewCellMapping = new Set<string>();
181+
for (let i = 0; i < this.length; i++) {
182+
oldVisibleViewCells.push(this.element(i));
183+
oldViewCellMapping.add(this.element(i).uri.toString());
184+
}
195185

196-
hideOutputs.forEach(output => this._onDidHideOutput.fire(output));
197-
deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output));
186+
const viewDiffs = diff<CellViewModel>(oldVisibleViewCells, newVisibleViewCells, a => {
187+
return oldViewCellMapping.has(a.uri.toString());
198188
});
199-
} else {
200-
DOM.scheduleAtNextAnimationFrame(() => {
189+
190+
if (e.synchronous) {
201191
viewDiffs.reverse().forEach((diff) => {
192+
// remove output in the webview
202193
const hideOutputs: IProcessedOutput[] = [];
203194
const deletedOutputs: IProcessedOutput[] = [];
204195

@@ -216,8 +207,29 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
216207
hideOutputs.forEach(output => this._onDidHideOutput.fire(output));
217208
deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output));
218209
});
219-
});
220-
}
210+
} else {
211+
DOM.scheduleAtNextAnimationFrame(() => {
212+
viewDiffs.reverse().forEach((diff) => {
213+
const hideOutputs: IProcessedOutput[] = [];
214+
const deletedOutputs: IProcessedOutput[] = [];
215+
216+
for (let i = diff.start; i < diff.start + diff.deleteCount; i++) {
217+
const cell = this.element(i);
218+
if (this._viewModel!.hasCell(cell.handle)) {
219+
hideOutputs.push(...cell?.model.outputs);
220+
} else {
221+
deletedOutputs.push(...cell?.model.outputs);
222+
}
223+
}
224+
225+
this.splice2(diff.start, diff.deleteCount, diff.toInsert);
226+
227+
hideOutputs.forEach(output => this._onDidHideOutput.fire(output));
228+
deletedOutputs.forEach(output => this._onDidRemoveOutput.fire(output));
229+
});
230+
});
231+
}
232+
});
221233
}));
222234

223235
this._viewModelStore.add(model.onDidChangeSelection(() => {
@@ -490,7 +502,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
490502

491503
domElementOfElement(element: ICellViewModel): HTMLElement | null {
492504
const index = this._getViewIndexUpperBound(element);
493-
if (index !== undefined) {
505+
if (index !== undefined && index >= 0) {
494506
return this.view.domElement(index);
495507
}
496508

@@ -874,6 +886,7 @@ export class NotebookCellList extends WorkbenchList<CellViewModel> implements ID
874886
}
875887

876888
dispose() {
889+
this._isDisposed = true;
877890
this._viewModelStore.dispose();
878891
this._localDisposableStore.dispose();
879892
super.dispose();

src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,10 @@ ${loaderJs}
318318
}
319319

320320
async initialize(content: string) {
321+
if (!document.body.contains(this.element)) {
322+
throw new Error('Element is already detached from the DOM tree');
323+
}
324+
321325
this.webview = this._createInset(this.webviewService, content);
322326
this.webview.mountTo(this.element);
323327
this._register(this.webview);

0 commit comments

Comments
 (0)