Skip to content

Commit ee15b3b

Browse files
fix: error overlay hijacking application focus (safari) (#53693)
### What? When Safari is in the background and HMR triggers a full page reload, Safari hijacks application focus. ### Why? Having a `role="dialog"` is correctly prompting Safari to autofocus the first focusable element (the close button). However, Safari's behavior seems to also bring the application to the foreground when a background focus event occurs. ### How? This only adds the role when the document is focused. #### Before https://github.com/vercel/next.js/assets/1939140/9d2cce52-c6ee-4d49-9262-03620efad86c #### After https://github.com/vercel/next.js/assets/1939140/dc7d337c-b621-49e9-9a17-03b5d8b5c3f4 Confirmed voiceover behavior still appears to be correct <img width="1371" alt="CleanShot 2023-08-07 at 12 14 34@2x" src="https://github.com/vercel/next.js/assets/1939140/e53acfbc-cf6b-4d74-8b83-cf98edb2c2ab"> slack x-ref: https://vercel.slack.com/archives/C03KAR5DCKC/p1691264077313599 Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
1 parent 9d1b3f4 commit ee15b3b

4 files changed

Lines changed: 62 additions & 6 deletions

File tree

  • packages
    • next-swc/crates/next-core/js/src/overlay/internal/components/Dialog
    • next/src/client/components/react-dev-overlay/internal/components/Dialog
    • react-dev-overlay/src/internal/components/Dialog
  • test/development/client-dev-overlay

packages/next-swc/crates/next-core/js/src/overlay/internal/components/Dialog/Dialog.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ export function Dialog({
1919
...props
2020
}: DialogProps) {
2121
const [dialog, setDialog] = React.useState<HTMLDivElement | null>(null)
22+
const [role, setRole] = React.useState<string | undefined>(
23+
typeof document !== 'undefined' && document.hasFocus()
24+
? 'dialog'
25+
: undefined
26+
)
2227
const onDialog = React.useCallback(
2328
(node: React.SetStateAction<HTMLDivElement | null>) => {
2429
setDialog(node)
@@ -55,16 +60,27 @@ export function Dialog({
5560
}
5661
}
5762

63+
function handleFocus() {
64+
// safari will force itself as the active application when a background page triggers any sort of autofocus
65+
// this is a workaround to only set the dialog role if the document has focus
66+
setRole(document.hasFocus() ? 'dialog' : undefined)
67+
}
68+
5869
shadowRoot.addEventListener('keydown', handler as EventListener)
59-
return () =>
70+
window.addEventListener('focus', handleFocus)
71+
window.addEventListener('blur', handleFocus)
72+
return () => {
6073
shadowRoot.removeEventListener('keydown', handler as EventListener)
74+
window.removeEventListener('focus', handleFocus)
75+
window.removeEventListener('blur', handleFocus)
76+
}
6177
}, [dialog])
6278

6379
return (
6480
<div
6581
ref={onDialog}
6682
tabIndex={-1}
67-
role="dialog"
83+
role={role}
6884
aria-labelledby={props['aria-labelledby']}
6985
aria-describedby={props['aria-describedby']}
7086
aria-modal="true"

packages/next/src/client/components/react-dev-overlay/internal/components/Dialog/Dialog.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ const Dialog: React.FC<DialogProps> = function Dialog({
1616
...props
1717
}) {
1818
const [dialog, setDialog] = React.useState<HTMLDivElement | null>(null)
19+
const [role, setRole] = React.useState<string | undefined>(
20+
typeof document !== 'undefined' && document.hasFocus()
21+
? 'dialog'
22+
: undefined
23+
)
1924
const onDialog = React.useCallback((node: HTMLDivElement | null) => {
2025
setDialog(node)
2126
}, [])
@@ -48,17 +53,28 @@ const Dialog: React.FC<DialogProps> = function Dialog({
4853
}
4954
}
5055

56+
function handleFocus() {
57+
// safari will force itself as the active application when a background page triggers any sort of autofocus
58+
// this is a workaround to only set the dialog role if the document has focus
59+
setRole(document.hasFocus() ? 'dialog' : undefined)
60+
}
61+
5162
shadowRoot.addEventListener('keydown', handler as EventListener)
52-
return () =>
63+
window.addEventListener('focus', handleFocus)
64+
window.addEventListener('blur', handleFocus)
65+
return () => {
5366
shadowRoot.removeEventListener('keydown', handler as EventListener)
67+
window.removeEventListener('focus', handleFocus)
68+
window.removeEventListener('blur', handleFocus)
69+
}
5470
}, [dialog])
5571

5672
return (
5773
<div
5874
ref={onDialog}
5975
data-nextjs-dialog
6076
tabIndex={-1}
61-
role="dialog"
77+
role={role}
6278
aria-labelledby={props['aria-labelledby']}
6379
aria-describedby={props['aria-describedby']}
6480
aria-modal="true"

packages/react-dev-overlay/src/internal/components/Dialog/Dialog.tsx

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,11 @@ const Dialog: React.FC<DialogProps> = function Dialog({
1616
...props
1717
}) {
1818
const [dialog, setDialog] = React.useState<HTMLDivElement | null>(null)
19+
const [role, setRole] = React.useState<string | undefined>(
20+
typeof document !== 'undefined' && document.hasFocus()
21+
? 'dialog'
22+
: undefined
23+
)
1924
const onDialog = React.useCallback((node: HTMLDivElement | null) => {
2025
setDialog(node)
2126
}, [])
@@ -48,17 +53,28 @@ const Dialog: React.FC<DialogProps> = function Dialog({
4853
}
4954
}
5055

56+
function handleFocus() {
57+
// safari will force itself as the active application when a background page triggers any sort of autofocus
58+
// this is a workaround to only set the dialog role if the document has focus
59+
setRole(document.hasFocus() ? 'dialog' : undefined)
60+
}
61+
5162
shadowRoot.addEventListener('keydown', handler as EventListener)
52-
return () =>
63+
window.addEventListener('focus', handleFocus)
64+
window.addEventListener('blur', handleFocus)
65+
return () => {
5366
shadowRoot.removeEventListener('keydown', handler as EventListener)
67+
window.removeEventListener('focus', handleFocus)
68+
window.removeEventListener('blur', handleFocus)
69+
}
5470
}, [dialog])
5571

5672
return (
5773
<div
5874
ref={onDialog}
5975
data-nextjs-dialog
6076
tabIndex={-1}
61-
role="dialog"
77+
role={role}
6278
aria-labelledby={props['aria-labelledby']}
6379
aria-describedby={props['aria-describedby']}
6480
aria-modal="true"

test/development/client-dev-overlay/index.test.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,4 +72,12 @@ describe('client-dev-overlay', () => {
7272
return exists ? 'found' : 'success'
7373
}, 'success')
7474
})
75+
76+
it('should have a role of "dialog" if the page is focused', async () => {
77+
await check(async () => {
78+
return (await elementExistsInNextJSPortalShadowDOM('[role="dialog"]'))
79+
? 'exists'
80+
: 'missing'
81+
}, 'exists')
82+
})
7583
})

0 commit comments

Comments
 (0)