Improve handling of !important transformation specificity
westonruter opened this issue · 3 comments
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:
and
URL where the issue can be seen: https://amp-wp-microsite.pantheonsite.io/development-and-workflow/?allow_header_display_none=true&
Compare with non-AMP version: https://amp-wp-microsite.pantheonsite.io/development-and-workflow/?allow_header_display_none=true
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.
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.