Skip to content
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

Jump to bottom

Fix crash when switching to code editor #61173

Closed
wants to merge 5 commits into from
Closed

Fix crash when switching to code editor #61173

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9d29a2e
Fix crash when switching to code editor
ajlende Apr 11, 2024
f029d2d
Fix typo
ajlende Apr 11, 2024
a8c290a
Remove unused eslint ignore
ajlende Apr 11, 2024
d2b797f
Add isZoomedOut variable
ajlende Apr 11, 2024
3c3f0db
Remove load callback that isn't needed
ajlende Apr 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
42 changes: 17 additions & 25 deletions packages/block-editor/src/components/iframe/index.js
View file
Open in desktop
Expand Up @@ -130,9 +130,6 @@ function Iframe( {
] = useResizeObserver();

const setRef = useRefEffect( ( node ) => {
node._load = () => {
setIframeDocument( node.contentDocument );
};
Comment on lines -133 to -135
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ellatrix I'm curious why this extra _load() function was added? It seems like it should be doing the same thing as onLoad() below.

Copy link
Member

Choose a reason for hiding this comment

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

This is not the same as document load which only happens after the initial DOM is loaded and assets are loaded. The load function is called before the browser moves on to parse any other styles and script and the body tag below. It's needed because we're rendering our own body element, which (if I remember correctly) must be available to any scripts loaded (instead of the one we're removing). See #52405.

let iFrameDocument;
// Prevent the default browser action for files dropped outside of dropzones.
function preventFileDropDefault( event ) {
Expand All @@ -145,6 +142,7 @@ function Iframe( {

documentElement.classList.add( 'block-editor-iframe__html' );

setIframeDocument( node.contentDocument );
clearerRef( documentElement );

// Ideally ALL classes that are added through get_body_class should
Expand Down Expand Up @@ -194,7 +192,6 @@ function Iframe( {
node.addEventListener( 'load', onLoad );

return () => {
delete node._load;
node.removeEventListener( 'load', onLoad );
iFrameDocument?.removeEventListener(
'dragover',
Expand Down Expand Up @@ -241,7 +238,6 @@ function Iframe( {
<html>
<head>
<meta charset="utf-8">
<script>window.frameElement._load()</script>
<style>
html{
height: auto !important;
Expand Down Expand Up @@ -277,48 +273,41 @@ function Iframe( {
? scale( contentWidth, contentHeight )
: scale;

const isZoomedOut = scale !== 1;

useEffect( () => {
if ( ! iframeDocument ) {
return;
}

if ( scale !== 1 ) {
if ( isZoomedOut ) {
// Hack to get proper margins when scaling the iframe document.
const bottomFrameSize = frameSize - contentHeight * ( 1 - scale );

iframeDocument.body.classList.add( 'is-zoomed-out' );

iframeDocument.documentElement.style.transform = `scale( ${ scale } )`;
iframeDocument.documentElement.style.marginTop = `${ frameSize }px`;
// TODO: `marginBottom` doesn't work in Firefox. We need another way to do this.
iframeDocument.documentElement.style.marginBottom = `${ bottomFrameSize }px`;
if ( iframeWindowInnerHeight > contentHeight * scale ) {
iframeDocument.body.style.minHeight = `${ Math.floor(
( iframeWindowInnerHeight - 2 * frameSize ) / scale
) }px`;
}

return () => {
iframeDocument.body.classList.remove( 'is-zoomed-out' );
iframeDocument.documentElement.style.transform = '';
iframeDocument.documentElement.style.marginTop = '';
iframeDocument.documentElement.style.marginBottom = '';
iframeDocument.body.style.minHeight = '';
};
}
}, [
scale,
frameSize,
iframeDocument,
contentHeight,
iframeWindowInnerHeight,
contentWidth,
] );
}, [ scale, frameSize, iframeDocument, contentHeight, isZoomedOut ] );

// Make sure to not render the before and after focusable div elements in view
// mode. They're only needed to capture focus in edit mode.
const shouldRenderFocusCaptureElements = tabIndex >= 0 && ! isPreviewMode;

const scaleMinHeight =
isZoomedOut && iframeWindowInnerHeight > contentHeight * scale
? `${ Math.floor(
( iframeWindowInnerHeight - 2 * frameSize ) / scale
) }px`
: undefined;

return (
<>
{ shouldRenderFocusCaptureElements && before }
Expand Down Expand Up @@ -363,16 +352,19 @@ function Iframe( {
>
{ iframeDocument &&
createPortal(
// We want to prevent React events from bubbling throught the iframe
// We want to prevent React events from bubbling through the iframe
// we bubble these manually.
/* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */
<body
ref={ bodyRef }
className={ classnames(
isZoomedOut && 'is-zoomed-out',
'block-editor-iframe__body',
'editor-styles-wrapper',
...bodyClasses
) }
style={ {
minHeight: scaleMinHeight,
} }
>
{ contentResizeListener }
<StyleProvider document={ iframeDocument }>
Expand Down