Make WordPress Core

Opened 6 weeks ago

Closed 6 weeks ago

#66044 closed defect (bug) (fixed)

spacing.blockGap in a block style variation stops emitting any gap

Reported by: ramonopoly Owned by: ramonopoly
Priority: normal Milestone: 7.1.1
Component: Themes Version: 7.1
Severity: normal Keywords: has-patch has-unit-tests dev-reviewed commit
Cc: Focuses:

Description

This ticket tracks the backporting of the following Gutenberg PR: ​https://github.com/WordPress/gutenberg/pull/82335

A block style variation that declares spacing.blockGap produced gap CSS in WordPress 7.0 and produces none in 7.1. Nothing else in the variation is affected — css, colours and the rest still emit. There is no error or warning; the gap silently falls back to whatever the theme or Core sets globally.

​https://github.com/WordPress/gutenberg/pull/82335 fixes it by passing to get_layout_styles() the block the variation belongs to, so the support check answers the question it is actually asking.

Props to @kimjiwoon

Change History (15)

This ticket was mentioned in ​PR #13398 on ​WordPress/wordpress-develop by ​@kimjiwoon.


6 weeks ago
#1

  • Keywords has-patch has-unit-tests added

Backport of ​https://github.com/WordPress/gutenberg/pull/82335

## Problem

A block style variation that declares spacing.blockGap emits no layout CSS at all in 7.1 — not only the gap, but the flow, constrained, flex and grid rules together. 7.0 emitted them. There is no error; the spacing silently falls back to whatever the theme or Core sets globally.

get_block_nodes() stores the variation slug as the node's name. That array reaches get_layout_styles() as $block_metadata, where name is read as a block name and looked up in the block type registry to check for layout support:

if ( isset( $block_metadata['name'] ) ) {
        $block_type = WP_Block_Type_Registry::get_instance()->get_registered( $block_metadata['name'] );
        if ( ! block_has_support( $block_type, array( 'layout' ), false ) && ! block_has_support( $block_type, array( '__experimentalLayout' ), false ) ) {
                return $block_rules;
        }
}

A variation slug is never a registered block, so the check fails and the method returns before generating anything.

## Fix

The metadata passed to get_layout_styles() now names the block the variation belongs to, at both call sites: the base spacing.blockGap, and the one inside a viewport breakpoint, which takes its own copy of the metadata.

The variation node keeps its own slug, so feature-selector processing is unaffected, and the layout support check is answered against a real block rather than skipped. get_layout_styles() itself is untouched — its guard is correct for real block nodes, and giving it the owning block name is what lets it do its job: a core/paragraph variation declaring blockGap now emits nothing, where dropping the key entirely would produce rules for layout classes that block never carries.

## Testing Instructions

npm run test:php -- --filter test_block_style_variation

Three tests: the base gap, the responsive gap inside its media query, and the layout support check. The first two fail without the change.

Manually, in a block theme add styles/blocks/buttons-connected.json:

{
	"version": 3,
	"title": "Connected",
	"slug": "connected",
	"blockTypes": [ "core/buttons" ],
	"styles": {
		"spacing": { "blockGap": "var:preset|spacing|25" }
	}
}

Add a Buttons block with two buttons, apply the Connected style, and inspect the container on the front end. Expected: a gap rule for the variation. Before the change: none.

This ticket was mentioned in ​Slack in #core-editor by kimjiwoon. ​View the logs.


6 weeks ago

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


6 weeks ago

This ticket was mentioned in ​PR #13419 on ​WordPress/wordpress-develop by ​@andraganescu.


6 weeks ago
#4

Package update ticket: https://core.trac.wordpress.org/ticket/66037

## What

Backport of #13398 to the 7.1 branch, for WordPress 7.1.1.

This is the core patch accompanying the Gutenberg package update for 7.1.1 (WordPress/gutenberg#82383), which cherry-picks WordPress/gutenberg#82335 into wp/7.1. The Gutenberg PR registered this backport in backport-changelog/7.1/13398.md.

The other PHP change shipping in that package update (WordPress/gutenberg#81908, block-supports/states.php) is already on the 7.1 branch via changeset 63474, so this is the only remaining core patch.

## Why

WP_Theme_JSON::get_styles_for_block() passes a block style variation node to get_layout_styles(), which reads the metadata name as a block name to check layout support. A variation node's name is the variation slug, which is never a registered block, so the check fails and every variation blockGap rule is discarded. The fix passes the block the variation belongs to.

## Testing

  • The diff is applied verbatim from #13398 (currently open against trunk); it applied cleanly on 7.1 and both files pass php -l.
  • Adds unit tests: test_block_style_variation_with_block_gap_emits_layout_styles, test_block_style_variation_with_responsive_block_gap_emits_layout_styles, test_block_style_variation_block_gap_respects_layout_support in tests/phpunit/tests/theme/wpThemeJson.php.

Props to @Jiwoon-Kim, the author of the original fix (kept as the git author of the commit).

🤖 Generated with ​Claude Code

​@andraganescu commented on ​PR #13419:


6 weeks ago
#5

Oh, we don't need this because ​https://github.com/WordPress/gutenberg/issues/13398 will be backported to 7.1.1 by committer according to handbook.

​@ramonopoly commented on ​PR #13419:


6 weeks ago
#6

Thanks for backporting to 7.1!
I'll go ahead and commit ​https://github.com/WordPress/wordpress-develop/pull/13398 to trunk then.

#7 @ramonopoly
6 weeks ago

  • Owner set to ramonopoly
  • Resolution → fixed
  • Status new → closed

In 63523:

Themes: restore layout styles for block style variations with a block gap

This commit passes a style variation's "block" to get_layout_styles() so the block support check passes.

It fixes a bug where a block style variation that declared spacing.blockGap silently produced no CSS.

The cause was due to running the layout block support check against a variation node, which is not a registered block and cannot be used to check for layout support.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13398

Props kimjiwoon, ramonopoly.

Fixes #66044.

#8 @adamsilverstein
6 weeks ago

  • Keywords dev-feedback added
  • Resolution fixed
  • Status closed → reopened

Reopening to consider backporting https://core.trac.wordpress.org/changeset/63523 for the 7.1 branch.

​@andraganescu commented on ​PR #13419:


6 weeks ago
#9

In https://core.trac.wordpress.org/ticket/66044#comment:8 the core patch will be backported, closing again.

#10 @adamsilverstein
6 weeks ago

  • Keywords dev-reviewed commit added; dev-feedback removed

@ramonopoly - I reviewed the PR and the changes look good for backporting to me. I will add dev-reviewed as my explicit 2nd sign off.

One small suggestion that isn't a blocker: I wasn't able to test this manually, I only reviewed the code and trust this fixes a genuine 7.1 problem. It would be nice if the PR or trac ticket included clear steps for manual reproduction - or even a Playground blueprint. I typically would try to reproduce an issue manually and observe the code change actually fixes the issue, maybe go back and check the issue didn't exist in 7.0, etc. I'm not sure how to do that exactly with this ticket.

#11 @kimjiwoon
6 weeks ago

@adamsilverstein Thanks, that is a good suggestion. Here is a minimal manual reproduction that makes the 7.0 / 7.1 / fixed comparison explicit.

Setup

  1. Start a clean installation with any block theme active.
  2. In the active theme, create styles/blocks/group-gap-test.json:
{
  "version": 3,
  "title": "Gap test",
  "slug": "gap-test",
  "blockTypes": [ "core/group" ],
  "styles": {
    "spacing": { "blockGap": "3em" }
  }
}
  1. Create a post containing a Group block. Change its layout to Row, add two Paragraph blocks, then select the Group and choose the Gap test block style. Publish and view the post on the front end.
  2. In DevTools, inspect the Group. It should have both is-style-gap-test and wp-block-group-is-layout-flex. Search the page's generated global-styles CSS for is-style-gap-test.

Expected comparison

  • 7.0: the generated stylesheet contains a rule equivalent to :root :where(.wp-block-group.is-style-gap-test.wp-block-group-is-layout-flex){gap: 3em;} , and the Row has a visibly 3em gap.
  • 7.1.0: the Group still receives is-style-gap-test, but no matching layout rule is generated; the Row falls back to its normal gap.
  • trunk / r63523: the same rule and 3em gap return.

For the patched build, the PR's standard Playground artifact is available here: ​Test PR #13398 with WordPress Playground. Repeat steps 2-4 there. This isolates the regression to generated layout CSS: the variation class is present in all three cases; only 7.1.0 drops the layout rule.

I agree this is the kind of concise manual path and Playground entry point that should accompany similar Core tickets going forward.

#12 @adamsilverstein
6 weeks ago

Thanks @kimjiwoon - thats perfect, appreciate the instructions.

#13 @ramonopoly
6 weeks ago

Thanks for helping move this forward folks! I didn't realize we needed double sign off for trunk (I committed), or was it just for the point release?

Should we close this ticket or wait until it's in the 7.1 branch?

#14 @adamsilverstein
6 weeks ago

The double signoff is only required for the backport (or for any commits during RC).

#15 @ramonopoly
6 weeks ago

  • Resolution → fixed
  • Status reopened → closed

In 63547:

Themes: restore layout styles for block style variations with a block gap

This commit passes a style variation's "block" to get_layout_styles() so the block support check passes.

It fixes a bug where a block style variation that declared spacing.blockGap silently produced no CSS.

The cause was due to running the layout block support check against a variation node, which is not a registered block and cannot be used to check for layout support.

Developed in: ​https://github.com/WordPress/wordpress-develop/pull/13398

Reviewed by adamsilverstein.

Merges [63523] to the 7.1 branch.

Props kimjiwoon, ramonopoly.

Fixes #66044.

Note: See TracTickets for help on using tickets.