From 50fc84f7d79beb19367d0bd470ef2059fdfbc94c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 4 Sep 2020 21:57:51 -0700 Subject: [PATCH 1/2] Support ILink.dispose When displaying hovers/overlays using the link provider API currently it's not possible to know when unused links are no longer cached in xterm.js which can result in leaked event listeners. --- src/browser/Linkifier2.ts | 7 ++++++ src/browser/Types.d.ts | 1 + test/api/Terminal.api.ts | 49 +++++++++++++++++++++++++++++++++++++++ typings/xterm.d.ts | 5 ++++ 4 files changed, 62 insertions(+) diff --git a/src/browser/Linkifier2.ts b/src/browser/Linkifier2.ts index 14ee5f6615..73ea02689f 100644 --- a/src/browser/Linkifier2.ts +++ b/src/browser/Linkifier2.ts @@ -125,6 +125,13 @@ export class Linkifier2 extends Disposable implements ILinkifier2 { private _askForLink(position: IBufferCellPosition, useLineCache: boolean): void { if (!this._activeProviderReplies || !useLineCache) { + this._activeProviderReplies?.forEach(reply => { + reply?.forEach(linkWithState => { + if (linkWithState.link.dispose) { + linkWithState.link.dispose(); + } + }); + }); this._activeProviderReplies = new Map(); this._activeLine = position.y; } diff --git a/src/browser/Types.d.ts b/src/browser/Types.d.ts index 2227e39f2b..e8e0cabd1c 100644 --- a/src/browser/Types.d.ts +++ b/src/browser/Types.d.ts @@ -275,6 +275,7 @@ interface ILink { activate(event: MouseEvent, text: string): void; hover?(event: MouseEvent, text: string): void; leave?(event: MouseEvent, text: string): void; + dispose?(): void; } interface ILinkDecorations { diff --git a/test/api/Terminal.api.ts b/test/api/Terminal.api.ts index 85fc889748..f25711cf4a 100644 --- a/test/api/Terminal.api.ts +++ b/test/api/Terminal.api.ts @@ -755,6 +755,55 @@ describe('API Integration Tests', function(): void { await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'provide 2', 'provide 1', 'hover 9-11']); await page.evaluate(`window.disposable.dispose()`); }); + + it('should dispose links when hovering away', async () => { + await openTerminal(page, { rendererType: 'dom' }); + await writeSync(page, 'foo bar baz'); + // Wait for renderer to catch up as links are cleared on render + await pollFor(page, `document.querySelector('.xterm-rows').textContent`, 'foo bar baz '); + await page.evaluate(` + window.calls = []; + window.disposable = window.term.registerLinkProvider({ + provideLinks: (position, cb) => { + window.calls.push('provide ' + position); + if (position === 1) { + cb([{ + range: { start: { x: 1, y: 1 }, end: { x: 3, y: 1 } }, + text: '', + activate: () => window.calls.push('activate'), + dispose: () => window.calls.push('dispose 1-3'), + hover: () => window.calls.push('hover 1-3'), + leave: () => window.calls.push('leave 1-3') + }, { + range: { start: { x: 5, y: 1 }, end: { x: 7, y: 1 } }, + text: '', + activate: () => window.calls.push('activate'), + dispose: () => window.calls.push('dispose 5-7'), + hover: () => window.calls.push('hover 5-7'), + leave: () => window.calls.push('leave 5-7') + }, { + range: { start: { x: 9, y: 1 }, end: { x: 11, y: 1 } }, + text: '', + activate: () => window.calls.push('activate'), + dispose: () => window.calls.push('dispose 9-11'), + hover: () => window.calls.push('hover 9-11'), + leave: () => window.calls.push('leave 9-11') + }]); + } + } + }); + `); + const dims = await getDimensions(); + await moveMouseCell(page, dims, 2, 1); + await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3']); + await moveMouseCell(page, dims, 6, 1); + await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7']); + await moveMouseCell(page, dims, 6, 2); + await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2']); + await moveMouseCell(page, dims, 10, 1); + await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2', 'provide 1', 'hover 9-11']); + await page.evaluate(`window.disposable.dispose()`); + }); }); }); diff --git a/typings/xterm.d.ts b/typings/xterm.d.ts index dd934e358f..f98c629d76 100644 --- a/typings/xterm.d.ts +++ b/typings/xterm.d.ts @@ -1158,6 +1158,11 @@ declare module 'xterm' { * @param text The text of the link. */ leave?(event: MouseEvent, text: string): void; + + /** + * Called when the link is released and no longer used by xterm.js. + */ + dispose?(): void; } /** From 9f3746a3a1c9f6d9f5e44db3ffe3585f71cb06c7 Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Fri, 4 Sep 2020 22:04:59 -0700 Subject: [PATCH 2/2] Improve test case --- test/api/Terminal.api.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/api/Terminal.api.ts b/test/api/Terminal.api.ts index f25711cf4a..59d123aadd 100644 --- a/test/api/Terminal.api.ts +++ b/test/api/Terminal.api.ts @@ -802,6 +802,8 @@ describe('API Integration Tests', function(): void { await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2']); await moveMouseCell(page, dims, 10, 1); await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2', 'provide 1', 'hover 9-11']); + await moveMouseCell(page, dims, 10, 2); + await pollFor(page, `window.calls`, ['provide 1', 'hover 1-3', 'leave 1-3', 'hover 5-7', 'leave 5-7', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2', 'provide 1', 'hover 9-11', 'leave 9-11', 'dispose 1-3', 'dispose 5-7', 'dispose 9-11', 'provide 2']); await page.evaluate(`window.disposable.dispose()`); }); });