Fix link element hover bleeding into button element default styles

[getdave]

What?

In #42055 we discovered that link element styles were bleeding into button element styles. This PR fixes that by targeting only non-button element a nodes with the CSS. It also fixes a bug with the detection of pseudo selectors in the Theme JSON.

Fixes #42055.

Why?

The link element is mapped to a raw a CSS selector:

gutenberg/lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

Lines 33 to 43 in 895ca1f

	const ELEMENTS = array(
	'link' => 'a',
	'h1' => 'h1',
	'h2' => 'h2',
	'h3' => 'h3',
	'h4' => 'h4',
	'h5' => 'h5',
	'h6' => 'h6',
	'button' => '.wp-element-button, .wp-block-button__link', // We have the .wp-block-button__link class so that this will target older buttons that have been serialized.
	'caption' => '.wp-element-caption, .wp-block-audio figcaption, .wp-block-embed figcaption, .wp-block-gallery figcaption, .wp-block-image figcaption, .wp-block-table figcaption, .wp-block-video figcaption', // The block classes are necessary to target older content that won't use the new class names.
	);

When this config is translated into CSS rules for elements then we get following - admittedly rather broad - selector:

a {
    // rules here
}

Unfortunately however, the button block uses an a in its markup.

<div class="wp-block-button">
    <a class="wp-block-button__link wp-element-button">I am a button</a>
</div>

This means that the styles intended for link elements only, end up unintentionally "polluting" the button block styling.

How?

The suggested solution is to more carefully target anchor nodes which are not button elements. This is done by checking that the a node in question is :not(.wp-element-button).

This PR therefore updates the elements constant with a new selector mapping:

const ELEMENTS = array(
-    'link'    => 'a',
+    'link'    => 'a:not(.wp-element-button)',
    // other element mappings
);

This results in a more specific rule being generated to target global a (aka link) elements:

a:not(.wp-element-button) {
    // rules here
}

Unfortunately however, this has a side effect of exposing a bug in the way we generated pseudo class styles for elements in Theme JSON. Specifically we use naive regex (I wrote it so it's ok to say that! 😆 ) that is based on the presence of a colon : to parse out the potential pseudo class (e.g. :hover from the selector a:hover).

gutenberg/lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

Lines 465 to 468 in 895ca1f

	// Attempt to parse a pseudo selector (e.g. ":hover") from the $selector ("a:hover").
	$pseudo_matches = array();
	preg_match( '/:[a-z]+/', $selector, $pseudo_matches );
	$pseudo_selector = isset( $pseudo_matches[0] ) ? $pseudo_matches[0] : null;

The problem is that :not() is incorrectly determined to be a "interactivity state" psuedo class (e.g. :hover, :focus...etc) which causes the rules to be misapplied.

This PR updates the logic to check for the presence of an element-specific set of whitelisted pseudo classes.

// Illustrative code only - see `Files changed` for actual code implementation.
$selector = 'a:not('.wp-element-button'):hover';
$element_pseudo_whitelist = array(':hover',':focus',':active');
$pseudo_matches = array_values(
    array_filter(
        $element_pseudo_whitelist,
        function( $pseudo_selector ) use ( $selector ) {
            return str_contains( $selector, $pseudo_selector ); 
        }
    )
);

The whitelist is as per that which already exists in trunk:

gutenberg/lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

Lines 19 to 26 in 895ca1f

	/**
	* Whitelist which defines which pseudo selectors are enabled for
	* which elements.
	* Note: this will effect both top level and block level elements.
	*/
	const VALID_ELEMENT_PSEUDO_SELECTORS = array(
	'link' => array( ':hover', ':focus', ':active' ),
	);

By doing this the pseudo selectors are output correctly.

Testing Instructions

1. Switch to emtpytheme
2. Add this to your theme.json file under elements

			"link": {
				"border": {
					"color": "grey",
					"radius": "1px",
					"width": "5px",
					"style": "solid"
				},
				"color": {
					"background": "lightblue",
					"text": "red"
				},
				"spacing": {
					"padding": "10px"
				},
				"typography": {
					"fontFamily": "sans-serif",
					"fontSize": "20px",
					"textDecoration": "none"
				},
				":hover": {
					"border": {
						"color": "black",
						"radius": "10px",
						"width": "5px",
						"style": "solid"
					},
					"color": {
						"background": "red",
						"text": "black"
					}
				},
				":focus": {
					"border": {
						"color": "black",
						"radius": "10px",
						"width": "5px",
						"style": "solid"
					},
					"color": {
						"background": "red",
						"text": "black"
					}
				}
			}

3. Add some buttons blocks
4. Switch to the front of your site.
5. Check that none of the rules for the link element are applied to buttons as well as to links.
6. Check against the testing instructions in Global Styles: Elements: Link style rules are being applied to buttons #42055

Screenshots or screencast

Before

Both the link and button elements take on the styles shown in the theme.json excerpt from the Testing Instructions. This shouldn't happen. Buttons should not be targetted.

After

The link elements take on the styles shown in the theme.json excerpt from the Testing Instructions whereas the button elements ignore these styles.

[getdave]

Told you I'd never learn to spell this word

[getdave]

This is the element-specific whitelist of valid pseudo selectors. As things stand only link is allowed to have pseudo selectors and these are whitelisted to hover, focus and active.

[getdave]

One question I have is are we going to end up in an escalating arms race here of excluding elements from being "targetted" by the global a selector?

Currently it's only

a:not(.wp-element-button) {
    // rules here
}

...but I could see that escalating into the :not() containing lots of selectors.

[adamziel]

One question I have is are we going to end up in an escalating arms race here of excluding elements from being "targetted" by the global a selector?

One way out of this would be to use a denylist and exclude all a elements with a class starting with wp-. We could use a selectors such as a:not( [class^='wp-'], [class*=' wp-'] ).

At this point, I'm starting to question the assumptions though. Which elements do we even want this to target? Is a.wp-block-pages-list__item__link okay? Why or why not? Also, if I write a plugin shipping a MyButton block, how can I ensure it won't get targeted by the global link styles?

[adamziel]

I wonder if we could, instead, tell the button styles to ignore the a styles. Perhaps by setting inherit values for any a rules affecting the button but not covered by it? For example:

a {
   color: #000;
}
.wp-block-button {
   color: inherit; /* Automatically added by WP */
   border: 1px solid #fff;
}

[scruffian]

I think that this is a special case. Firstly the element selector for links is much broader than most element selectors will be, and secondly buttons using a tags is an exception which I don't expect to happen often, if at all. I'd say let's merge this one as is, and if we do need to add more selectors we can come back to it.

[getdave]

Conflicts now resolved. Waiting on tests to merge.

[ZebulanStanphill]

The selector used here doesn't work with Button blocks generated in older versions of the block editor, since they lack the wp-element-button CSS class. Worse, the new selector has higher specificity than the old one, so the underline style is even more likely to conflict with the styles being applied to the old Button blocks. And indeed, that's exactly what I ran into after updating Gutenberg on a site I'm working on.

Any ideas on how to resolve this? This feels awfully similar to the situation that #42122 is attempting to address, namely that there are a bunch of static blocks with outdated markup already on the front-end, and the only way to change their markup is by either resaving the posts they're in, or else by using dynamic rendering filters in PHP to alter the output markup.

[getdave]

@ZebulanStanphill Thank you for taking the time to report this.

I think that the direction of #42122 might be where we have to go for this. Dynamically adding the class names ensures we always have them even in older versions of Gutenberg.

The problem is that PR is still a while from shipping and then we have to write the same logic but for the buttons block.

@adamziel what's your take on this?

[adamziel]

@getdave @ZebulanStanphill I'd love to have these classes added to all the older blocks. #42485 could help us do just that.