From a3beb7acb9b42b4dcbb6bd5e332fecd02a5d8868 Mon Sep 17 00:00:00 2001 From: Benjamin Pasero Date: Mon, 9 Jun 2025 16:43:01 +0200 Subject: [PATCH] debt - ensure to remove listeners --- src/vs/code/browser/workbench/workbench.ts | 5 ++-- .../browser/stickyScrollController.ts | 18 ++++---------- .../browser/accessibilitySignalService.ts | 24 +++++++++++++------ .../browser/extensionFeaturesTab.ts | 8 +++---- .../testing/browser/testingDecorations.ts | 4 ++-- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/src/vs/code/browser/workbench/workbench.ts b/src/vs/code/browser/workbench/workbench.ts index 3928342a0492..f9ee35ab3705 100644 --- a/src/vs/code/browser/workbench/workbench.ts +++ b/src/vs/code/browser/workbench/workbench.ts @@ -4,6 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { isStandalone } from '../../../base/browser/browser.js'; +import { addDisposableListener } from '../../../base/browser/dom.js'; import { mainWindow } from '../../../base/browser/window.js'; import { VSBuffer, decodeBase64, encodeBase64 } from '../../../base/common/buffer.js'; import { Emitter } from '../../../base/common/event.js'; @@ -340,9 +341,7 @@ class LocalStorageURLCallbackProvider extends Disposable implements IURLCallback return; } - const fn = () => this.onDidChangeLocalStorage(); - mainWindow.addEventListener('storage', fn); - this.onDidChangeLocalStorageDisposable = { dispose: () => mainWindow.removeEventListener('storage', fn) }; + this.onDidChangeLocalStorageDisposable = addDisposableListener(mainWindow, 'storage', () => this.onDidChangeLocalStorage()); } private stopListening(): void { diff --git a/src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts b/src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts index 9f443e037d69..736492e7292e 100644 --- a/src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts +++ b/src/vs/editor/contrib/stickyScroll/browser/stickyScrollController.ts @@ -310,26 +310,18 @@ export class StickyScrollController extends Disposable implements IEditorContrib } this._revealPosition(position); })); - const mouseMoveListener = (mouseEvent: MouseEvent) => { + this._register(dom.addDisposableListener(mainWindow, dom.EventType.MOUSE_MOVE, mouseEvent => { this._mouseTarget = mouseEvent.target; this._onMouseMoveOrKeyDown(mouseEvent); - }; - const keyDownListener = (mouseEvent: KeyboardEvent) => { + })); + this._register(dom.addDisposableListener(mainWindow, dom.EventType.KEY_DOWN, mouseEvent => { this._onMouseMoveOrKeyDown(mouseEvent); - }; - const keyUpListener = (e: KeyboardEvent) => { + })); + this._register(dom.addDisposableListener(mainWindow, dom.EventType.KEY_UP, () => { if (this._showEndForLine !== undefined) { this._showEndForLine = undefined; this._renderStickyScroll(); } - }; - mainWindow.addEventListener(dom.EventType.MOUSE_MOVE, mouseMoveListener); - mainWindow.addEventListener(dom.EventType.KEY_DOWN, keyDownListener); - mainWindow.addEventListener(dom.EventType.KEY_UP, keyUpListener); - this._register(toDisposable(() => { - mainWindow.removeEventListener(dom.EventType.MOUSE_MOVE, mouseMoveListener); - mainWindow.removeEventListener(dom.EventType.KEY_DOWN, keyDownListener); - mainWindow.removeEventListener(dom.EventType.KEY_UP, keyUpListener); })); this._register(gesture.onMouseMoveOrRelevantKeyDown(([mouseEvent, _keyboardEvent]) => { diff --git a/src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts b/src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts index e4382f461f86..8bc11939b10c 100644 --- a/src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts +++ b/src/vs/platform/accessibilitySignal/browser/accessibilitySignalService.ts @@ -3,10 +3,11 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ +import { addDisposableListener } from '../../../base/browser/dom.js'; import { CachedFunction } from '../../../base/common/cache.js'; import { getStructuralKey } from '../../../base/common/equals.js'; import { Event, IValueWithChangeEvent } from '../../../base/common/event.js'; -import { Disposable, IDisposable, toDisposable } from '../../../base/common/lifecycle.js'; +import { Disposable, DisposableStore, IDisposable, toDisposable } from '../../../base/common/lifecycle.js'; import { FileAccess } from '../../../base/common/network.js'; import { derived, observableFromEvent, ValueWithChangeEventFromObservable } from '../../../base/common/observable.js'; import { localize } from '../../../nls.js'; @@ -277,17 +278,26 @@ function checkEnabledState(state: EnabledState, getScreenReaderAttached: () => b * Play the given audio url. * @volume value between 0 and 1 */ -function playAudio(url: string, volume: number): Promise { - return new Promise((resolve, reject) => { +async function playAudio(url: string, volume: number): Promise { + const disposables = new DisposableStore(); + try { + return await doPlayAudio(url, volume, disposables); + } finally { + disposables.dispose(); + } +} + +function doPlayAudio(url: string, volume: number, disposables: DisposableStore): Promise { + return new Promise((resolve, reject) => { const audio = new Audio(url); audio.volume = volume; - audio.addEventListener('ended', () => { + disposables.add(addDisposableListener(audio, 'ended', () => { resolve(audio); - }); - audio.addEventListener('error', (e) => { + })); + disposables.add(addDisposableListener(audio, 'error', (e) => { // When the error event fires, ended might not be called reject(e.error); - }); + })); audio.play().catch(e => { // When play fails, the error event is not fired. reject(e); diff --git a/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts b/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts index bcef6a198c03..0b7daf56e120 100644 --- a/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts +++ b/src/vs/workbench/contrib/extensions/browser/extensionFeaturesTab.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import { Disposable, DisposableStore, IDisposable, MutableDisposable, toDisposable } from '../../../../base/common/lifecycle.js'; -import { $, append, clearNode } from '../../../../base/browser/dom.js'; +import { $, append, clearNode, addDisposableListener, EventType } from '../../../../base/browser/dom.js'; import { Emitter, Event } from '../../../../base/common/event.js'; import { ExtensionIdentifier, IExtensionManifest } from '../../../../platform/extensions/common/extensions.js'; import { Orientation, Sizing, SplitView } from '../../../../base/browser/ui/splitview/splitview.js'; @@ -304,15 +304,13 @@ class RuntimeStatusMarkdownRenderer extends Disposable implements IExtensionFeat hoverDisposable.value = undefined; } }; - svg.addEventListener('mousemove', mouseMoveListener); - disposables.add(toDisposable(() => svg.removeEventListener('mousemove', mouseMoveListener))); + disposables.add(addDisposableListener(svg, EventType.MOUSE_MOVE, mouseMoveListener)); const mouseLeaveListener = () => { highlightCircle.style.display = 'none'; hoverDisposable.value = undefined; }; - svg.addEventListener('mouseleave', mouseLeaveListener); - disposables.add(toDisposable(() => svg.removeEventListener('mouseleave', mouseLeaveListener))); + disposables.add(addDisposableListener(svg, EventType.MOUSE_LEAVE, mouseLeaveListener)); } } diff --git a/src/vs/workbench/contrib/testing/browser/testingDecorations.ts b/src/vs/workbench/contrib/testing/browser/testingDecorations.ts index ac47408247d1..cac9d9f09d04 100644 --- a/src/vs/workbench/contrib/testing/browser/testingDecorations.ts +++ b/src/vs/workbench/contrib/testing/browser/testingDecorations.ts @@ -1387,10 +1387,10 @@ class TestErrorContentWidget extends Disposable implements IContentWidget { text = lf === -1 ? msg : msg.slice(0, lf); } - this.node.root.addEventListener('click', e => { + this._register(dom.addDisposableListener(this.node.root, dom.EventType.CLICK, e => { this.peekOpener.peekUri(uri); e.preventDefault(); - }); + })); const ctrl = TestingOutputPeekController.get(editor); if (ctrl) {