Make WordPress Core

Opened 8 months ago

Closed 6 months ago

Last modified 6 months ago

#59086 closed defect (bug) (fixed)

Twenty Twenty: black background hides text editing after 6.3 update

Reported by: floydwilde's profile floydwilde Owned by: audrasjb's profile audrasjb
Milestone: 6.4 Priority: high
Severity: major Version: 6.3
Component: Bundled Theme Keywords: has-patch fixed-major
Focuses: css Cc:

Description (last modified by sabernhardt)

I'm using the Twenty Twenty theme and after the update using a site with black background set to hex #000000 makes it difficult to edit a post. Since the text is not visible. Not sure if a problem with my self hosted installation or an issue after the new release, but observed this issue after applying 6.3 update.

Attachments (3)

59086.diff (1.5 KB) - added by poena 8 months ago.
Add a condition that changes the body text color CSS selector depending on theme version, solve JavaScript warning in the editor.
59086-2.diff (1.9 KB) - added by poena 8 months ago.
Use enqueue_block_assets or enqueue_block_editor_assets depending on theme version
59086.3.diff (1.6 KB) - added by sabernhardt 8 months ago.
switch body to html, and use enqueue_block_assets action with 6.3

Download all attachments as: .zip

Change History (40)

#1 @poena
8 months ago

Hi!
Thank you for the report. I can confirm this issue.

Steps to reproduce and to test the patch
1) Activate Twenty-Twenty
2) Go to Appearance > Customize
3) Select the Colors panel, change the background color to black
4) Open the block editor
5) Confirm if the text color is readable.
6) Open the console in the browser developer tools and confirm if there is a JavaScript warning for "X was added to the iframe incorrectly"


The theme uses 'enqueue_block_editor_assets' to enqueue the CSS in the block editor, and this causes issues with the new iframe, and the following JavaScript warnings:

twentytwenty-block-editor-styles-css was added to the iframe incorrectly. Please use block.json or enqueue_block_assets to add styles to the iframe.
twentytwenty-block-editor-styles-inline-css was added to the iframe incorrectly. Please use block.json or enqueue_block_assets to add styles to the iframe.
Last edited 8 months ago by poena (previous) (diff)

#2 @poena
8 months ago

In twentytwenty/inc/custom-css.php, the CSS selector for the text in the editor
includes body before .editor-styles-wrapper, so the color is not applied.
Removing body here solves the text color issue in WordPress 6.3.0.

// Text color.
if ( $body && $body !== $body_default ) {
twentytwenty_generate_css( 'body .editor-styles-wrapper, .editor-post-title__block .editor-post-title__input, .editor-post-title__block .editor-post-title__input:focus', 'color', $body );

The theme supports WordPress version 4.7 and up, so I suggest that the next task is either:
-Remove the body from the selector for version 6.3.0 and up, which would be the fast way to do it;
-Or, find out which WordPress version requires the body to be included, and if no versions require it, remove it.

#3 @poena
8 months ago

  • Keywords needs-patch added

@poena
8 months ago

Add a condition that changes the body text color CSS selector depending on theme version, solve JavaScript warning in the editor.

#4 @poena
8 months ago

  • Keywords has-patch needs-testing added; needs-patch removed
  • Milestone changed from Awaiting Review to 6.3.1

#5 @poena
8 months ago

  • Keywords needs-refresh added; has-patch needs-testing removed

Adding add_action( 'enqueue_block_assets', 'twentytwenty_block_editor_styles', 1, 1 ); had unintended consequences on the front, so I am refreshing the patch.

@poena
8 months ago

Use enqueue_block_assets or enqueue_block_editor_assets depending on theme version

#6 @poena
8 months ago

twentytwenty_block_editor_styles() adds a script and CSS for both blocks and the editor canvas itself.
Because extenders may be using this function name already, I did not want to split it into multiple functions and enqueue these separately. So I added a version compare and an is_admin() conditional.

@sabernhardt @greenshady does this make sense?

/**
 * Enqueue supplemental block editor styles.
 *
 * @since Twenty Twenty 1.0
 */
function twentytwenty_block_editor_styles() {

	// Enqueue the editor styles.
	wp_enqueue_style( 'twentytwenty-block-editor-styles', get_theme_file_uri( '/assets/css/editor-style-block.css' ), array(), wp_get_theme()->get( 'Version' ), 'all' );
	wp_style_add_data( 'twentytwenty-block-editor-styles', 'rtl', 'replace' );

	// Add inline style from the Customizer.
	$customizer_css = twentytwenty_get_customizer_css( 'block-editor' );
	if ( $customizer_css ) {
		wp_add_inline_style( 'twentytwenty-block-editor-styles', $customizer_css );
	}

	// Add inline style for non-latin fonts.
	$custom_css = TwentyTwenty_Non_Latin_Languages::get_non_latin_css( 'block-editor' );
	if ( $custom_css ) {
		wp_add_inline_style( 'twentytwenty-block-editor-styles', $custom_css );
	}

	// Enqueue the editor script.
	wp_enqueue_script( 'twentytwenty-block-editor-script', get_theme_file_uri( '/assets/js/editor-script-block.js' ), array( 'wp-blocks', 'wp-dom' ), wp_get_theme()->get( 'Version' ), true );
}

if ( version_compare( $GLOBALS['wp_version'], '6.3', '>=' ) && is_admin() ) {
	add_action( 'enqueue_block_assets', 'twentytwenty_block_editor_styles', 1, 1 );
} else {
	add_action( 'enqueue_block_editor_assets', 'twentytwenty_block_editor_styles', 1, 1 );
}

Or maybe it should just be

if ( is_admin() ) {
	add_action( 'enqueue_block_assets', 'twentytwenty_block_editor_styles', 1, 1 );
}

#7 @poena
8 months ago

  • Keywords has-patch needs-testing added; needs-refresh removed

#8 @huzaifaalmesbah
8 months ago

Test Report

I test with 59086-2.diff

Environment

OS: macOS m1
Web Server: nginx/1.25.0
PHP: 8.1.21
WordPress: 6.3
Browser: Chrome 115.0.5790.98
Theme: Twenty Twenty
Active Plugins: No plugins activated.

Results

I have tested this issue without a patch and with the patch. it's working with the patch properly. I give a screenshot here:

Before Applying Patch

https://i.ibb.co/Dg36YY9/Test-Color.png

After Applying Patch

https://i.ibb.co/8MkgVmg/After-Applying.png

#9 @greenshady
8 months ago

That should work, @poena. The one thing I'm not sure we should be doing is loading the editor-script.js on enqueue_block_assets since it's specifically for the editor and not the "content".

This is untested, but this is how I'd probably approach this:

I'd use add_editor_style() for the editor-style-block.css stylesheet. Then, I'd do something like this for the inline CSS:

add_filter( 'block_editor_settings_all', 'twentytwenty_editor_settings' );

function twentytwenty_editor_settings( $settings ) {
	// Add inline style from the Customizer.
	$customizer_css = twentytwenty_get_customizer_css( 'block-editor' );
	
	// Add inline style for non-latin fonts.
	$custom_css = TwentyTwenty_Non_Latin_Languages::get_non_latin_css( 'block-editor' );

	$settings['styles'][] = [
		'css'            => $customizer_css . $custom_css,
		'__unstableType' => 'theme',
		'source'         => 'twentytwenty'
	];

	return $settings;
}

And the script should be loaded on enqueue_block_editor_assets as normal.

#10 @poena
8 months ago

This is an old function, I don't think we can replace it without breaking plugins and child themes.

#11 @poena
8 months ago

editor-script.js should not be enqueued at all above WP version 5.3 because it only unregisters a custom block style that was removed from core in 5.3: https://core.trac.wordpress.org/ticket/59090

#12 @greenshady
8 months ago

This is an old function, I don't think we can replace it without breaking plugins and child themes.

I see. In that case, the earlier solution you posted would be the path with the least potential breakage. I don't see any other good ways around it.

#13 @sabernhardt
8 months ago

  • Focuses css added
  • Summary changed from Twenty Twenty Theme black background hides text editing after 6.3 update to Twenty Twenty: black background hides text editing after 6.3 update

According to the dev note, having one registered block at version 2 could make it render outside of an iframe. Then the WordPress version would not help.

Some body selectors were added in PR 351 and then reduced to only the one in PR 890. Checking this with WordPress 5.0, that version added a text color on .editor-styles-wrapper later in the cascade. Any (parent) element should have overridden the other style just as well as body.

Changing body to :root (within the browser inspector) worked for me in both 5.0 and 6.3:

':root .editor-styles-wrapper, .editor-post-title__block .editor-post-title__input, .editor-post-title__block .editor-post-title__input:focus'

Another option to try is including both ways in the list:

'body .editor-styles-wrapper, .editor-styles-wrapper, .editor-post-title__block .editor-post-title__input, .editor-post-title__block .editor-post-title__input:focus'

(or body .editor-styles-wrapper, body.editor-styles-wrapper, ...)

Last edited 8 months ago by sabernhardt (previous) (diff)

#14 @sabernhardt
8 months ago

  • Description modified (diff)
  • Priority changed from normal to high
  • Severity changed from normal to major

Note: the text color is also wrong with other dark background colors (either preset or custom).

#15 @poena
8 months ago

According to the dev note, having one registered block at version 2 could make it render outside of an iframe. Then the WordPress version would not help.

Are you able to reproduce the issue when the iframe is not used?

Changing body to root would make sense, it still needs to be combined with a fix for the JavaScript warning.

#16 @sabernhardt
8 months ago

Searching the plugin directory, I found Safe SVG has a version 2 block. Using that plugin in 6.3 retained the non-iframe editor, which still set white text against dark background colors when editing posts.

@sabernhardt
8 months ago

switch body to html, and use enqueue_block_assets action with 6.3

#17 @sabernhardt
8 months ago

In the patch, I used html to keep the same specificity as body, and the is_admin() condition is first because that is more likely false than whether WordPress is at least 6.3.

The enqueue_block_assets action apparently works for both iframe and non-iframe editors in 6.3, so the version check should be fine for that. Unless Gutenberg removes the JS warning and continues its backward compatibility, Twenty Ten through Twenty Nineteen would need a similar condition (not as urgently as the text color fix in this ticket).

Another way to test non-iframe editing in 6.3 is by going to Preferences and showing the Custom Fields panel (that is why I did not get the iframe in my SVN installation).

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


8 months ago

#19 @audrasjb
8 months ago

  • Milestone changed from 6.3.1 to 6.3.2

WP 6.3.1 is going to be released in the next few days, so let's move this ticket to 6.3.2 to give it more time to be committed and backported.

#20 @pooja1210
7 months ago

Hi,

Test Report for - https://core.trac.wordpress.org/attachment/ticket/59086/59086.3.diff

Environment
WordPress - v6.3.1
Theme - Twenty Twenty
OS - Mac
Browser - Chrome

Verified the patch and now after applying the patch the text is appearing properly. But there is one console error visible - https://prnt.sc/sCTAJ987M-0R

Thanks!

#21 @shailu25
7 months ago

Test Report

Patch Tested: https://core.trac.wordpress.org/attachment/ticket/59086/59086.3.diff

Environment:

WordPress - 6.3.1
OS - Windows
Browser - Chrome
Theme: Twenty Twenty
PHP - 8.0.18
Active Plugin - None

Steps to Reproduce:

1) Activate Twenty-Twenty
2) Go to Appearance > Customize
3) Select the Colors panel, change the background color to black
4) Open the block editor
5) 🐞 Bug Occur: Text color is not readable.

Expected Results:

✅ After the patch it should work

Actual Results:

✅ Patch is working fine & Text is Readable in Block Editor.

Screenshots:

Before Patch: https://prnt.sc/3uMvUcA_Fg63
After Patch : https://prnt.sc/pRzmRw6Twgjt

For Console Error : We are tracking this issue in https://core.trac.wordpress.org/ticket/59090

#22 @sabernhardt
7 months ago

#59090 was marked as a duplicate.

This ticket was mentioned in PR #5135 on WordPress/wordpress-develop by @sabernhardt.


7 months ago
#23

  • Uses html instead of body for the first CSS selector that makes text white against a dark background
  • Moves twentytwenty_block_editor_styles() from the enqueue_block_editor_assets action to enqueue_block_assets for WordPress 6.3 and later
  • Removes the obsolete twentytwenty-block-editor-script from the styles function to avoid an error in the iframe

Trac 59086

#24 @sabernhardt
7 months ago

Instead of enqueuing the editor script conditionally, as suggested on #59090, I simply removed it from the styles function. Issue 573 identified the Squared style as unnecessary and potentially confusing because it is the same as the Default style. However, experiencing that confusion would require multiple conditions:

  1. updating the site to use an upcoming version of Twenty Twenty;
  2. keeping the WordPress version at 5.0, 5.1 or 5.2; and
  3. creating or editing a Button block link, checking the sidebar, and expanding the Styles settings to find the extra option there.
Last edited 7 months ago by sabernhardt (previous) (diff)

#25 @huzaifaalmesbah
7 months ago

  • Keywords needs-refresh added

#26 @sabernhardt
7 months ago

  • Keywords needs-refresh removed

PR refreshed following [56526]

This ticket was mentioned in Slack in #core by poena. View the logs.


7 months ago

#28 @poena
7 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/5135

Environment

  • OS: macOS 13.6
  • Web Server: nginx
  • PHP: 7.4.30
  • WordPress: 6.4-alpha-56267-src
  • Browser: Chrome Version 117.0.5938.92
  • Theme: Twenty Twenty
  • Active Plugins: none

Actual Results

  • ✅ Issue resolved with patch.

This ticket was mentioned in Slack in #core by joemcgill. View the logs.


7 months ago

#30 @joemcgill
7 months ago

  • Keywords commit added; needs-testing removed
  • Status changed from new to assigned

Thanks for refreshing this @sabernhardt and testing @poena. Going to mark this for commit and see if we can get it in for 6.3.2.

#31 @audrasjb
6 months ago

  • Owner set to audrasjb
  • Status changed from assigned to accepted

Self assigning for commit.

#32 @audrasjb
6 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 56783:

Twenty Twenty: Fix style issues within iframed editor.

This changeset:

  • Replaces body with html for the first CSS selector that makes text white against a dark background
  • Moves twentytwenty_block_editor_styles() from the enqueue_block_editor_assets action to enqueue_block_assets for WordPress 6.3 and later
  • Removes the obsolete twentytwenty-block-editor-script from the styles function to avoid an error in the iframe

Props floydwilde, poena, huzaifaalmesbah, greenshady, sabernhardt, audrasjb, pooja1210, shailu25, joemcgill.
Fixes #59086.

#33 @audrasjb
6 months ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


6 months ago

#35 @audrasjb
6 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 56784:

Twenty Twenty: Fix style issues within iframed editor.

This changeset:

  • Replaces body with html for the first CSS selector that makes text white against a dark background
  • Moves twentytwenty_block_editor_styles() from the enqueue_block_editor_assets action to enqueue_block_assets for WordPress 6.3 and later
  • Removes the obsolete twentytwenty-block-editor-script from the styles function to avoid an error in the iframe

Props floydwilde, poena, huzaifaalmesbah, greenshady, sabernhardt, audrasjb, pooja1210, shailu25, joemcgill.
Merges [56783] to the 6.3 branch.
Fixes #59086.

#37 @sabernhardt
6 months ago

  • Keywords commit removed
  • Milestone changed from 6.3.2 to 6.4

This can be coordinated with WordPress 6.4 in two more weeks.

Note: See TracTickets for help on using tickets.