Skip to content

Commit d9c4920

Browse files
authored
fix: restore selection should consider the window of the container (#30951)
## Summary Fixes #30864 Before this PR the active elemen was always taken from the global `window`. This is incorrect if the renderer is in one window rendering into a container element in another window. The changes in this PR adds another code branch to use a `defaultView` of the container element if it exists so that `restoreSelection` after a commit will actually restore to the correct window. ## How did you test this change? I patched these changes to the repro repo in the linked issue #39864 https://github.com/ling1726/react-child-window-focus-repro/blob/master/patches/react-dom%2B18.3.1.patch. I followed the same repro steps in the linked issue and and could not repro the reported problem. Attaching screen recordings below: Before ![focus repro](https://github.com/user-attachments/assets/81c4b4f9-08b5-4356-8251-49b909771f3f) After ![after](https://github.com/user-attachments/assets/84883032-5558-4650-9b9a-bd4d5fd9cb13)
1 parent d3d4d3a commit d9c4920

File tree

3 files changed

+64
-9
lines changed

3 files changed

+64
-9
lines changed

packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ export function getPublicInstance(instance: Instance): Instance {
325325

326326
export function prepareForCommit(containerInfo: Container): Object | null {
327327
eventsEnabled = ReactBrowserEventEmitterIsEnabled();
328-
selectionInformation = getSelectionInformation();
328+
selectionInformation = getSelectionInformation(containerInfo);
329329
let activeInstance = null;
330330
if (enableCreateEventHandleAPI) {
331331
const focusedElem = selectionInformation.focusedElem;
@@ -357,7 +357,7 @@ export function afterActiveInstanceBlur(): void {
357357
}
358358

359359
export function resetAfterCommit(containerInfo: Container): void {
360-
restoreSelection(selectionInformation);
360+
restoreSelection(selectionInformation, containerInfo);
361361
ReactBrowserEventEmitterSetEnabled(eventsEnabled);
362362
eventsEnabled = null;
363363
selectionInformation = null;

packages/react-dom-bindings/src/client/ReactInputSelection.js

+7-7
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ function isSameOriginFrame(iframe) {
5656
}
5757
}
5858

59-
function getActiveElementDeep() {
60-
let win = window;
61-
let element = getActiveElement();
59+
function getActiveElementDeep(containerInfo) {
60+
let win = containerInfo?.ownerDocument?.defaultView ?? window;
61+
let element = getActiveElement(win.document);
6262
while (element instanceof win.HTMLIFrameElement) {
6363
if (isSameOriginFrame(element)) {
6464
win = element.contentWindow;
@@ -97,8 +97,8 @@ export function hasSelectionCapabilities(elem) {
9797
);
9898
}
9999

100-
export function getSelectionInformation() {
101-
const focusedElem = getActiveElementDeep();
100+
export function getSelectionInformation(containerInfo) {
101+
const focusedElem = getActiveElementDeep(containerInfo);
102102
return {
103103
focusedElem: focusedElem,
104104
selectionRange: hasSelectionCapabilities(focusedElem)
@@ -112,8 +112,8 @@ export function getSelectionInformation() {
112112
* restore it. This is useful when performing operations that could remove dom
113113
* nodes and place them back in, resulting in focus being lost.
114114
*/
115-
export function restoreSelection(priorSelectionInformation) {
116-
const curFocusedElem = getActiveElementDeep();
115+
export function restoreSelection(priorSelectionInformation, containerInfo) {
116+
const curFocusedElem = getActiveElementDeep(containerInfo);
117117
const priorFocusedElem = priorSelectionInformation.focusedElem;
118118
const priorSelectionRange = priorSelectionInformation.selectionRange;
119119
if (curFocusedElem !== priorFocusedElem && isInDocument(priorFocusedElem)) {

packages/react-dom/src/__tests__/ReactDOMFiber-test.js

+55
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,23 @@ let act;
1919
let assertConsoleErrorDev;
2020
let assertLog;
2121
let root;
22+
let JSDOM;
2223

2324
describe('ReactDOMFiber', () => {
2425
let container;
2526

2627
beforeEach(() => {
2728
jest.resetModules();
29+
30+
// JSDOM needs to be setup with a TextEncoder and TextDecoder when used standalone
31+
// https://github.com/jsdom/jsdom/issues/2524
32+
(() => {
33+
const {TextEncoder, TextDecoder} = require('util');
34+
global.TextEncoder = TextEncoder;
35+
global.TextDecoder = TextDecoder;
36+
JSDOM = require('jsdom').JSDOM;
37+
})();
38+
2839
React = require('react');
2940
ReactDOM = require('react-dom');
3041
PropTypes = require('prop-types');
@@ -1272,4 +1283,48 @@ describe('ReactDOMFiber', () => {
12721283
});
12731284
expect(didCallOnChange).toBe(true);
12741285
});
1286+
1287+
it('should restore selection in the correct window', async () => {
1288+
// creating new JSDOM instance to get a second window as window.open is not implemented
1289+
// https://github.com/jsdom/jsdom/blob/c53efc81e75f38a0558fbf3ed75d30b78b4c4898/lib/jsdom/browser/Window.js#L987
1290+
const {window: newWindow} = new JSDOM('');
1291+
// creating a new container since the default cleanup expects the existing container to be in the document
1292+
const newContainer = newWindow.document.createElement('div');
1293+
newWindow.document.body.appendChild(newContainer);
1294+
root = ReactDOMClient.createRoot(newContainer);
1295+
1296+
const Test = () => {
1297+
const [reverse, setReverse] = React.useState(false);
1298+
const [items] = React.useState(() => ['a', 'b', 'c']);
1299+
const onClick = () => {
1300+
setReverse(true);
1301+
};
1302+
1303+
// shuffle the items so that the react commit needs to restore focus
1304+
// to the correct element after commit
1305+
const itemsToRender = reverse ? items.reverse() : items;
1306+
1307+
return (
1308+
<div>
1309+
{itemsToRender.map(item => (
1310+
<button onClick={onClick} key={item} id={item}>
1311+
{item}
1312+
</button>
1313+
))}
1314+
</div>
1315+
);
1316+
};
1317+
1318+
await act(() => {
1319+
root.render(<Test />);
1320+
});
1321+
1322+
newWindow.document.getElementById('a').focus();
1323+
await act(() => {
1324+
newWindow.document.getElementById('a').click();
1325+
});
1326+
1327+
expect(newWindow.document.activeElement).not.toBe(newWindow.document.body);
1328+
expect(newWindow.document.activeElement.innerHTML).toBe('a');
1329+
});
12751330
});

0 commit comments

Comments
 (0)