From 613aa6ba23fd486511ca531aa897566f31536d15 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 8 Jan 2026 16:51:45 -0800 Subject: [PATCH 1/5] Fire onSelectionChange when starting to drag --- src/browser/services/SelectionService.test.ts | 14 ++++++ src/browser/services/SelectionService.ts | 8 ++++ test/playwright/Terminal.test.ts | 47 +++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/src/browser/services/SelectionService.test.ts b/src/browser/services/SelectionService.test.ts index 67158def..9213e8f8 100644 --- a/src/browser/services/SelectionService.test.ts +++ b/src/browser/services/SelectionService.test.ts @@ -496,5 +496,19 @@ describe('SelectionService', () => { assert.isFalse(selectionService.areCoordsInSelection([2, 1], [2, 0], [2, 1])); }); }); + + describe('onSelectionChange', () => { + it('should not fire when setting model properties directly', () => { + buffer.lines.set(0, stringToRow('foo bar')); + + let eventFired = false; + selectionService.onSelectionChange(() => { eventFired = true; }); + + selectionService.model.selectionStart = [0, 0]; + selectionService.model.selectionEnd = undefined; + + assert.isFalse(eventFired); + }); + }); }); diff --git a/src/browser/services/SelectionService.ts b/src/browser/services/SelectionService.ts index 39d666de..6abd5fb9 100644 --- a/src/browser/services/SelectionService.ts +++ b/src/browser/services/SelectionService.ts @@ -532,6 +532,9 @@ export class SelectionService extends Disposable implements ISelectionService { * @param event The mouse event. */ private _handleSingleClick(event: MouseEvent): void { + // Track if there was a selection before clearing + const hadSelection = this.hasSelection; + this._model.selectionStartLength = 0; this._model.isSelectAllActive = false; this._activeSelectionMode = this.shouldColumnSelect(event) ? SelectionMode.COLUMN : SelectionMode.NORMAL; @@ -543,6 +546,11 @@ export class SelectionService extends Disposable implements ISelectionService { } this._model.selectionEnd = undefined; + // Fire selection change event if a selection was cleared + if (hadSelection) { + this._fireOnSelectionChange(this._model.finalSelectionStart, this._model.finalSelectionEnd, false); + } + // Ensure the line exists const line = this._bufferService.buffer.lines.get(this._model.selectionStart[1]); if (!line) { diff --git a/test/playwright/Terminal.test.ts b/test/playwright/Terminal.test.ts index f03d687d..5deea6d1 100644 --- a/test/playwright/Terminal.test.ts +++ b/test/playwright/Terminal.test.ts @@ -374,6 +374,53 @@ test.describe('API Integration Tests', () => { await pollFor(ctx.page, `window.callCount`, 2); }); + test('onSelectionChange should fire on mousedown when clearing selection', async () => { + await openTerminal(ctx); + await ctx.proxy.write('foo bar baz'); + await ctx.page.evaluate(` + window.callCount = 0; + window.term.onSelectionChange(() => window.callCount++); + `); + await ctx.page.evaluate(`window.term.selectAll()`); + await pollFor(ctx.page, `window.callCount`, 1); + + const termCoords: any = await ctx.page.evaluate(` + (function() { + const rect = window.term.element.getBoundingClientRect(); + const dim = window.term.dimensions; + return { left: rect.left, top: rect.top, cellWidth: dim.css.cell.width, cellHeight: dim.css.cell.height }; + })(); + `); + const x = termCoords.left + termCoords.cellWidth * 5; + const y = termCoords.top + termCoords.cellHeight * 0.5; + await ctx.page.mouse.click(x, y); + + await pollFor(ctx.page, `window.callCount`, 2); + }); + + test('onSelectionChange should not fire on mousedown when no prior selection', async () => { + await openTerminal(ctx); + await ctx.proxy.write('foo bar baz'); + await ctx.page.evaluate(` + window.callCount = 0; + window.term.onSelectionChange(() => window.callCount++); + `); + await pollFor(ctx.page, `window.callCount`, 0); + + const termCoords: any = await ctx.page.evaluate(` + (function() { + const rect = window.term.element.getBoundingClientRect(); + const dim = window.term.dimensions; + return { left: rect.left, top: rect.top, cellWidth: dim.css.cell.width, cellHeight: dim.css.cell.height }; + })(); + `); + const x = termCoords.left + termCoords.cellWidth * 5; + const y = termCoords.top + termCoords.cellHeight * 0.5; + await ctx.page.mouse.click(x, y); + + await pollFor(ctx.page, `window.callCount`, 0); + }); + test('onRender', async () => { await openTerminal(ctx); await timeout(20); // Ensure all init events are fired From 6efb319ece71b9c96649b9d2c32f60085ea50bd6 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 8 Jan 2026 17:01:56 -0800 Subject: [PATCH 2/5] Add tests more thorough tests for onSelectionChange --- test/playwright/Terminal.test.ts | 111 ++++++++++++++++--------------- 1 file changed, 59 insertions(+), 52 deletions(-) diff --git a/test/playwright/Terminal.test.ts b/test/playwright/Terminal.test.ts index dee14aa4..051a7ace 100644 --- a/test/playwright/Terminal.test.ts +++ b/test/playwright/Terminal.test.ts @@ -361,64 +361,71 @@ test.describe('API Integration Tests', () => { await pollFor(ctx.page, `window.calls`, [1, 2]); }); - test('onSelectionChange', async () => { - await openTerminal(ctx); - await ctx.page.evaluate(` - window.callCount = 0; - window.term.onSelectionChange(() => window.callCount++); - `); - await pollFor(ctx.page, `window.callCount`, 0); - await ctx.page.evaluate(`window.term.selectAll()`); - await pollFor(ctx.page, `window.callCount`, 1); - await ctx.page.evaluate(`window.term.clearSelection()`); - await pollFor(ctx.page, `window.callCount`, 2); - }); + test.describe('onSelectionChange', () => { + let callCount: number; - test('onSelectionChange should fire on mousedown when clearing selection', async () => { - await openTerminal(ctx); - await ctx.proxy.write('foo bar baz'); - await ctx.page.evaluate(` - window.callCount = 0; - window.term.onSelectionChange(() => window.callCount++); - `); - await ctx.page.evaluate(`window.term.selectAll()`); - await pollFor(ctx.page, `window.callCount`, 1); + test.beforeEach(async () => { + await openTerminal(ctx); + callCount = 0; + ctx.proxy.onSelectionChange(() => callCount++); + }); - const termCoords: any = await ctx.page.evaluate(` - (function() { - const rect = window.term.element.getBoundingClientRect(); - const dim = window.term.dimensions; - return { left: rect.left, top: rect.top, cellWidth: dim.css.cell.width, cellHeight: dim.css.cell.height }; - })(); - `); - const x = termCoords.left + termCoords.cellWidth * 5; - const y = termCoords.top + termCoords.cellHeight * 0.5; - await ctx.page.mouse.click(x, y); + test('should fire for programmatic selection changes', async () => { + strictEqual(callCount, 0); + await ctx.proxy.selectAll(); + strictEqual(callCount, 1); + await ctx.proxy.clearSelection(); + strictEqual(callCount, 2); + }); - await pollFor(ctx.page, `window.callCount`, 2); - }); + test('should fire on mousedown when clearing selection', async () => { + await ctx.proxy.write('foo bar baz'); + await ctx.proxy.selectAll(); + strictEqual(callCount, 1); - test('onSelectionChange should not fire on mousedown when no prior selection', async () => { - await openTerminal(ctx); - await ctx.proxy.write('foo bar baz'); - await ctx.page.evaluate(` - window.callCount = 0; - window.term.onSelectionChange(() => window.callCount++); - `); - await pollFor(ctx.page, `window.callCount`, 0); + const dims = (await ctx.proxy.dimensions)!; + const termRect: any = await ctx.page.evaluate(`window.term.element.getBoundingClientRect()`); + const x = termRect.left + dims.css.cell.width * 5; + const y = termRect.top + dims.css.cell.height * 0.5; + await ctx.page.mouse.click(x, y); - const termCoords: any = await ctx.page.evaluate(` - (function() { - const rect = window.term.element.getBoundingClientRect(); - const dim = window.term.dimensions; - return { left: rect.left, top: rect.top, cellWidth: dim.css.cell.width, cellHeight: dim.css.cell.height }; - })(); - `); - const x = termCoords.left + termCoords.cellWidth * 5; - const y = termCoords.top + termCoords.cellHeight * 0.5; - await ctx.page.mouse.click(x, y); + strictEqual(callCount, 2); + }); - await pollFor(ctx.page, `window.callCount`, 0); + test('should not fire on mousedown when no prior selection', async () => { + await ctx.proxy.write('foo bar baz'); + strictEqual(callCount, 0); + + const dims = (await ctx.proxy.dimensions)!; + const termRect: any = await ctx.page.evaluate(`window.term.element.getBoundingClientRect()`); + const x = termRect.left + dims.css.cell.width * 5; + const y = termRect.top + dims.css.cell.height * 0.5; + await ctx.page.mouse.click(x, y); + + strictEqual(callCount, 0); + }); + + test('should fire once on mousedown to clear, and again on mouseup after drag', async () => { + await ctx.proxy.write('foo bar baz'); + await ctx.proxy.selectAll(); + strictEqual(callCount, 1); + + const dims = (await ctx.proxy.dimensions)!; + const termRect: any = await ctx.page.evaluate(`window.term.element.getBoundingClientRect()`); + const startX = termRect.left + dims.css.cell.width * 0.5; + const endX = termRect.left + dims.css.cell.width * 5; + const y = termRect.top + dims.css.cell.height * 0.5; + + await ctx.page.mouse.move(startX, y); + await ctx.page.mouse.down(); + strictEqual(callCount, 2); + + await ctx.page.mouse.move(endX, y); + strictEqual(callCount, 2); + + await ctx.page.mouse.up(); + strictEqual(callCount, 3); + }); }); test('onRender', async () => { From 55d12e03d1bfa2cdd985e15dfbb39172df05c395 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 8 Jan 2026 17:04:35 -0800 Subject: [PATCH 3/5] Remove unneeded test --- src/browser/services/SelectionService.test.ts | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/browser/services/SelectionService.test.ts b/src/browser/services/SelectionService.test.ts index 9213e8f8..67158def 100644 --- a/src/browser/services/SelectionService.test.ts +++ b/src/browser/services/SelectionService.test.ts @@ -496,19 +496,5 @@ describe('SelectionService', () => { assert.isFalse(selectionService.areCoordsInSelection([2, 1], [2, 0], [2, 1])); }); }); - - describe('onSelectionChange', () => { - it('should not fire when setting model properties directly', () => { - buffer.lines.set(0, stringToRow('foo bar')); - - let eventFired = false; - selectionService.onSelectionChange(() => { eventFired = true; }); - - selectionService.model.selectionStart = [0, 0]; - selectionService.model.selectionEnd = undefined; - - assert.isFalse(eventFired); - }); - }); }); From a437d12004843336af3b4e04fc3a96862428912e Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 8 Jan 2026 17:25:26 -0800 Subject: [PATCH 4/5] Poll to avoid a race --- test/playwright/Terminal.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/playwright/Terminal.test.ts b/test/playwright/Terminal.test.ts index 051a7ace..5e7da129 100644 --- a/test/playwright/Terminal.test.ts +++ b/test/playwright/Terminal.test.ts @@ -418,13 +418,13 @@ test.describe('API Integration Tests', () => { await ctx.page.mouse.move(startX, y); await ctx.page.mouse.down(); - strictEqual(callCount, 2); + await pollFor(ctx.page, () => callCount, 2); await ctx.page.mouse.move(endX, y); strictEqual(callCount, 2); await ctx.page.mouse.up(); - strictEqual(callCount, 3); + await pollFor(ctx.page, () => callCount, 3); }); }); From bd8454db551630b3affa0592d1c100f34df2a1d4 Mon Sep 17 00:00:00 2001 From: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Date: Thu, 8 Jan 2026 17:37:44 -0800 Subject: [PATCH 5/5] Another poll case --- test/playwright/Terminal.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/playwright/Terminal.test.ts b/test/playwright/Terminal.test.ts index 5e7da129..6dee11da 100644 --- a/test/playwright/Terminal.test.ts +++ b/test/playwright/Terminal.test.ts @@ -389,7 +389,7 @@ test.describe('API Integration Tests', () => { const y = termRect.top + dims.css.cell.height * 0.5; await ctx.page.mouse.click(x, y); - strictEqual(callCount, 2); + await pollFor(ctx.page, () => callCount, 2); }); test('should not fire on mousedown when no prior selection', async () => { @@ -401,6 +401,7 @@ test.describe('API Integration Tests', () => { const x = termRect.left + dims.css.cell.width * 5; const y = termRect.top + dims.css.cell.height * 0.5; await ctx.page.mouse.click(x, y); + await timeout(20); strictEqual(callCount, 0); });