Skip to content

Commit

Permalink
Fix links sometimes not activating
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Tyriar committed Sep 27, 2024
1 parent cb3bd46 commit f764c0a
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/browser/CoreBrowserTerminal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<ILinkifier2> = this._register(new MutableDisposable());
public get linkifier(): ILinkifier2 | undefined { return this._linkifier.value; }
private _overviewRulerRenderer: OverviewRulerRenderer | undefined;
private _viewport: Viewport | undefined;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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)));
Expand Down
12 changes: 11 additions & 1 deletion src/browser/Linkifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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
);
}

0 comments on commit f764c0a

Please sign in to comment.