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

Regression: iframed post editor can show some admin styles with classic themes #57176

Closed
sabernhardt opened this issue Dec 18, 2023 · 5 comments · Fixed by #62350
Closed

Regression: iframed post editor can show some admin styles with classic themes #57176

sabernhardt opened this issue Dec 18, 2023 · 5 comments · Fixed by #62350
Assignees
Labels
Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@sabernhardt
Copy link
Contributor

sabernhardt commented Dec 18, 2023

Description

In classic themes, styles such as the background-color and line-height from the admin common.css can apply to the body element within the iframed post editor canvas.

Step-by-step reproduction instructions

  1. Activate Twenty Nineteen or another classic theme that does not set its own background color to the body (without using the Customizer, that also includes bundled themes from Twenty Ten to Twenty Sixteen).
  2. Visit the post editor, and check that it is inside an iframe.
  3. Note that the background is a light gray (#f0f0f1) instead of white.

Screenshots, screen recording, code snippet

Twenty Nineteen is one of the themes that shows the admin's light gray background color instead of white.

post editor uses admin CSS for the body background color

In an uncommon and extreme example, a Calendar block has a line-height of 18.2 pixels—even when the font size is much bigger—in Twenty Twenty.

font size is 36 pixels and the line-height is only 18.2

Environment info

The admin CSS bleed can happen with WordPress 6.4.2, either without any plugins active or with Gutenberg 17.2.3. I also experienced it with WordPress 6.3.2, but only when Gutenberg was active (version 16.9.0).

For the screenshots, I used:

  • WordPress 6.4.2 with Gutenberg 17.2.3
  • Firefox 120.0.1 in Windows 10 Home

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

@sabernhardt sabernhardt added the [Type] Bug An existing feature does not function as intended label Dec 18, 2023
@sabernhardt
Copy link
Contributor Author

I can think of two viable fixes.

Option 1: Restore html in reset.scss

To override (at least) body from admin common.css, the :where(.editor-styles-wrapper) selector needs a higher specificity.

The simplest edit would be adding html back to the same selector from which it was removed (reverting one line).

However, if the extra element is necessary for only the iframe body, its reset styles could be nested inside the current selector:

:where(.editor-styles-wrapper) {
	html & {
		// add styles for .editor-styles-wrapper whether it is a div or the iframe body
	}
}

Option 2: Remove or replace the reset stylesheet when in an iframe

The iframed editor proposal promised "no admin CSS bleed at all," so it seems counterintuitive to add and override admin styles in that context.

Enqueuing reset.css plus its two dependencies within the iframe adds a lot of code, and the inline comment only mentions a need to reset the list styles. Also, if the comment refers to block margins in wp-editor-classic-layout-styles, those styles print later in the queue.

If some styles are necessary to override, the iframe could have its own new (smaller) reset stylesheet.

(The current reset stylesheet theoretically could remove the 'common' and 'forms' dependencies only when in an iframe context, though that method might be overcomplicated.)

@jordesign jordesign added [Package] Editor /packages/editor Needs Testing Needs further testing to be confirmed. labels Dec 18, 2023
@peXed
Copy link

peXed commented Jan 17, 2024

This is breaking older, custom themes from us as well. Would it not be the easiest way to reverting it back to use html? Was it not only removed for performance reasons?

Copy link

Hi,
This issue has gone 30 days without any activity. This means it is time for a check-in to make sure it is still relevant. If you are still experiencing this issue with the latest versions, you can help the project by responding to confirm the problem and by providing any updated reproduction steps.
Thanks for helping out.

@github-actions github-actions bot added the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Feb 17, 2024
@sabernhardt
Copy link
Contributor Author

This is still a problem in WordPress 6.5-beta1 and Gutenberg 17.7.0.

Themes such as Twenty Thirteen and Twenty Fourteen show the issue more clearly because they do not specify a font-size or background-color in the editor. They have the 13px text and #f0f0f1 light gray from admin common.css.

Twenty Thirteen
editor canvas has wrong background color and small body text

Twenty Fourteen
editor canvas has wrong background color and small body text

@github-actions github-actions bot removed the [Status] Stale Gives the original author opportunity to update before closing. Can be reopened as needed. label Feb 18, 2024
@karmatosed
Copy link
Member

I wanted to confirm that this is still the case today. As has been said, this impacts all 'classic' themes, here is a list of themes which are impacted:

  • Twenty Ten
  • Twenty Eleven
  • Twenty Twelve
  • Twenty Thirteen
  • Twenty Fourteen
  • Twenty Sixteen
  • Twenty Nineteen

This means that a significant amount of them are impacted, so my advice would be to look at a resolution or recommend an approach theme by theme if it is decided that the approach in editor is the preferred way. I suspect an approach in the editor is better than going theme by theme though.

@karmatosed karmatosed added Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. and removed Needs Testing Needs further testing to be confirmed. labels Mar 29, 2024
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 24, 2024
@sabernhardt sabernhardt removed their assignment May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Dev Ready for, and needs developer efforts Needs Technical Feedback Needs testing from a developer perspective. [Package] Editor /packages/editor [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
4 participants