fix: pr-bug-scan validated finding from #1899 (#1965)

* fix: address pr-bug-scan validated finding from #1899

Gate autoFocus: only focus the action button when no input/textarea/contentEditable is currently focused. Blocks focus theft from external inputs.

* fix: address review findings

---------

Co-authored-by: orca-bug-scan-bot <orca-bug-scan-bot@stably.ai>
Co-authored-by: Jinjing <6427696+AmethystLiang@users.noreply.github.com>
This commit is contained in:
buf0-bot[bot] 2026-05-15 18:03:38 -07:00 committed by GitHub
parent 46331f881f
commit 234552351e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 139 additions and 2 deletions

View File

@ -2,6 +2,7 @@ import { useEffect, useId, useRef, useState, type ReactElement } from 'react'
import { Button } from '@/components/ui/button'
import { cn } from '@/lib/utils'
import type { DriverState } from '@/lib/pane-manager/mobile-driver-state'
import { shouldFocusMobileDriverAction } from './mobile-driver-overlay-focus'
type Props = {
driver: DriverState
@ -127,8 +128,22 @@ function LoudOverlay({
}: LoudOverlayProps): ReactElement {
const titleId = useId()
const bodyId = useId()
const rootRef = useRef<HTMLDivElement>(null)
const actionRef = useRef<HTMLButtonElement>(null)
// Why: focus the recovery action on mount only when the user isn't already
// typing into another input (composer, command palette, settings field).
// Unconditional autoFocus yanks focus on every overlay mount, so a phone
// taking the floor while the desktop user is typing elsewhere would route
// the next Space/Enter into Take back / Restore. See PR #1899 follow-up.
useEffect(() => {
const paneScope = rootRef.current?.parentElement
if (shouldFocusMobileDriverAction(document.activeElement, document.body, paneScope)) {
actionRef.current?.focus()
}
}, [])
return (
<div
ref={rootRef}
role="dialog"
aria-live="assertive"
aria-labelledby={titleId}
@ -160,14 +175,14 @@ function LoudOverlay({
Collapse
</Button>
)}
{/* autoFocus lands keyboard users on the recovery action when the pane-local lock appears. */}
{/* Focus is moved to this button only when no user input is active; see effect above. */}
<Button
ref={actionRef}
type="button"
variant="default"
size="sm"
onClick={onAction}
disabled={actionPending}
autoFocus
>
{actionLabel}
</Button>

View File

@ -0,0 +1,78 @@
import { describe, expect, it } from 'vitest'
import { shouldFocusMobileDriverAction } from './mobile-driver-overlay-focus'
function focusLike(args: {
tagName?: string
xterm?: boolean
contentEditable?: boolean
editableAncestor?: boolean
selfMatchesEditable?: boolean
}): {
tagName?: string
isContentEditable: boolean
classList: { contains: (token: string) => boolean }
closest: (selector: string) => unknown
contains: (node: unknown) => boolean
} {
const element = {
tagName: args.tagName,
isContentEditable: args.contentEditable === true,
classList: {
contains: (token: string) => args.xterm === true && token === 'xterm-helper-textarea'
},
closest: (selector: string) =>
(args.editableAncestor === true || args.selfMatchesEditable === true) &&
selector === 'input, textarea, select, [contenteditable=""], [contenteditable="true"]'
? {}
: null,
contains: (node: unknown) => node === element
}
return element
}
function scopeContaining(node: unknown): { contains: (candidate: unknown) => boolean } {
return {
contains: (candidate: unknown) => candidate === node
}
}
describe('shouldFocusMobileDriverAction', () => {
it('focuses the recovery action when focus is neutral', () => {
const body = focusLike({})
expect(shouldFocusMobileDriverAction(null, body)).toBe(true)
expect(shouldFocusMobileDriverAction(body, body)).toBe(true)
})
it('preserves focus for real editable app controls', () => {
expect(
shouldFocusMobileDriverAction(focusLike({ tagName: 'INPUT', selfMatchesEditable: true }))
).toBe(false)
expect(
shouldFocusMobileDriverAction(focusLike({ tagName: 'TEXTAREA', selfMatchesEditable: true }))
).toBe(false)
expect(
shouldFocusMobileDriverAction(focusLike({ tagName: 'SELECT', selfMatchesEditable: true }))
).toBe(false)
expect(shouldFocusMobileDriverAction(focusLike({ editableAncestor: true }))).toBe(false)
expect(shouldFocusMobileDriverAction(focusLike({ contentEditable: true }))).toBe(false)
})
it('still focuses the recovery action for xterm helper textareas', () => {
const xterm = focusLike({ tagName: 'TEXTAREA', xterm: true })
expect(shouldFocusMobileDriverAction(xterm, undefined, scopeContaining(xterm))).toBe(true)
})
it('preserves focus for xterm helper textareas outside the overlay pane', () => {
const xterm = focusLike({ tagName: 'TEXTAREA', xterm: true })
expect(shouldFocusMobileDriverAction(xterm, undefined, scopeContaining({}))).toBe(false)
expect(shouldFocusMobileDriverAction(xterm)).toBe(false)
})
it('preserves browser guest focus represented by Electron webviews', () => {
expect(shouldFocusMobileDriverAction(focusLike({ tagName: 'WEBVIEW' }))).toBe(false)
})
})

View File

@ -0,0 +1,44 @@
type FocusLike = {
tagName?: string
isContentEditable?: boolean
classList?: { contains?: (token: string) => boolean }
closest?: (selector: string) => unknown
contains?: (node: unknown) => boolean
}
function isFocusLike(value: unknown): value is FocusLike {
return typeof value === 'object' && value !== null
}
export function shouldFocusMobileDriverAction(
active: unknown,
body?: unknown,
focusScope?: unknown
): boolean {
if (!isFocusLike(active) || active === body) {
return true
}
// Why: xterm owns keyboard input through a hidden textarea. When mobile takes
// that terminal over, focus should move to the recovery action, not stay in
// the now-paused terminal input. Scope this to the pane that owns the overlay
// so another active terminal pane keeps keyboard focus.
if (active.classList?.contains?.('xterm-helper-textarea')) {
return isFocusLike(focusScope) && focusScope.contains?.(active) === true
}
// Why: focused Electron webviews represent guest-page keyboard focus; they
// are not editable DOM controls in the host document, but stealing focus from
// them still interrupts the user's typing in the page.
if (active.tagName === 'WEBVIEW') {
return false
}
if (active.isContentEditable === true) {
return false
}
return !active.closest?.(
'input, textarea, select, [contenteditable=""], [contenteditable="true"]'
)
}