Batching gap styles is causing Block Spacing settings to fail

[ndiego]

Description

It looks like since the introduction of #42452, block spacing settings are applied incorrectly when batched.

In looking at the original code, the order is important.

if ( $gap_value && ! $should_skip_gap_serialization ) {
	array_push(
		$layout_styles,
		array(
			'selector'     => "$selector > *",
			'declarations' => array(
				'margin-block-start' => '0',
				'margin-block-end'   => '0',
			),
		),
		array(
			'selector'     => "$selector > * + *",
			'declarations' => array(
				'margin-block-start' => $gap_value,
				'margin-block-end'   => '0',
			),
		)
	);
}

However, when multiple containers with custom block spacing get batched, the batched $selector > * styles can get placed after the $selector > * + * styles.

Step-by-step reproduction instructions

1. Using trunk or Gutenberg Nightly, add a Columns block with a single Column block.
2. Add two Paragraph blocks and then Group the Paragraph blocks.
3. Set the Block Spacing on the Column block to 40px.
4. On the Group block inside of the Column block, set the Block Spacing 20px.
5. Everyone looks ok in Editor, but on the Frontend, the styles are rendered incorrect. See the screenshot below, the batched batched $selector > * styles are rendering after the $selector > * + * styles which gives them higher priority.

Screenshots, screen recording, code snippet

Environment info

• WordPress 6.0.1
• Gutenberg Nightly/Trunk

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]

@aristath @ramonjd @andrewserong sorry for the ping, but I know you all have been working on the style engine. 🙏

[ramonjd]

Thanks for reporting, @ndiego ! I'll add it to the list of TODOs

[ramonjd]

For this individual case, updating the CSS rule registration to this would work:

array_push(
	$layout_styles,
	array(
		'selector'     => "$selector > *,$selector > * + *",
		'declarations' => array(
			'margin-block-end' => '0',
		),
	),
	array(
		'selector'     => "$selector > * + *",
		'declarations' => array(
			'margin-block-start' => $gap_value,
		),
	)
);

I have a PR up for this

• Layout: merge CSS rule for block gap #43052

However, this PR doesn't address the root cause, for which I don't know if there should be a "fix"¹ the style engine combines selectors and prints them, not in the order in which the styles are registered, but at the bottom of the stylesheet.

@aristath will probably have a neat trick up his sleeve 😄, but I'd guess that it'd be hard if not impossible to respect the order of individual selectors when combining rules.

Maybe in this instance we accept it as a "feature" and document the behaviour. After that it should be up to the consumer to:

1. Ensure their selectors are specific enough to withstand a reorder and/or
2. Determine themselves whether they'd like the style engine to "optimize" rules. For example, we could ensure that an "optimize" flag is passed to the style engine at render time here.

However, the loud, little paranoid person inside me is saying that we might encounter more of this type of thing, and maybe we should be beefing up our test cases before we unleash the optimization feature onto the world in 6.1.

In anticipation that the discussion moves in that direction I've thrown up a PR to temporarily disable optimization:

• Style engine: disable stylesheet optimization temporarily #43051

Footnotes

1. Unless we calculate specificity https://www.w3.org/TR/selectors/#specificity 😱 Seems like an overkill ↩

[ramonjd]

Adding a thought from @andrewserong in relation to any optimization/combination we perform on the backend:

I think that’s another consideration when it comes to the CSS output / optimisation too. Should we have a comparable JS version, so that we don’t introduce weird inconsistencies between the editor and site frontend?

It makes sense if we want to ensure that the editor and frontend are 1:1.

I guess it depends. If our backend code has good coverage, then maybe not everything. 🤷

It would also mean we'd have to port PHP code to JS, and maintain both. But we have to mostly do that anyway for the editor and frontend designs to be (close to) the same.

[andrewserong]

Thanks @ramonjd — I don't have a strong opinion on it, but was curious what folks think. From my perspective, I think it's more important for the site frontend's CSS to be optimised for performance rather than what's happening in the editor, where the optimisation itself could slow things down slightly while making adjustments.

The main thought I had was whether or not it's important to try to ensure we don't have wildly different implementations where it's difficult to get the same practical styling results in the editor vs site frontend, since sometimes those bugs can be difficult to diagnose.

It would also mean we'd have to port PHP code to JS, and maintain both.

I think that's probably the bigger concern — if there's no real performance benefit to implementing a JS version, then trying to do so might just be creating more work that isn't needed. 🤔

[aristath]

I think that’s another consideration when it comes to the CSS output / optimisation too. Should we have a comparable JS version, so that we don’t introduce weird inconsistencies between the editor and site frontend?

That should not be necessary... The CSS optimizations we do on the frontend should be minimally invasive and shouldn't cause any issues. If they do, then it's because the CSS we write is extremely fragile and the CSS itself should be fixed!

[aristath]

$selector > * has the same specificity as $selector > * + *, so perhaps it should be $selector.$selector > * + *? 🤔

[aristath]

$selector > * has the same specificity as $selector > * + *, so perhaps it should be $selector.$selector > * + *? 🤔

tested and that fixes the issue 👍