Improve handling of !important transformation specificity

[westonruter]

On https://scalewp.io/development-and-workflow/ there is an element:

<header id="header" style="display: none;">

In spite of the inline style, it gets shown because the stylesheet has:

#header {
    display: block !important;
    width: 100%;
    background: #FFF;
}

Nevertheless, when it is served as AMP the header is not displayed. The inline style is currently getting converted into a style rule as follows:

:root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a {
    display: none;
}

And the !important property gets extracted into a separate rule as:

:root:not(#_) #header {
    display: block;
}

Something is wrong in the logic because currently the header is not appearing in AMP where it is appearing in non-AMP. In other words, the generated selector for the inline style :root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a is winning over an !important property in an ID-selector style rule which gets transformed as :root:not(#_) #header.

Something needs to change in the collect_inline_styles and transform_important_qualifiers methods:

https://github.com/Automattic/amp-wp/blob/6861830b80e2d2cc17306f034feef976c35f6e22/includes/sanitizers/class-amp-style-sanitizer.php#L1064-L1102

and

https://github.com/Automattic/amp-wp/blob/6861830b80e2d2cc17306f034feef976c35f6e22/includes/sanitizers/class-amp-style-sanitizer.php#L981-L1048

[westonruter]

URL where the issue can be seen: https://amp-wp-microsite.pantheonsite.io/development-and-workflow/?allow_header_display_none=true&amp

Compare with non-AMP version: https://amp-wp-microsite.pantheonsite.io/development-and-workflow/?allow_header_display_none=true

[hellofromtonya]

We are applying a base specificity multiplier of 5 to the inline styles:

$root  = ':root' . str_repeat( ':not(#_)', 5 );

We do this to ensure that inline styles take precedence over non !important style rulesets.

In order to have !important rulesets take precedence, we can apply that same base specificity multiplier + 1.

		// Compute the specificity multiplier.
		$specificity_multiplier = self::INLINE_SPECIFICITY_MULTIPLIER + 1 + floor( $old_selector->getSpecificity() / 100 );
		if ( $old_selector->getSpecificity() % 100 > 0 ) {
			$specificity_multiplier++;
		}
		if ( $old_selector->getSpecificity() % 10 > 0 ) {
			$specificity_multiplier++;
		}

		$selector_mod = str_repeat( ':not(#_)', $specificity_multiplier );
		$new_selector = $old_selector->getSelector();

		// Amend the selector mod to the first element in selector if it is already the root; otherwise add new root ancestor.
		if ( preg_match( '/^\s*(html|:root)\b/i', $new_selector, $matches ) ) {
			$new_selector = substr( $new_selector, 0, strlen( $matches[0] ) ) . $selector_mod . substr( $new_selector, strlen( $matches[0] ) );
		} else {
			$new_selector = sprintf( ':root%s %s', $selector_mod, $new_selector );
		}
		return new Selector( $new_selector );
	}

The net result is this:

:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #header {
    display: block;
}

The transformed CSS ruleset now takes precedence.

PR is coming...tomorrow.

[kienstra]

Moving To 'Ready For Merging'

If it's alright, I'm moving this to 'Ready For Merging,' as I don't think it needs functional testing. Feel free to move it back.