From f764c0ac917ba751d27003390436ba62402007a9 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Fri, 27 Sep 2024 12:58:34 -0700 Subject: [PATCH] Fix links sometimes not activating This was a very long standing bug that was hard to nail down a repro case. Turns out this was because at some point the current link can get cleared and re-evaluated, I think because of a different fix we made. That caused the object check to fail. So the fix there is to check link equality, not js object equality. Additionally I found we don't dispose of old linkifiers when open is called multiple times. See microsoft/vscode#230010 --- src/browser/CoreBrowserTerminal.ts | 7 ++++--- src/browser/Linkifier.ts | 12 +++++++++++- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/browser/CoreBrowserTerminal.ts b/src/browser/CoreBrowserTerminal.ts index f35f288299..2e15b5f379 100644 --- a/src/browser/CoreBrowserTerminal.ts +++ b/src/browser/CoreBrowserTerminal.ts @@ -69,7 +69,8 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal { private _helperContainer: HTMLElement | undefined; private _compositionView: HTMLElement | undefined; - public linkifier: ILinkifier2 | undefined; + private readonly _linkifier: MutableDisposable = this._register(new MutableDisposable()); + public get linkifier(): ILinkifier2 | undefined { return this._linkifier.value; } private _overviewRulerRenderer: OverviewRulerRenderer | undefined; private _viewport: Viewport | undefined; @@ -485,7 +486,7 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal { this._mouseService = this._instantiationService.createInstance(MouseService); this._instantiationService.setService(IMouseService, this._mouseService); - this.linkifier = this._register(this._instantiationService.createInstance(Linkifier, this.screenElement)); + const linkifier = this._linkifier.value = this._register(this._instantiationService.createInstance(Linkifier, this.screenElement)); // Performance: Add viewport and helper elements from the fragment this.element.appendChild(fragment); @@ -515,7 +516,7 @@ export class CoreBrowserTerminal extends CoreTerminal implements ITerminal { this._selectionService = this._register(this._instantiationService.createInstance(SelectionService, this.element, this.screenElement, - this.linkifier + linkifier )); this._instantiationService.setService(ISelectionService, this._selectionService); this._register(this._selectionService.onRequestScrollLines(e => this.scrollLines(e.amount, e.suppressScrollEvent))); diff --git a/src/browser/Linkifier.ts b/src/browser/Linkifier.ts index 5856d6df70..c5c31d62ff 100644 --- a/src/browser/Linkifier.ts +++ b/src/browser/Linkifier.ts @@ -227,7 +227,7 @@ export class Linkifier extends Disposable implements ILinkifier2 { return; } - if (this._mouseDownLink === this._currentLink && this._linkAtPosition(this._currentLink.link, position)) { + if (this._mouseDownLink && linkEquals(this._mouseDownLink.link, this._currentLink.link) && this._linkAtPosition(this._currentLink.link, position)) { this._currentLink.link.activate(event, this._currentLink.link.text); } } @@ -391,3 +391,13 @@ export class Linkifier extends Disposable implements ILinkifier2 { return { x1, y1, x2, y2, cols: this._bufferService.cols, fg }; } } + +function linkEquals(a: ILink, b: ILink): boolean { + return ( + a.text === b.text && + a.range.start.x === b.range.start.x && + a.range.start.y === b.range.start.y && + a.range.end.x === b.range.end.x && + a.range.end.y === b.range.end.y + ); +}