fix(terminal): close the display:none fail-open in the inert-focus visibility guard

The checkVisibility-less fallback read computed `visibility` off the container
alone. `display` does not cascade to descendants in either engine, so a pane
inside a Tailwind `hidden` wrapper — the real Terminal.tsx shape for a hidden
worktree — was reported as rendered. That failed OPEN, toward claiming the
chord, which is the destructive arm. Walk ancestors for display:none, and treat
a container with no view as not rendered rather than as rendered.
This commit is contained in:
Brennan Benson
2026-08-28 06:13:34 -07:00
parent 0f67f57cce
commit dcbb970c66
2 changed files with 156 additions and 3 deletions
@@ -252,6 +252,59 @@ describe('inert-focus paste fallback', () => {
expect(onPasteKey).toHaveBeenCalledTimes(1)
})
// Coordinator finding, verified here at the fallback level: with several panes
// mounted, a pane that lost focus only because it was HIDDEN must not claim the
// chord. Real Tailwind `hidden` is display:none on a WRAPPER, and computed
// `display` does not cascade to descendants in either engine, so the check has
// to walk ancestors — reading `visibility` off the container alone fails OPEN.
it('does not claim inert focus when an ancestor wrapper is display:none', () => {
const wrapper = document.createElement('div')
document.body.append(wrapper)
wrapper.append(container)
const fallback = install()
inside.focus()
inside.blur()
expect(fallback.ownsInertFocus()).toBe(true)
// Exactly what Terminal.tsx does to a hidden worktree: `absolute inset-0 hidden`.
wrapper.style.display = 'none'
expect(fallback.ownsInertFocus()).toBe(false)
pressPasteChordOn(document.body)
expect(onPasteKey).not.toHaveBeenCalled()
})
// The defensive path has to fail CLOSED. A missed recovery is a dropped chord —
// the pre-PR behaviour, and the user can click and retry. A false claim pastes
// into an invisible terminal and consumes the chord app-wide, which they cannot
// undo. So an engine that can tell us nothing must not be read as "rendered".
it('refuses to claim when the container has no view to ask about visibility', () => {
const foreign = document.implementation.createHTMLDocument('detached')
const foreignContainer = foreign.createElement('div')
const foreignInside = foreign.createElement('textarea')
foreignContainer.append(foreignInside)
foreign.body.append(foreignContainer)
expect(foreignContainer.isConnected).toBe(true)
expect(foreignContainer.ownerDocument.defaultView).toBe(null)
// Claim ownership at install time, the path the pane itself takes on remount.
foreignInside.focus()
expect(foreign.activeElement).toBe(foreignInside)
const recovered: KeyboardEvent[] = []
const fallback = installInertFocusPasteFallback({
container: foreignContainer,
documentTarget: foreign,
onPasteKey: (event) => recovered.push(event),
onPasteEvent: () => {}
})
foreignInside.blur()
expect(foreign.activeElement).toBe(foreign.body)
expect(fallback.ownsInertFocus()).toBe(false)
expect(recovered).toEqual([])
fallback.dispose()
})
it('stops recovering pastes after dispose', () => {
const fallback = install()
inside.focus()
@@ -267,6 +320,91 @@ describe('inert-focus paste fallback', () => {
})
})
// Coordinator finding on #17020: `TerminalPane` is not a singleton, so several
// fallbacks are installed at the document at once. Two questions follow — can a
// hidden instance capture a chord aimed elsewhere, and can two instances both act
// on the same event? Real DOM and real dispatched events throughout.
describe('inert-focus paste fallback with several panes mounted', () => {
type Scene = {
panes: { container: HTMLElement; inside: HTMLTextAreaElement; keys: KeyboardEvent[] }[]
dispose: () => void
}
const buildScene = (count: number): Scene => {
document.body.innerHTML = ''
const disposers: (() => void)[] = []
const panes = Array.from({ length: count }, () => {
const wrapper = document.createElement('div')
const paneContainer = document.createElement('div')
const helper = document.createElement('textarea')
helper.className = 'xterm-helper-textarea'
paneContainer.append(helper)
wrapper.append(paneContainer)
document.body.append(wrapper)
const keys: KeyboardEvent[] = []
const fallback = installInertFocusPasteFallback({
container: paneContainer,
onPasteKey: (event) => keys.push(event),
onPasteEvent: () => {}
})
disposers.push(fallback.dispose)
return { container: paneContainer, inside: helper, keys }
})
return { panes, dispose: () => disposers.forEach((d) => d()) }
}
const chord = (): void => {
document.body.dispatchEvent(
new KeyboardEvent('keydown', { key: 'v', ctrlKey: true, bubbles: true, cancelable: true })
)
}
it('lets a hidden background pane capture nothing after a focus-less switch away', () => {
// The reported chain: pane A holds focus, the user switches by KEYBOARD (no
// pointerdown), A's wrapper is hidden so the browser blurs it to <body> with
// NO focusin, and the destination surface takes no focus of its own.
const scene = buildScene(3)
scene.panes[0]!.inside.focus()
;(scene.panes[0]!.container.parentElement as HTMLElement).style.display = 'none'
expect(document.activeElement).toBe(scene.panes[0]!.inside)
// happy-dom does not blur on hide, so blur explicitly: the browser's behaviour
// is the premise, not the thing under test.
scene.panes[0]!.inside.blur()
expect(document.activeElement).toBe(document.body)
chord()
expect(scene.panes.map((pane) => pane.keys.length)).toEqual([0, 0, 0])
scene.dispose()
})
it('never lets two panes act on the same chord', () => {
const scene = buildScene(3)
scene.panes[0]!.inside.focus()
// A real focus move to another pane, which every instance sees at the document.
scene.panes[1]!.inside.focus()
scene.panes[1]!.inside.blur()
chord()
expect(scene.panes.map((pane) => pane.keys.length)).toEqual([0, 1, 0])
scene.dispose()
})
it('leaves the chord to the app when no pane ever held focus', () => {
const scene = buildScene(3)
const elsewhere = document.createElement('input')
document.body.append(elsewhere)
elsewhere.focus()
elsewhere.blur()
chord()
expect(scene.panes.map((pane) => pane.keys.length)).toEqual([0, 0, 0])
scene.dispose()
})
})
describe('restoreInertFocusPasteTarget', () => {
const buildPane = (): { container: HTMLElement; terminal: { focus: Mock<() => void> } } => {
document.body.innerHTML = ''
@@ -95,9 +95,24 @@ function isContainerRendered(container: HTMLElement): boolean {
if (typeof checkVisibility === 'function') {
return checkVisibility.call(container, { visibilityProperty: true })
}
// Fallback for engines without checkVisibility; `visibility` is inherited, so
// reading it off the container also catches a hidden ancestor.
return container.ownerDocument.defaultView?.getComputedStyle(container).visibility !== 'hidden'
// Why: only for engines without checkVisibility. `display` does not cascade to
// descendants, so reading it off the container alone would report a pane inside
// a `hidden` wrapper as rendered — failing open, toward claiming the chord. Walk
// the ancestors instead. `visibility` is inherited, so the container answers for
// itself. Bounded by tree depth and only reached on this defensive path.
const view = container.ownerDocument.defaultView
if (!view) {
return false
}
if (view.getComputedStyle(container).visibility === 'hidden') {
return false
}
for (let node: HTMLElement | null = container; node; node = node.parentElement) {
if (view.getComputedStyle(node).display === 'none') {
return false
}
}
return true
}
/** Focus is on <body> but the pane still owns it logically; putting it back before