Skip to content

Iframe: avoid asset parsing & fix script localisation #52405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mcsf merged 2 commits into from
Jul 10, 2023
Merged

Iframe: avoid asset parsing & fix script localisation #52405

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
244de6e
Iframe: avoid asset parsing & fix script localisation
ellatrix Jul 7, 2023
89b5fe3
Add e2e test for script localisation
ellatrix Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 16 additions & 47 deletions packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
createPortal,
forwardRef,
useMemo,
useReducer,
useEffect,
} from '@wordpress/element';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -78,29 +77,6 @@ function bubbleEvents( doc ) {
}
}

function useParsedAssets( html ) {
return useMemo( () => {
const doc = document.implementation.createHTMLDocument( '' );
doc.body.innerHTML = html;
return Array.from( doc.body.children );
}, [ html ] );
}

async function loadScript( head, { id, src } ) {
return new Promise( ( resolve, reject ) => {
const script = head.ownerDocument.createElement( 'script' );
script.id = id;
if ( src ) {
script.src = src;
script.onload = () => resolve();
script.onerror = () => reject();
} else {
resolve();
}
head.appendChild( script );
} );
}

function Iframe( {
contentRef,
children,
Expand All @@ -112,21 +88,22 @@ function Iframe( {
forwardedRef: ref,
...props
} ) {
const assets = useSelect(
const { styles = '', scripts = '' } = useSelect(
( select ) =>
select( blockEditorStore ).getSettings().__unstableResolvedAssets,
[]
);
const [ , forceRender ] = useReducer( () => ( {} ) );
const [ iframeDocument, setIframeDocument ] = useState();
const [ bodyClasses, setBodyClasses ] = useState( [] );
const compatStyles = useCompatibilityStyles();
const scripts = useParsedAssets( assets?.scripts );
const clearerRef = useBlockSelectionClearer();
const [ before, writingFlowRef, after ] = useWritingFlow();
const [ contentResizeListener, { height: contentHeight } ] =
useResizeObserver();
const setRef = useRefEffect( ( node ) => {
node._load = () => {
setIframeDocument( node.contentDocument );
};
let iFrameDocument;
// Prevent the default browser action for files dropped outside of dropzones.
function preventFileDropDefault( event ) {
Expand All @@ -138,7 +115,6 @@ function Iframe( {
iFrameDocument = contentDocument;

bubbleEvents( contentDocument );
setIframeDocument( contentDocument );
clearerRef( documentElement );

// Ideally ALL classes that are added through get_body_class should
Expand All @@ -154,7 +130,6 @@ function Iframe( {
);

contentDocument.dir = ownerDocument.dir;
documentElement.removeChild( contentDocument.body );

for ( const compatStyle of compatStyles ) {
if ( contentDocument.getElementById( compatStyle.id ) ) {
Expand Down Expand Up @@ -199,35 +174,29 @@ function Iframe( {
};
}, [] );

const headRef = useRefEffect( ( element ) => {
scripts
.reduce(
( promise, script ) =>
promise.then( () => loadScript( element, script ) ),
Promise.resolve()
)
.finally( () => {
// When script are loaded, re-render blocks to allow them
// to initialise.
forceRender();
} );
}, [] );
const disabledRef = useDisabled( { isDisabled: ! readonly } );
const bodyRef = useMergeRefs( [
contentRef,
clearerRef,
writingFlowRef,
disabledRef,
headRef,
] );

// Correct doctype is required to enable rendering in standards
// mode. Also preload the styles to avoid a flash of unstyled
// content.
const html =
'<!doctype html>' +
'<style>html{height:auto!important;min-height:100%;}body{margin:0}</style>' +
( assets?.styles ?? '' );
const html = `<!doctype html>
<html>
<head>
<script>window.frameElement._load()</script>
<style>html{height:auto!important;min-height:100%;}body{margin:0}</style>
${ styles }
${ scripts }
</head>
<body>
<script>document.currentScript.parentElement.remove()</script>
</body>
Comment on lines +196 to +198
Copy link
Member

@Mamaduka Mamaduka Dec 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ellatrix, can you provide some additional details about this hack? Why is it needed, and what does it solve?

It looks like some internal changes in React 19 affect it: #61521 (comment).

Copy link
Member

@jsnajdr jsnajdr Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What this does: the styles and scripts are HTML literal values that contain styles and scripts to load. Their source is the __unstableResolvedAssets setting: a HTML chunk generated by the PHP _wp_get_iframed_editor_assets function.

The initial document in the iframe is this html that does the preloading. It:

  1. Notifies the parent frame that it has loaded (the _load() call)
  2. Starts loading the styles and scripts
  3. After all the loading is finished, the body element is removed by the script inside it.

At the same time, React will render a portal markup inside this iframe once the _load callback is called. This is the code that does this:

const [ iframeDocument, setIframeDocument ] = useState();
const ref = useRefEffect( ( node ) => {
  node._load = () => {
    setIframeDocument( node.contentDocument );
  };
} );
return <iframe ref={ ref }>{ iframeDocument && createPortal(
  <body><Editor /></body>,
  iframeDocument.documentElement )
}</iframe>;

The rendering of the portal is triggered by the _load call. That means that the portal is rendered, and its body is rendered, while the scripts and styles in the original document are still loading.

Whether this all works depends on the subtle details of what exactly React does when rendering the portal. The original content of the iframe is:

<html>
  <head>
    <script><link>
  </head>
  <body>
    <script>removeYourself();</script>
  </body>
</html>

The portal is rendered into the <html> element, that's what iframeDocument.documentElement points to. What happens to the existing <head> and <body> markup? I don't know. Usually the elements into which portals are rendered are supposed to be empty.

I don’t fully understand how this hack was solving script localization bug

Script localization is done with inline script tags. But if you look at the original code before this patch, namely the loadScript function, it can handle only <script src="..."> tags that load from a URL. Now the HTML chunks with the script tags are directly pasted as strings into the html content. And inline script tags are working.

Copy link
Member

@jsnajdr jsnajdr Dec 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #61521 @tyxla changed the portal-rendering condition to:

<iframe>{ iframeDocument?.body && createPortal() }</iframe>

adding a check if .body is there. But this is problematic. The node._load() call which triggers setState( iframeDocument ) happens in the iframe document's head. When that script is executed, however, the body element doesn't exist yet. The portal is not rendered. Later, when the body element is created, nobody tells React about that, there is no state update. The portal remains not rendered. Until some other unrelated state update triggers the component rerender and the .body check finally succeeds.

Try saving this into a .html file and open it in a browser:

<html>
<script>console.log('head body:',document.body)</script>
<body>hello</body>
<script>console.log('tail body:', document.body)</script>
</html>

The first console.log will log null, the second one will log a body element.

One very important thing about this is that between the ._load() call and the <body> element the HTML contains several scripts and styles loaded over the network. There is significant delay between them.

@Mamaduka created a Codesanbox toy example that doesn't have this property. There is nothing async there between the _load() and the <body>. The iframeDocument?.body code will reliably see the already existing body.

Copy link
Member

@jsnajdr jsnajdr Dec 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just verified on a toy app how React 18 and React 19 differ when rendering portals.

Load an iframe with this HTML as src in a blob URL:

<html>
  <head>
    <meta name=hello value=world>
    <script>window.frameElement._load()</script>
  </head>
  <body>
    <div>original</div>
  </body>
</html>

and then render a portal into that iframe's html element:

createPortal(
  <body>
    <div>portal</div>
  </body>,
  iframeElement.contentDocument.documentElement
);

React 18 creates two body elements, one with original content, other with portal content.
React 19 removes the original element and replaces it with body with portal content.

The head element is left intact, the meta and the script tag are still there. In both React 18 and 19.

So far this seems that React 19 fixed a bug with the double body element. And that we no longer need the self-destructing body element.

I still don't understand how this change could break some behavior and e2e tests. I'll continue looking into this.

Copy link
Member

@Mamaduka Mamaduka Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @jsnajdr!

Btw, I also noted that difference in #61521 (comment). Sorry, I should've highlighted it.

Copy link
Member Author

@ellatrix ellatrix Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's necessary to delete the body element, not just to avoid having two body elements, but also to prevent other scripts from attaching event handlers to the wrong body element.

Copy link
Member

@tyxla tyxla Dec 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this wasn't breaking E2E tests, it was literally breaking the editor. I did change it a few months ago when testing with one of React 19 betas, so I'll need to come back to it and see if it's still necessary, and if yes, if there is a better solution. Hoping to come back to this early next year.

Copy link
Member

@tyxla tyxla Aug 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reverted these changes in #61521 and retried this.

I think we still need to stop removing the body element, since in React 19 it will now ensure there's always one and only one body tag. I'd love to learn more about the script event handler issue you're talking about @ellatrix - how can we repro this?

The only other change from the original code is that we're moving the frame loading just a bit later, after the other scripts in the iframe.

</html>`;

const [ src, cleanup ] = useMemo( () => {
const _src = URL.createObjectURL(
Expand Down
13 changes: 13 additions & 0 deletions packages/e2e-tests/plugins/iframed-enqueue-block-assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,18 @@ static function() {
filemtime( plugin_dir_path( __FILE__ ) . 'iframed-enqueue-block-assets/style.css' )
);
wp_add_inline_style( 'iframed-enqueue-block-assets', 'body{padding:20px!important}' );
wp_enqueue_script(
'iframed-enqueue-block-assets-script',
plugin_dir_url( __FILE__ ) . 'iframed-enqueue-block-assets/script.js',
array(),
filemtime( plugin_dir_path( __FILE__ ) . 'iframed-enqueue-block-assets/script.js' )
);
wp_localize_script(
'iframed-enqueue-block-assets-script',
'iframedEnqueueBlockAssetsL10n',
array(
'test' => 'Iframed Enqueue Block Assets!',
)
);
}
);
3 changes: 3 additions & 0 deletions packages/e2e-tests/plugins/iframed-enqueue-block-assets/script.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
window.addEventListener( 'load', () => {
document.body.dataset.iframedEnqueueBlockAssetsL10n = window.iframedEnqueueBlockAssetsL10n.test;
} );
7 changes: 7 additions & 0 deletions packages/e2e-tests/specs/editor/plugins/iframed-equeue-block-assets.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe( 'iframed inline styles', () => {
} );

it( 'should load styles added through enqueue_block_assets', async () => {
await page.waitForSelector( 'iframe[name="editor-canvas"]' );
// Check stylesheet.
expect(
await getComputedStyle( canvas(), 'body', 'background-color' )
Expand All @@ -40,5 +41,11 @@ describe( 'iframed inline styles', () => {
expect( await getComputedStyle( canvas(), 'body', 'padding' ) ).toBe(
'20px'
);

expect(
await canvas().evaluate( () => ( { ...document.body.dataset } ) )
).toEqual( {
iframedEnqueueBlockAssetsL10n: 'Iframed Enqueue Block Assets!',
} );
} );
} );