diff --git a/docs/reports/mobile-tab-keyboard/README.md b/docs/reports/mobile-tab-keyboard/README.md new file mode 100644 index 000000000..bbc433ebb --- /dev/null +++ b/docs/reports/mobile-tab-keyboard/README.md @@ -0,0 +1,22 @@ +# Mobile tab keyboard focus (#35) + +Root cause: explicit touch selection shared a policy that only focused the terminal +when KeyboardHandler already considered the keyboard visible; reselecting the active +tab returned before focusing. The fork's tab sheet also scheduled search focus 180ms +after opening, without cancelling it when a session was selected. A quick selection +therefore focused the terminal and then the closed sheet's offscreen search input. + +The fix focuses terminal input synchronously for explicit selections, including the +active tab, preserves passive app-driven restore when the keyboard is hidden, and +cancels/guards delayed search focus before closing the sheet and selecting a session. + +Chromium phone-emulation checks used two isolated disposable shell sessions on a +unique data directory and tmux socket. Switching tabs and quick active-tab reselection +kept focus on xterm's textarea after the delay; the sheet closed and no page errors +occurred. Screenshots show that isolated UI. No production session input was used. +Headless Chromium validates DOM focus, not native iOS keyboard presentation. WebKit +was unavailable on this machine; real-device iOS verification remains necessary. + +Six focused browser checks and 172 related tests passed. The broader smoke retained +the two previously recorded long-scrollback wheel failures; no new focus/browser +errors appeared. See the GitHub issue for build, deployment and CI gates. diff --git a/docs/reports/mobile-tab-keyboard/desktop.png b/docs/reports/mobile-tab-keyboard/desktop.png new file mode 100644 index 000000000..3ff7f0b36 Binary files /dev/null and b/docs/reports/mobile-tab-keyboard/desktop.png differ diff --git a/docs/reports/mobile-tab-keyboard/mobile.png b/docs/reports/mobile-tab-keyboard/mobile.png new file mode 100644 index 000000000..19675b89d Binary files /dev/null and b/docs/reports/mobile-tab-keyboard/mobile.png differ diff --git a/src/web/public/app.js b/src/web/public/app.js index 89a126e94..0458d90a6 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -10275,9 +10275,9 @@ class CodemanApp { // instead of switching to it; on a window too narrow for the grid it is an // ordinary click. if ((event?.ctrlKey || event?.metaKey) && this.addSessionToTiles?.(sessionId)) return; - // On touch with the keyboard hidden, blur the tapped tab so switching - // sessions doesn't pop the on-screen keyboard. Focus policy itself lives - // in selectSession via _shouldFocusTerminalForTabSwitch(). + // A hidden mobile keyboard can leave the terminal textarea focused. Blur + // first so this explicit tap can reopen it with a fresh synchronous focus. + // Automatic selections keep their passive focus policy in selectSession(). const keyboardOpen = typeof KeyboardHandler !== 'undefined' && KeyboardHandler.keyboardVisible === true; if (!keyboardOpen && MobileDetection.isTouchDevice()) { document.activeElement?.blur?.(); @@ -11590,11 +11590,12 @@ class CodemanApp { bar.hidden = false; } - _shouldFocusTerminalForTabSwitch() { + _shouldFocusTerminalForTabSwitch(options = {}) { if (typeof MobileDetection === 'undefined' || !MobileDetection.isTouchDevice()) { return true; } - return typeof KeyboardHandler !== 'undefined' && KeyboardHandler.keyboardVisible; + return options.auto !== true || + (typeof KeyboardHandler !== 'undefined' && KeyboardHandler.keyboardVisible); } async selectSession(sessionId, options = {}) { @@ -11626,6 +11627,14 @@ class CodemanApp { // fails toward acknowledging rather than toward an alert that can never be // cleared. const userInitiated = options?.auto !== true; + const focusTerminalWithoutActivation = () => { + const focusTerminal = () => this.terminal.focus(); + if (this._terminalViewerActivity?.runWithoutActivation) { + this._terminalViewerActivity.runWithoutActivation(focusTerminal); + } else { + focusTerminal(); + } + }; // Terminal sizing activity currently shares this audited `auto: true` // intent contract: app-restored selections stay passive, human choices own. // `!activeWebviewId`: with a dashboard foregrounded, re-selecting the @@ -11641,6 +11650,7 @@ class CodemanApp { this.applyTerminalFontPolicy?.(this.sessions.get(sessionId)?.mode); try { this.syncTerminalGeometry?.(); } catch {} this._terminalViewerActivity?.activate('session-select'); + if (this.terminal && this._shouldFocusTerminalForTabSwitch(options)) focusTerminalWithoutActivation(); } return; } @@ -11685,17 +11695,9 @@ class CodemanApp { // programmatic focus() within the user-gesture call stack (e.g. tab click). // After the first await the gesture context is lost and focus() is silently // ignored, leaving the keyboard unable to send input to the terminal. - // Desktop always focuses; touch focuses only while the on-screen keyboard - // is already open (so a tab switch doesn't pop the keyboard). - const shouldFocusTerminal = this._shouldFocusTerminalForTabSwitch(); - const focusTerminalWithoutActivation = () => { - const focusTerminal = () => this.terminal.focus(); - if (this._terminalViewerActivity?.runWithoutActivation) { - this._terminalViewerActivity.runWithoutActivation(focusTerminal); - } else { - focusTerminal(); - } - }; + // Explicit mobile selection opens the keyboard. App-driven selection only + // preserves an already-visible keyboard; boot/replay must not pop it open. + const shouldFocusTerminal = this._shouldFocusTerminalForTabSwitch(options); if (shouldFocusTerminal && this.terminal) { focusTerminalWithoutActivation(); } diff --git a/src/web/public/mobile-tabs.js b/src/web/public/mobile-tabs.js index b0040016a..414b28dfa 100644 --- a/src/web/public/mobile-tabs.js +++ b/src/web/public/mobile-tabs.js @@ -431,13 +431,18 @@ const MobileTabSheet = { const scrim = document.getElementById('mobileTabScrim'); const pill = document.getElementById('mobileTabPill'); if (!sheet || !scrim) return; + clearTimeout(this._searchFocusTimer); + this._searchFocusTimer = null; const willOpen = typeof open === 'boolean' ? open : !sheet.classList.contains('open'); sheet.classList.toggle('open', willOpen); scrim.classList.toggle('open', willOpen); if (pill) pill.setAttribute('aria-expanded', willOpen ? 'true' : 'false'); if (willOpen) { this.render(); - setTimeout(() => document.getElementById('mobileTabSearch')?.focus(), 180); + this._searchFocusTimer = setTimeout(() => { + this._searchFocusTimer = null; + if (this.isOpen()) document.getElementById('mobileTabSearch')?.focus(); + }, 180); document.body.style.overflow = 'hidden'; } else { document.body.style.overflow = ''; @@ -547,10 +552,12 @@ const MobileTabSheet = { listEl.innerHTML = html.join(''); }, activate(id) { + // Retire delayed search focus before handing the gesture to the terminal. + this.toggle(false); try { - window.app.selectSession(id); + if (window.app.handleSessionTabClick) window.app.handleSessionTabClick(null, id); + else window.app.selectSession(id); } catch {} - this.toggle(false); }, closeTab(id) { try { diff --git a/test/mobile-tab-keyboard-focus.browser.test.ts b/test/mobile-tab-keyboard-focus.browser.test.ts new file mode 100644 index 000000000..8150865e0 --- /dev/null +++ b/test/mobile-tab-keyboard-focus.browser.test.ts @@ -0,0 +1,133 @@ +/** Real browser DOM/focus regression; uses exact methods, no live sessions or native keyboard claims. */ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import ts from 'typescript'; +import { chromium, type Browser, type Page } from 'playwright'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +const publicDir = resolve(import.meta.dirname, '../src/web/public'); +function method(file: string, name: string): string { + const source = readFileSync(resolve(publicDir, file), 'utf8'); + const tree = ts.createSourceFile(file, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.JS); + let result = ''; + const visit = (node: ts.Node) => { + if (ts.isMethodDeclaration(node) && node.name.getText(tree) === name) result = node.getText(tree); + ts.forEachChild(node, visit); + }; + visit(tree); + if (!result) throw new Error(`Missing ${name}`); + return result; +} +const methods = ['handleSessionTabClick', '_shouldFocusTerminalForTabSwitch', 'selectSession'] + .map((name) => method('app.js', name)) + .join(','); +async function harness(browser: Browser, touch = true): Promise { + const page = await browser.newPage({ + viewport: { width: touch ? 393 : 1280, height: 852 }, + hasTouch: touch, + isMobile: touch, + }); + await page.setContent( + '' + ); + await page.addScriptTag({ + content: ` + window.KeyboardHandler={keyboardVisible:false};window.MobileDetection={isTouchDevice:()=>${touch}}; + window._crashDiag={log(){}};window.focusCalls=0; + window.app={${methods},activeSessionId:'a',isSoloWindow:true,detachedSessions:new Set(), + sessions:new Map([['b',{name:'b'}]]),_selectGeneration:0, + terminal:{focus(){focusCalls++;document.getElementById('terminal').focus()}}, + _clearTimer(){},_flushLocalEchoTo(){},_setTerminalLoadState(){},_cleanupPreviousSession(){throw Error('STOP_BEFORE_SESSION_IO')},markIdleAlertSeen(){}}; + const select=app.selectSession;app.selectSession=(...args)=>select.apply(app,args).catch(e=>{ + if(e.message!=='STOP_BEFORE_SESSION_IO')throw e; + }); + document.getElementById('same').onclick=e=>app.handleSessionTabClick(e,'a'); + document.getElementById('other').onclick=e=>app.handleSessionTabClick(e,'b'); + `, + }); + return page; +} +describe('explicit mobile tab keyboard focus', () => { + let browser: Browser; + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + }); + afterAll(async () => { + await browser.close(); + }); + for (const target of ['same', 'other']) { + it(`focuses the terminal synchronously for a closed-keyboard ${target} tab tap`, async () => { + const page = await harness(browser); + try { + await page.locator('#terminal').focus(); // stale textarea focus with keyboard gone + await page.locator(`#${target}`).tap(); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('terminal'); + expect(await page.evaluate('focusCalls')).toBe(1); + } finally { + await page.close(); + } + }); + } + it('direct same-session selection also restores terminal focus', async () => { + const page = await harness(browser); + try { + await page.evaluate('app.selectSession("a")'); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('terminal'); + } finally { + await page.close(); + } + }); + it('keeps keyboard-hidden automatic selection passive, and preserves an existing keyboard', async () => { + const page = await harness(browser); + try { + await page.evaluate('app.selectSession("b",{auto:true})'); + expect(await page.evaluate('focusCalls')).toBe(0); + await page.evaluate('KeyboardHandler.keyboardVisible=true;app.selectSession("b",{auto:true})'); + expect(await page.evaluate('focusCalls')).toBe(1); + } finally { + await page.close(); + } + }); + it('preserves desktop selection focus', async () => { + const page = await harness(browser, false); + try { + await page.locator('#other').click(); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('terminal'); + } finally { + await page.close(); + } + }); +}); + +describe('mobile sheet delayed search focus', () => { + let browser: Browser; + beforeAll(async () => { + browser = await chromium.launch({ headless: true }); + }); + afterAll(async () => { + await browser.close(); + }); + it('cancels pending search focus on a quick selection and leaves input on the terminal', async () => { + const page = await harness(browser); + try { + await page.addStyleTag({ content: readFileSync(resolve(publicDir, 'mobile.css'), 'utf8') }); + await page.evaluate(() => { + document.body.insertAdjacentHTML( + 'beforeend', + '
' + ); + }); + await page.addScriptTag({ + content: `window.MobileTabSheet={${['toggle', 'activate', 'isOpen'].map((n) => method('mobile-tabs.js', n)).join(',')},render(){}};`, + }); + await page.evaluate('MobileTabSheet.toggle(true);MobileTabSheet.activate("b")'); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('terminal'); + await page.waitForTimeout(320); + expect(await page.evaluate(() => document.activeElement?.id)).toBe('terminal'); + expect(await page.evaluate('MobileTabSheet.isOpen()')).toBe(false); + await page.screenshot({ path: '/tmp/mobile-tab-keyboard-focus.png' }); + } finally { + await page.close(); + } + }); +}); diff --git a/test/mobile/tabs.test.ts b/test/mobile/tabs.test.ts index fad84a4e6..e6340303b 100644 --- a/test/mobile/tabs.test.ts +++ b/test/mobile/tabs.test.ts @@ -800,7 +800,7 @@ describe('Tab Navigation', () => { }); describe('Tab Touch Focus', () => { - it('switching tabs with the keyboard closed does not leave the terminal textarea focused', async () => { + it('a closed-keyboard tab click retires stale textarea focus before explicit selection', async () => { const { context, page } = await createDevicePage(standardPhone, baseUrl, 'chromium'); try { await page.waitForTimeout(WAIT.PAGE_SETTLE); @@ -836,7 +836,7 @@ describe('Tab Navigation', () => { expect(result.hasHandler).toBe(true); expect(result.selected).toBe('mock-session-2'); - expect(result.preserveKeyboard).toBe(false); + expect(result.preserveKeyboard).toBeUndefined(); expect(result.activeIsTextarea).toBe(false); } finally { await context.close();