Deflake registerLinkProvider tests

The root cause of this flakiness problem was related to the click event
handler. For some reason it would not go off on some links but it would
when clicking elsewhere in the terminal. I tried a bunch of things to
keep the click handler to no avail, instead opting to move to using
mousedown and mouseup events for activating links which comes with the
nice benefit of ensuring that the link on mouseup is the same one on
mousedown.

Fixes #3821
This commit is contained in:
Daniel Imms
2022-05-19 15:30:12 -07:00
parent 5353808437
commit 8f97a5a786
2 changed files with 30 additions and 15 deletions
+11 -6
View File
@@ -18,6 +18,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
private _linkProviders: ILinkProvider[] = [];
public get currentLink(): ILinkWithState | undefined { return this._currentLink; }
protected _currentLink: ILinkWithState | undefined;
private _mouseDownLink: ILinkWithState | undefined;
private _lastMouseEvent: MouseEvent | undefined;
private _linkCacheDisposables: IDisposable[] = [];
private _lastBufferCell: IBufferCellPosition | undefined;
@@ -61,7 +62,8 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
this._clearCurrentLink();
}));
this.register(addDisposableDomListener(this._element, 'mousemove', this._onMouseMove.bind(this)));
this.register(addDisposableDomListener(this._element, 'click', this._onClick.bind(this)));
this.register(addDisposableDomListener(this._element, 'mousedown', this._handleMouseDown.bind(this)));
this.register(addDisposableDomListener(this._element, 'mouseup', this._handleMouseUp.bind(this)));
}
private _onMouseMove(event: MouseEvent): void {
@@ -129,7 +131,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
let linkProvided = false;
// There is no link cached, so ask for one
this._linkProviders.forEach((linkProvider, i) => {
for (const [i, linkProvider] of this._linkProviders.entries()) {
if (useLineCache) {
const existingReply = this._activeProviderReplies?.get(i);
// If there isn't a reply, the provider hasn't responded yet.
@@ -156,7 +158,7 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
}
});
}
});
}
}
private _removeIntersectingLinks(y: number, replies: Map<Number, ILinkWithState[] | undefined>): void {
@@ -222,18 +224,21 @@ export class Linkifier2 extends Disposable implements ILinkifier2 {
return linkProvided;
}
private _onClick(event: MouseEvent): void {
private _handleMouseDown(): void {
this._mouseDownLink = this._currentLink;
}
private _handleMouseUp(event: MouseEvent): void {
if (!this._element || !this._mouseService || !this._currentLink) {
return;
}
const position = this._positionFromMouseEvent(event, this._element, this._mouseService);
if (!position) {
return;
}
if (this._linkAtPosition(this._currentLink.link, position)) {
if (this._mouseDownLink === this._currentLink && this._linkAtPosition(this._currentLink.link, position)) {
this._currentLink.link.activate(event, this._currentLink.link.text);
}
}
+19 -9
View File
@@ -792,6 +792,9 @@ describe('API Integration Tests', function(): void {
describe('registerLinkProvider', () => {
it('should fire provideLinks when hovering cells', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
await page.evaluate(`
window.calls = [];
window.disposable = window.term.registerLinkProvider({
@@ -811,6 +814,9 @@ describe('API Integration Tests', function(): void {
it('should fire hover and leave events on the link', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
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 ');
@@ -846,6 +852,9 @@ describe('API Integration Tests', function(): void {
it('should work fine when hover and leave callbacks are not provided', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
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 ');
@@ -886,18 +895,12 @@ describe('API Integration Tests', function(): void {
it('should fire activate events when clicking the link', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
await writeSync(page, 'a b c');
// Wait for renderer to catch up as links are cleared on render
await pollFor(page, `document.querySelector('.xterm-rows').textContent`, 'a b c ');
// Focus terminal to avoid a render event clearing the active link
const dims = await getDimensions();
await moveMouseCell(page, dims, 5, 5);
await page.mouse.down();
await page.mouse.up();
await timeout(200); // Not sure how to avoid this timeout, checking for xterm-focus doesn't help
await page.evaluate(`
window.calls = [];
window.disposable = window.term.registerLinkProvider({
@@ -913,6 +916,7 @@ describe('API Integration Tests', function(): void {
}
});
`);
const dims = await getDimensions();
await moveMouseCell(page, dims, 3, 1);
await pollFor(page, `window.calls`, ['provide 1', 'hover 1']);
await page.mouse.down();
@@ -933,6 +937,9 @@ describe('API Integration Tests', function(): void {
it('should work when multiple links are provided on the same line', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
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 ');
@@ -979,6 +986,9 @@ describe('API Integration Tests', function(): void {
it('should dispose links when hovering away', async () => {
await openTerminal(page, { rendererType: 'dom' });
// Focus the terminal as the cursor will show and trigger a rerender, which can clear the
// active link
await page.evaluate('window.term.focus()');
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 ');