Skip to content

Commit f28c21e

Browse files
committed
Fix link event listener leak
The bug here is that _clearCurrentLink was being called only on the line(s) that changed, but _askForLink was then being triggered regardless. This caused more onRenderedViewportChange listeners to be registered and for the thread blocking to get exponentially worse. Fixes xtermjs#4341
1 parent 3142460 commit f28c21e

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

src/browser/Linkifier2.ts

+16-8
Original file line numberDiff line numberDiff line change
@@ -309,19 +309,27 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
309309
}
310310
});
311311

312-
// Add listener for rerendering
312+
// Listen to viewport changes re-render the link under the cursor when the viewport line
313+
// changes
313314
if (this._renderService) {
314315
this._linkCacheDisposables.push(this._renderService.onRenderedViewportChange(e => {
316+
// Sanity check, this shouldn't happen in practice as this listener would be disposed
317+
if (!this._currentLink) {
318+
return;
319+
}
315320
// When start is 0 a scroll most likely occurred, make sure links above the fold also get
316321
// cleared.
317322
const start = e.start === 0 ? 0 : e.start + 1 + this._bufferService.buffer.ydisp;
318-
const oldEvent = this._currentLink ? this._lastMouseEvent : undefined;
319-
this._clearCurrentLink(start, e.end + 1 + this._bufferService.buffer.ydisp);
320-
if (oldEvent && this._element) {
321-
// re-eval previously active link after changes
322-
const position = this._positionFromMouseEvent(oldEvent, this._element, this._mouseService!);
323-
if (position) {
324-
this._askForLink(position, false);
323+
const end = this._bufferService.buffer.ydisp + 1 + e.end;
324+
// Only clear the link if the viewport change happened on this line
325+
if (this._currentLink.link.range.start.y >= start && this._currentLink.link.range.end.y <= end) {
326+
this._clearCurrentLink(start, end);
327+
if (this._lastMouseEvent && this._element) {
328+
// re-eval previously active link after changes
329+
const position = this._positionFromMouseEvent(this._lastMouseEvent, this._element, this._mouseService!);
330+
if (position) {
331+
this._askForLink(position, false);
332+
}
325333
}
326334
}
327335
}));

0 commit comments

Comments
 (0)