From f30d2f24c50bfb5fc86501134c5f450385f173a1 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Fri, 18 Sep 2026 10:54:04 -0400 Subject: [PATCH] Let a read-only view search: Enter in a field is not a grid gesture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The read-only gate swallowed every Enter in the capture phase to keep the inline cell editor shut. Since v1.24.0 Enter is also how a search commits, so on .slx, .mdl, .mat and .prj the keystroke died one element above the search box and the filter did nothing at all. .sldd was fine because it is editable, which is exactly the shape the bug was reported in. The header column-filter popup, which applies on Enter, was the second victim. The sibling cut/copy/paste guard already exempted text fields — one rule on two paths, and only one path knew it. Extract that test as isTypingInField and read it from both guards, so a key a field owns is never the table's to claim. Enter on the grid is still swallowed on a read-only document. Tests drive the listener that ships, not a copy of it: table-main.ts is imported with acquireVsCodeApi stubbed, then a read-only and an editable payload each get a real Enter in the search box and on the grid. --- src/webview/table-main.ts | 31 +++++++--- test/readonlyEditorGate.test.ts | 104 ++++++++++++++++++++++++++++++-- 2 files changed, 123 insertions(+), 12 deletions(-) diff --git a/src/webview/table-main.ts b/src/webview/table-main.ts index ca4e839..ed88d95 100644 --- a/src/webview/table-main.ts +++ b/src/webview/table-main.ts @@ -389,6 +389,19 @@ window.addEventListener('message', (event: MessageEvent) => { } }); +// Whether this keystroke is TEXT ENTRY rather than a gesture on the grid: the search +// box, the column-filter popup, the inline cell editor. Read off the COMPOSED PATH, +// because every one of those fields lives inside the table's shadow tree and `e.target` +// at this listener is the host element, which tells us nothing about where the caret is. +// +// The rule lives here once because both capture guards below need it and they must not +// disagree: a key that a field owns is not the table's to claim. +function isTypingInField(ev: KeyboardEvent): boolean { + const active = (ev.composedPath?.()[0] as HTMLElement) ?? (ev.target as HTMLElement); + const tag = active?.tagName; + return tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT' || !!active?.isContentEditable; +} + // Read-only documents never open the inline cell editor. The vendored table // opens it on double-click and on Enter, gating only on per-row flags (which we // keep intact for cell coloring). Intercept both gestures in the CAPTURE phase @@ -405,7 +418,13 @@ table.addEventListener( table.addEventListener( 'keydown', (e: Event) => { - if ((e as KeyboardEvent).key === 'Enter' && !shouldOpenCellEditor(editable)) { + const ev = e as KeyboardEvent; + // Only Enter on the GRID is the cell-editor gesture this gate exists for. Enter in a + // FIELD belongs to that field: the search box commits a search with it and the + // column-filter popup applies with it. Without this exemption, a read-only view + // (.slx, .mdl, .mat, .prj) had a search box that did nothing at all — the keystroke + // died here, one element above the box, and .sldd looked fine because it is editable. + if (ev.key === 'Enter' && !isTypingInField(ev) && !shouldOpenCellEditor(editable)) { e.stopPropagation(); } }, @@ -462,13 +481,9 @@ table.addEventListener( const action = resolveShortcutAction(ev); if (!action) return; - // While typing in the inline cell editor or the column filter, C/X/V and - // Delete/Backspace are text editing — let the field handle them natively. - const active = (ev.composedPath?.()[0] as HTMLElement) ?? (ev.target as HTMLElement); - const tag = active?.tagName; - if (tag === 'INPUT' || tag === 'TEXTAREA' || tag === 'SELECT' || active?.isContentEditable) { - return; - } + // While typing in the inline cell editor, the search box or the column filter, C/X/V + // and Delete/Backspace are text editing — let the field handle them natively. + if (isTypingInField(ev)) return; // The primary selection anchors the gesture, the same way the right-clicked row // does: it is what a paste targets the section of. diff --git a/test/readonlyEditorGate.test.ts b/test/readonlyEditorGate.test.ts index fa57ba4..98d14bb 100644 --- a/test/readonlyEditorGate.test.ts +++ b/test/readonlyEditorGate.test.ts @@ -1,6 +1,6 @@ // Copyright 2026 The MathWorks, Inc. // @vitest-environment happy-dom -import { describe, it, expect, beforeEach } from 'vitest'; +import { describe, it, expect, beforeAll, beforeEach } from 'vitest'; import { shouldOpenCellEditor } from '../src/webview/menuItems.js'; // The vendored dex-tree-table opens its inline cell editor from handlers bound @@ -11,9 +11,9 @@ import { shouldOpenCellEditor } from '../src/webview/menuItems.js'; // stopPropagation on the host must prevent a shadow-internal listener from // firing, and must do so exactly when the document is read-only. // -// This mirrors the component's event contract without depending on the vendored -// component (which we must not edit) or on table-main.ts (which runs top-level -// acquireVsCodeApi()/DOM wiring that can't be imported in isolation). +// This half mirrors the component's event contract on a bare host+shadow pair, so the +// mechanism is pinned independently of the table. The second half below drives the +// listener table-main.ts actually installs — which is where the interesting bug was. describe('read-only cell-editor gate (capture-phase interception)', () => { let host: HTMLElement; @@ -125,3 +125,99 @@ describe('read-only cell-editor gate (capture-phase interception)', () => { }); }); }); + +// ── the same gate, as it actually ships ─────────────────────────────────────────── +// Everything above installs a COPY of the guard. That is one rule on two paths, and +// this is the bug it let through: Enter became how a search commits, and the guard — +// which knows only "read-only" and "the key is Enter" — swallowed it in the capture +// phase before it could reach the search box. Every read-only view (.slx, .mdl, .mat, +// .prj) had a filter box that did nothing; .sldd, being editable, was fine, which is +// exactly the shape the bug was reported in. +// +// So this half drives the LISTENER THAT SHIPS. table-main.ts is importable with two +// stubs — `acquireVsCodeApi` as a global, since the module calls it at top level, and a +// in the body for it to bind to — both in place before the dynamic +// import. Imported once, in beforeAll: a module body runs once per test FILE, and this +// one wires window and body listeners. +describe('the shipped gate keeps a read-only view searchable', () => { + let table: any; + + beforeAll(async () => { + (globalThis as any).acquireVsCodeApi = () => ({ postMessage: () => {} }); + document.body.innerHTML = ''; + await import('../src/webview/table-main.js'); + table = document.querySelector('dex-tree-table'); + }); + + // A .slx-shaped payload: two rows and the read-only flag the gate reads. + async function paint(editable: boolean): Promise { + window.dispatchEvent( + new MessageEvent('message', { + data: { + type: 'setRows', + docUri: 'file:///fx/m.slx', + rows: [ + { ID: 's', parent: null, Name: { label: 'Model Elements' } }, + { ID: 'b', parent: 's', Name: { label: 'ChainGain' } }, + ], + columns: ['Name', 'Value', 'DataType'], + columnLabels: { Name: 'Name', Value: 'Value', DataType: 'Data Type' }, + editable, + }, + }), + ); + await table.updateComplete; + } + + const barOf = () => table.shadowRoot.querySelector('dex-filter-bar'); + const boxOf = () => barOf().shadowRoot.querySelector('.filter-input') as HTMLInputElement; + + /** + * Type a search and commit it with a real Enter, from inside the bar's shadow root. + * Each call is a FRESH question: the bar APPENDS its committed tail to what is already + * applied, and one table serves every case here. + */ + async function searchFor(text: string): Promise { + table._setFilterText(''); + await table.updateComplete; + const input = boxOf(); + input.value = text; + input.dispatchEvent(new Event('input', { bubbles: true })); + input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, composed: true })); + await barOf().updateComplete; + await table.updateComplete; + } + + it('read-only: Enter in the search box commits the search', async () => { + await paint(false); + await searchFor('ChainGain'); + expect(table._filterText).toBe('ChainGain'); + expect(table._getVisibleRows().map((r: { ID: string }) => r.ID)).toEqual(['s', 'b']); + }); + + it('editable: Enter in the search box commits the search too', async () => { + // The two views must agree. The gate is about the GRID, and the box is not the grid. + await paint(true); + await searchFor('ChainGain'); + expect(table._filterText).toBe('ChainGain'); + }); + + it('read-only: Enter on the grid is still swallowed before the cell editor sees it', async () => { + // The gate's whole purpose, unchanged: this is what the exemption above must not cost. + await paint(false); + let reached = 0; + const grid = table.shadowRoot.querySelector('[role="treegrid"]') as HTMLElement; + grid.addEventListener('keydown', () => reached++); + grid.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, composed: true })); + expect(reached).toBe(0); + }); + + it('editable: Enter on the grid still reaches it, so a cell can be opened', async () => { + await paint(true); + let reached = 0; + const grid = table.shadowRoot.querySelector('[role="treegrid"]') as HTMLElement; + grid.addEventListener('keydown', () => reached++); + grid.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true, composed: true })); + expect(reached).toBe(1); + }); +});