From f0dd459e0d97081cb3c313ec52285e3e422f8dbf Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 1 Mar 2022 15:23:16 -0800 Subject: [PATCH] reassign src from props not the dom element src property (#23395) The src property on the dom element will return a fully qualified name and this does not match the dom src attribute or the props provided to react. instead of reading from the element and re-assigning the property we assign the property from props which is how it was initially assigned during the render Co-authored-by: Josh Story --- .../ReactDOMImageLoad-test.internal.js | 55 +++++++++++++++++++ .../src/client/ReactDOMHostConfig.js | 2 +- 2 files changed, 56 insertions(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js index dc6c35ad93..50d3687c4e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMImageLoad-test.internal.js @@ -18,6 +18,7 @@ let ReactDOMClient; let originalCreateElement; // let TextResource; // let textResourceShouldFail; +let originalHTMLImageElementSrcDescriptor; let images = []; let onLoadSpy = null; @@ -165,6 +166,11 @@ describe('ReactDOMImageLoad', () => { return element; }; + originalHTMLImageElementSrcDescriptor = Object.getOwnPropertyDescriptor( + HTMLImageElement.prototype, + 'src', + ); + Object.defineProperty(HTMLImageElement.prototype, 'src', { get() { return this.getAttribute('src'); @@ -179,6 +185,11 @@ describe('ReactDOMImageLoad', () => { afterEach(() => { document.createElement = originalCreateElement; + Object.defineProperty( + HTMLImageElement.prototype, + 'src', + originalHTMLImageElementSrcDescriptor, + ); }); it('captures the load event if it happens before commit phase and replays it between layout and passive effects', async function() { @@ -575,4 +586,48 @@ describe('ReactDOMImageLoad', () => { 'Committed', ]); }); + + it('preserves the src property / attribute when triggering a potential new load event', () => { + // this test covers a regression identified in https://github.com/mui/material-ui/pull/31263 + // where the resetting of the src property caused the property to change from relative to fully qualified + + // make sure we are not using the patched src setter + Object.defineProperty( + HTMLImageElement.prototype, + 'src', + originalHTMLImageElementSrcDescriptor, + ); + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + + React.startTransition(() => + root.render( + + + + + , + ), + ); + + // render to yield to capture state of img src attribute and property before commit + expect(Scheduler).toFlushAndYieldThrough([ + 'render start', + 'Img default', + 'Yield', + ]); + const img = last(images); + const renderSrcProperty = img.src; + const renderSrcAttr = img.getAttribute('src'); + + // finish render and commit causing the src property to be rewritten + expect(Scheduler).toFlushAndYield(['a', 'last layout', 'last passive']); + const commitSrcProperty = img.src; + const commitSrcAttr = img.getAttribute('src'); + + // ensure attribute and properties agree + expect(renderSrcProperty).toBe(commitSrcProperty); + expect(renderSrcAttr).toBe(commitSrcAttr); + }); }); diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index fbd04da485..739c3b0232 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -442,7 +442,7 @@ export function commitMount( return; case 'img': { if ((newProps: any).src) { - ((domElement: any): HTMLImageElement).src = ((domElement: any): HTMLImageElement).src; + ((domElement: any): HTMLImageElement).src = (newProps: any).src; } return; }