Skip to content

Commit bbbfbf9

Browse files
xavier-lcXavier Lozano Carrerasjsnajdr
authored
useTypingObserver: capture the window reference for cleanup (#78772)
* useTypingObserver: guard against null defaultView during cleanup When the block editor is hosted inside an iframe, React can fire the useRefEffect cleanup after the iframe document has been detached from its window. At that point document.defaultView === null, so the unguarded `node.ownerDocument.defaultView.clearTimeout( timerId )` inside the cleanup throws, which also aborts the removeEventListener calls that follow it (leaking those listeners on the unmounting node). Optional-chain the defaultView access in the cleanup, plus the matching setTimeout in stopTypingOnNonTextField and getSelection in stopTypingOnSelectionUncollapse, so the hook never assumes the iframe's window is still attached. * useTypingObserver: add regression test for null defaultView cleanup Mounts the hook in the isTyping state so the cleanup branch that touches `defaultView` is reached, overrides `ownerDocument.defaultView` to null to simulate the iframe being detached from its window, then unmounts. Asserts both that the cleanup does not throw and that the `selectionchange`, `focus`, and `keydown` listeners are still removed in that scenario — the leak path the original throw was masking. * Re-trigger CI (unrelated flake in CustomSelectControl test) * Remove null-defaultView cleanup test. The test mocked an internal edge case (manually nulling `ownerDocument.defaultView`) that doesn't correspond to a reachable real-world state, so it asserted against a synthetic condition rather than observable behavior. * Use top-level timers so cleanup survives a detached frame. The cleanup function called `node.ownerDocument.defaultView.clearTimeout()`. When the editor canvas iframe is detached from its window before React runs the ref cleanup, `defaultView` is `null` and the access throws, aborting the rest of the cleanup and leaking the `focus`, `keydown`, and `selectionchange` listeners. The typing timer never needed the iframe's window in the first place, so set and clear it on the top-level `setTimeout`/`clearTimeout` instead. The timer id stays valid regardless of the frame's state, and the cleanup no longer touches `defaultView`. * Update CHANGELOG * Fix comment * Re-trigger CI (flaky @swc native binding load in Storybook smoke tests) --------- Co-authored-by: Xavier Lozano Carreras <xavier.lozano.carreras@a8c.com> Co-authored-by: Jarda Snajdr <jsnajdr@gmail.com>
1 parent 8bc5bfc commit bbbfbf9

2 files changed

Lines changed: 14 additions & 5 deletions

File tree

packages/block-editor/CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,14 @@
22

33
## Unreleased
44

5+
### Bug Fixes
6+
7+
- `useTypingObserver`: Capture the window reference at mount and reuse it during cleanup so the ref cleanup no longer reads `node.ownerDocument.defaultView` (which is `null` once the iframe-hosted editor has been detached from its window) and throws, which was also leaking the `removeEventListener` calls that follow it ([#78772](https://github.com/WordPress/gutenberg/pull/78772)).
8+
59
### Breaking Changes
610

711
- The `__next40pxDefaultSize` prop is now true by default. The prop can be safely removed from the following:
8-
- `LetterSpacingControl` ([#79533](https://github.com/WordPress/gutenberg/pull/79533)).
12+
- `LetterSpacingControl` ([#79533](https://github.com/WordPress/gutenberg/pull/79533)).
913

1014
### Deprecations
1115

packages/block-editor/src/components/observe-typing/index.js

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,12 @@ export function useTypingObserver() {
129129
if ( isTyping ) {
130130
let timerId;
131131

132+
// Capture the window reference while the node is still
133+
// attached. Reusing the reference we held at mount keeps the
134+
// cleanup and the handlers working against the same window
135+
// we set things up on.
136+
const { defaultView } = node.ownerDocument;
137+
132138
/**
133139
* Stops typing when focus transitions to a non-text field element.
134140
*
@@ -141,7 +147,7 @@ export function useTypingObserver() {
141147
// before the keydown event, wait until after current stack
142148
// before evaluating whether typing is to be stopped. Otherwise,
143149
// typing will re-start.
144-
timerId = node.ownerDocument.defaultView.setTimeout( () => {
150+
timerId = defaultView.setTimeout( () => {
145151
if ( ! isTextField( target ) ) {
146152
stopTyping();
147153
}
@@ -168,8 +174,7 @@ export function useTypingObserver() {
168174
* uncollapsed (shift) selection.
169175
*/
170176
function stopTypingOnSelectionUncollapse() {
171-
const selection =
172-
node.ownerDocument.defaultView.getSelection();
177+
const selection = defaultView.getSelection();
173178
if ( ! selection.isCollapsed ) {
174179
stopTyping();
175180
}
@@ -184,7 +189,7 @@ export function useTypingObserver() {
184189
);
185190

186191
return () => {
187-
node.ownerDocument.defaultView.clearTimeout( timerId );
192+
defaultView.clearTimeout( timerId );
188193
node.removeEventListener(
189194
'focus',
190195
stopTypingOnNonTextField

0 commit comments

Comments
 (0)