Make WordPress Core

Opened 10 months ago

Closed 8 months ago

Last modified 8 months ago

#58811 closed defect (bug) (fixed)

Gutenberg Bug Nested WP Element Styles Broken

Reported by: timdix's profile timdix Owned by: rajinsharwar's profile rajinsharwar
Milestone: 6.4 Priority: normal
Severity: major Version: 6.2
Component: Editor Keywords: has-patch has-unit-tests commit
Focuses: css Cc:

Description

I'm noticing some very weird behavior in the latest version of Gutenberg. It appears that the logic for inline styles is broken. Here's a video showing, 6.2.2, and an unmodified twenty twenty three theme

https://www.youtube.com/watch?v=YRpz5xJfHZw

It seems to me that WordPress is unable to output the styles in the correct order, causing the wrong styles to override. WordPress is combining multiple styles when the color is the same, which causes specific styles on descendant elements to be hoisted to a higher level.

Change History (26)

#2 @costdev
10 months ago

  • Milestone changed from Awaiting Review to 6.3
  • Version set to trunk

Hi @timdix, thanks for reporting this bug and for posting an issue on the Gutenberg repository!

I'm also seeing this in 6.3-beta4. As this bug has been merged to WordPress trunk, I'm milestoning this ticket for 6.3 for visibility and tracking the merge from Gutenberg when it's ready.

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


10 months ago

#4 @timdix
10 months ago

This might not be an issue in the Gutenberg repo, but in WordPress core:

https://github.com/WordPress/gutenberg/issues/52646#issuecomment-1636200731

#5 @costdev
10 months ago

@timdix The mentioned file still appears to be maintained in the Gutenberg repository. See here.

Version 0, edited 10 months ago by costdev (next)

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


10 months ago

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


10 months ago

#8 @audrasjb
10 months ago

  • Keywords needs-patch added

Just wanted to confirm that this file can be patched on Core side (see the file history on Github/Trac).

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


10 months ago
#9

  • Keywords has-patch added; needs-patch removed

Setting the default optimization option to false.

Trac ticket: https://core.trac.wordpress.org/ticket/58811

#10 @rajinsharwar
10 months ago

Passing the default optimization option to be false, rather than to be true as stated in the comment in code itself.

@audrasjb commented on PR #4902:


10 months ago
#11

PHP Unit tests are failing, I think they need to be modified accordingly.

#12 @audrasjb
10 months ago

Since PHP Unit tests are failing and still need to be updated, I'm inclined to punt this ticket to 6.4.

Pinging @isabel_brison as committer and Editor Tech Lead: what do you think?

#13 @ramonopoly
10 months ago

Given that this appears to have been around for a while it might be okay to punt to 6.4.

It would also give the plugin time to address it in a holistic way so that we don't have to disable optimization as a work around.

Optimization groups selectors for common style declarations and can potentially save many bytes in output.

I'll create a corresponding issue on the Gutenberg repo.

cc @aristath and @andrewserong, who also have knowledge in this area

@ramonopoly commented on PR #4902:


10 months ago
#14

Thanks for the patch.

There's a corresponding Gutenberg issue:

I think it should be addressed there unless there's urgency in Core.

@ramonopoly commented on PR #4902:


10 months ago
#15

Thanks for this patch!

Here's an equivalent Gutenberg PR with tests:

https://github.com/WordPress/gutenberg/pull/53085

#16 @rajinsharwar
10 months ago

  • Keywords has-unit-tests added

Modified Unit tests added, and now unit tests are succeeding: https://github.com/WordPress/wordpress-develop/pull/4902

#17 @rajinsharwar
10 months ago

Improved annotations as per the suggestion of @ramonopoly and @mukesh27

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


10 months ago

@SergeyBiryukov commented on PR #4902:


10 months ago
#19

Thanks for the PR! If optimize is no longer enabled by default, then a few other DocBlocks will have to be updated as well, see r55820 for all the instances that should be changed to Default false.

@rajinsharwar commented on PR #4902:


10 months ago
#20

Thanks for letting me know @SergeyBiryukov. Just updated the other DocBlocks.

#21 @JeffPaul
10 months ago

Glad to see progress here, but I concur with @audrasjb that this is a solid candidate to punt unless someone wants to more formally own this and see it through to commit in the very near future?

#22 @rajinsharwar
10 months ago

Updated the Doc bLock as per the suggestion of @audrasjb.

#23 @audrasjb
10 months ago

  • Milestone changed from 6.3 to 6.4
  • Version changed from trunk to 6.2

We are too close to the release date to commit this in the 6.3 cycle, thus let's move it to milestone 6.4.

#24 @rajinsharwar
9 months ago

  • Keywords commit added
  • Owner set to rajinsharwar
  • Status changed from new to assigned

Taking the ownership of the ticket, and marking it for commit.

#25 @isabel_brison
8 months ago

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

In 56574:

Editor: disable default style engine optimisation.

Stops style engine from combining CSS selectors by default so that rule order is preserved.

Props ramonopoly, rajinsharwar, timdix, costdev, audrasjb, SergeyBiryukov, JeffPaul, mukesh27.
Fixes #58811.

@isabel_brison commented on PR #4902:


8 months ago
#26

Committed in r56574.

Note: See TracTickets for help on using tickets.