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.

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

Site editor iframe clips body styling #44374

Closed
colorful-tones opened this issue Sep 22, 2022 · 14 comments
Closed

Site editor iframe clips body styling #44374

colorful-tones opened this issue Sep 22, 2022 · 14 comments
Assignees
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@colorful-tones
Copy link
Member

colorful-tones commented Sep 22, 2022

Description

Testing in 6.1 beta1 with Twenty Twenty-Three theme. This bug only surfaces in the latest beta and if you activate the latest Gutenberg then it goes away. So, did something not get ported into the latest 6.1 beta? Or should I move this over to Trac?

This affects the Twenty Twenty-Three theme: WordPress/twentytwentythree#195

The <body> element styling is clipped for the following Style Variations: Pilgrimage, Sherbet, Whisper.

Pilgrimage (body background gradient cut off in iframe)
Screen Shot 2022-09-22 at 8 55 28 AM

Sherbet (body background gradient cut off in iframe)
Screen Shot 2022-09-22 at 8 56 20 AM

Whisper (body border cut off in iframe)
Screen Shot 2022-09-22 at 8 56 34 AM

This bug has been confirmed in Chrome 105, Safari 16.0, and Firefox 105

This seems to be caused by the wp-admin/load-styles.php add in a <div style="display: none;">
Screen Shot 2022-09-22 at 10 33 56 AM

Step-by-step reproduction instructions

Running WordPress Beta Tester plugin and WordPress 6.1 beta1 with newly added Twenty Twenty-Three:

  1. Open the Appearance > Editor (Beta)
  2. Choose one of the following style variations in global styles: Pilgrimage, Sherbet, Whisper
  3. Scroll down and see that either background is clipped (Pilgrimage, Sherbet) or border is clipped (Whisper)

Furthermore, if you now enable the latest Gutenberg plugin then the issue goes away.

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 6.1 beta1 (see notes above about Gutenberg enabled/disabled)
  • Twenty Twenty-Three 1.0
  • macOS 12.6
  • Chrome 105, Safari 16.0, and Firefox 105

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@ndiego ndiego added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Priority] High Used to indicate top priority items that need quick attention labels Sep 22, 2022
@ndiego
Copy link
Member

ndiego commented Sep 22, 2022

@ockham @michalczaplinski @annezazu I am able to confirm this. Note this issue does not occur when the latest version of Gutenberg is active. But as soon as you deactivate GB, the issue appears. Perhaps something did not get ported over to 6.1 🤔 So far I have been unable to isolate the issue.

@annezazu
Copy link
Contributor

Noting that this issue feels similar to #30768 so, when this is closed, I want to confirm it can be too.

@colorful-tones
Copy link
Member Author

Noting that this issue feels similar to #30768 so, when this is closed, I want to confirm it can be too.

@annezazu this is definitely a duplicate of 30768. However, I feel like I've maybe captured a bit more detail here. 🤷 I'm glad you pointed #30768 out though. ❤️

@colorful-tones
Copy link
Member Author

@annezazu I also suspect that this has the potential to get booted to Trac since truly the issue lies in WP core, and with Gutenberg active the bug is resolved.

@ndiego
Copy link
Member

ndiego commented Sep 23, 2022

@annezazu thanks for sharing, this is interesting because clearly, this has been an issue for a while. In the other issue, you had Gutenberg active and the issue existed. Today, if you activate Gutenberg, it goes away. I am wondering if this got fixed awhile ago, but never got ported to Core. I assumed there was a recent change in Gutenberg, but perhaps not 🤔

@annezazu
Copy link
Contributor

Oh definitely game to keep this open -- I wasn't trying to say "duplicate please close" by any means. More so trying to figure out what's going on, add more pieces to the puzzle, and make sure both issues get closed when completed. As @ndiego notes above, it seems like something has evolved since it was first reported and, perhaps, we can find the fix as a result.

@t-hamano
Copy link
Contributor

t-hamano commented Sep 25, 2022

I would like to explain this issue as best I can. (However, I may be wrong on some points.)

First, I believe the root cause of this problem is the application of height:100% to the html and body elements, and #30768 is the same problem. Turning off this style in the browser console solves the problem:

capture

And this CSS is came from common.css in the WordPress core:
https://github.com/WordPress/wordpress-develop/blob/8f3254d794ca4e8b8e0ec8254e5886be840a60c6/src/wp-admin/css/common.css#L212

My understanding is that the styles containing common.css are loaded dynamically by load-styles.php. This is also loaded in the iframe editor instance, which is why this problem occurs.

However, if you enable SCRIPT_DEBUG, this file will be loaded as a separate link element. Furthermore, common.css as a link element is not loaded in the iframe editor instance. Therefore, we can confirm that enabling SCRIPT_DEBUG solves this problem even in WordPress 6.1 Beta1 without the gutenberg plugin.

Also, if the gutenberg plugin is enabled, somehow load-styles.php is not loaded into the iframe editor instance, so this problem does not occur.

To solve this problem, we need to override the styles of the html and body elements for when load-styles.php (i.e. common.css) is loaded in the iframe editor. For example, use height:auto to override the html and body element styles in the iframe editor component.

Perhaps changes need to be made in this section:

<head ref={ headRef }>
{ head }
<style>
{ `html { transition: background 5s; ${
isZoomedOut
? 'background: #2f2f2f; transition: background 0s;'
: ''
} }` }
</style>
</head>
<body

@youknowriad
I noticed that you implemented the zoom-out view in #41156 and made changes to this file.

Is there a way to properly backport this issue? If zoomed-out views will not be included in WordPress 6.1, I feel that simple backporting might be difficult.

@youknowriad
Copy link
Contributor

I think we need to identify what solved the issue exactly as otherwise we'd be back porting things that could potentially introduce new bugs. Potentially we could do a targeted fix for this branch only, maybe adding a minHeight: 100% to the iframe html element or something.

@t-hamano
Copy link
Contributor

Thank you for the advice, @youknowriad !

I would like to explain what I have learned from further research in three cases.

The first important point is that only CSS files containing .editor-styles-wrapper or .wp-block will be copied to the iframe editor instance.

return (
selectorText &&
( selectorText.includes(
`.${ BODY_CLASS_NAME }`
) ||
selectorText.includes( `.${ BLOCK_PREFIX }` ) )
);


WordPress without Gutenberg plugin and SCRIPT_DEBUG is false

load-styles.php loads all stylesheets at once. Since this stylesheet contains .editor-styles-wrapper and .wp-block selectors, it is copied to the iframe editor instance. This stylesheet also contains common.css, which applies height:100% to html / body.

Most WordPress users will be based in this environment, which is why this issue causes.

WordPress with / without Gutenberg plugin and SCRIPT_DEBUG is true

load-styles.php is not loaded and all stylesheets are output individually.
common.css is not copied to the iframe editor instance because it does not have .editor-styles-wrapper or .wp-block selectors. Therefore, this problem does not occur.

WordPress with Gutenberg plugin and SCRIPT_DEBUG is false

Stylesheets related to Gutenberg are output separately, and load-styles.php bundles only the WordPress core styles. The stylesheets generated by load-styles.php don't include .editor-styles-wrapper and .wp-block selectors, so they are not copied to the iframe editor instance. Therefore, this issue does not occur.


As mentioned above, this problem cannot be reproduced if the gutenberg plugin is enabled.

I believe that applying a style such as min-height:100% to the iframe editor instance would solve this problem, but I'm not familiar with cherry-picking, so it would be great if someone else could handle this issue 🙏

@talldan
Copy link
Contributor

talldan commented Oct 17, 2022

Is there a way to properly backport this issue? If zoomed-out views will not be included in WordPress 6.1, I feel that simple backporting might be difficult.

If it can't be done in one PR, I think it should be ok to make two PRs. One targeting the wp/6.1 branch, and another targeting trunk. The one targeting trunk could have the backport label, but then let the release leads know that rather than cherry-picking they can merge the first PR and not have to solve conflicts.

@t-hamano
Copy link
Contributor

Thank you for the advice, @talldan !

I have confirmed that this problem is still reproduced in RC2. I will create two PRs as soon as possible in time for RC3.

@t-hamano t-hamano self-assigned this Oct 19, 2022
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Oct 19, 2022
@t-hamano
Copy link
Contributor

I first created a PR #45118 to backport to WP6.1. I would appreciate if someone could review it.

@t-hamano
Copy link
Contributor

The PR for this issue is below:

@ockham
Copy link
Contributor

ockham commented Oct 25, 2022

Closing, now that both #45118 and #45261 have been merged. Thanks, everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

No branches or pull requests

8 participants