Skip to content

Commit

Permalink
Merge pull request #1320 from Automattic/add/improve-important-specif…
Browse files Browse the repository at this point in the history 
…icity

Improve handling of !important transformation specificity
  • Loading branch information
@westonruter
westonruter authored Aug 5, 2018
2 parents 3485b27 + 549972d commit 0ebc97f
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 16 deletions.
19 changes: 13 additions & 6 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
*/
const TREE_SHAKING_ERROR_CODE = 'removed_unused_css_rules';

/**
* Inline style selector's specificity multiplier, i.e. used to generate the number of ':not(#_)' placeholders.
*
* @var int
*/
const INLINE_SPECIFICITY_MULTIPLIER = 5; // @todo The correctness of using "5" should be validated.

/**
* Array of flags used to control sanitization.
*
Expand Down Expand Up @@ -1591,15 +1598,15 @@ private function transform_important_qualifiers( RuleSet $ruleset, CSSList $css_
* @return Selector The new more-specific selector.
*/
function( Selector $old_selector ) {
$specific = ':not(#_)'; // Here "_" is just a short single-char ID.

$selector_mod = str_repeat( $specific, floor( $old_selector->getSpecificity() / 100 ) );
// Calculate the specificity multiplier for the placeholder.
$specificity_multiplier = AMP_Style_Sanitizer::INLINE_SPECIFICITY_MULTIPLIER + 1 + floor( $old_selector->getSpecificity() / 100 );
if ( $old_selector->getSpecificity() % 100 > 0 ) {
$selector_mod .= $specific;
$specificity_multiplier++;
}
if ( $old_selector->getSpecificity() % 10 > 0 ) {
$selector_mod .= $specific;
$specificity_multiplier++;
}
$selector_mod = str_repeat( ':not(#_)', $specificity_multiplier ); // Here "_" is just a short single-char ID.

$new_selector = $old_selector->getSelector();

Expand Down Expand Up @@ -1646,7 +1653,7 @@ private function collect_inline_styles( $element ) {
}

$class = 'amp-wp-' . substr( md5( $style_attribute->nodeValue ), 0, 7 );
$root = ':root' . str_repeat( ':not(#_)', 5 ); // @todo The correctness of using "5" should be validated.
$root = ':root' . str_repeat( ':not(#_)', self::INLINE_SPECIFICITY_MULTIPLIER );
$rule = sprintf( '%s .%s { %s }', $root, $class, $style_attribute->nodeValue );

$this->set_current_node( $element ); // And sources when needing to be located.
Expand Down
29 changes: 19 additions & 10 deletions tests/test-amp-style-sanitizer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,23 +61,32 @@ public function get_body_style_attribute_data() {
'<span style="padding:1px; margin: 2px !important; outline: 3px;">!important is converted.</span>',
'<span class="amp-wp-6a75598">!important is converted.</span>',
array(
':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-6a75598{padding:1px;outline:3px}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-6a75598{margin:2px}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-6a75598{padding:1px;outline:3px}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-6a75598{margin:2px}',
),
),

'!important_with_spaces_also_converted' => array(
'<span style="color: red ! important;">!important is converted.</span>',
'<span class="amp-wp-952600b">!important is converted.</span>',
array(
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-952600b{color:red}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-952600b{color:red}',
),
),

'!important_multiple_is_converted' => array(
'<span style="color: red !important; background: blue!important;">!important is converted.</span>',
'<span class="amp-wp-1e2bfaa">!important is converted.</span>',
array(
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-1e2bfaa{color:red;background:blue}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-1e2bfaa{color:red;background:blue}',
),
),

'!important_takes_precedence_over_inline' => array(
'<header id="header" style="display: none;"><h1>This is the header.</h1></header><style>#header { display: block !important;width: 100%;background: #fff; }',
'<header id="header" class="amp-wp-224b51a"><h1>This is the header.</h1></header>',
array(
'#header{width:100%;background:#fff}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #header{display:block}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-224b51a{display:none}',
),
),

Expand All @@ -102,7 +111,7 @@ public function get_body_style_attribute_data() {
'<style>div > span { font-weight:bold !important; font-style: italic; } @media screen and ( max-width: 640px ) { div > span { font-weight:normal !important; font-style: normal; } }</style><div><span>bold!</span></div>',
'<div><span>bold!</span></div>',
array(
'div > span{font-style:italic}:root:not(#_):not(#_) div > span{font-weight:bold}@media screen and ( max-width: 640px ){div > span{font-style:normal}:root:not(#_):not(#_) div > span{font-weight:normal}}',
'div > span{font-style:italic}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) div > span{font-weight:bold}@media screen and ( max-width: 640px ){div > span{font-style:normal}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) div > span{font-weight:normal}}',
),
),

Expand Down Expand Up @@ -146,9 +155,9 @@ public function get_body_style_attribute_data() {
'<style>#child {color:red !important} #parent #child {color:pink !important} .foo { color:blue !important; } #me .foo { color: green !important; }</style><div id="parent"><span id="child" class="foo bar baz">one</span><span style="color: yellow;">two</span><span style="color: purple !important;">three</span></div>',
'<div id="parent"><span id="child" class="foo bar baz">one</span><span class="amp-wp-64b4fd4">two</span><span class="amp-wp-ab79d9e">three</span></div>',
array(
':root:not(#_) #child{color:red}:root:not(#_):not(#_) #parent #child{color:pink}:root:not(#_) .foo{color:blue}:root:not(#_):not(#_) #me .foo{color:green}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #child{color:red}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #parent #child{color:pink}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .foo{color:blue}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) #me .foo{color:green}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-64b4fd4{color:yellow}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-ab79d9e{color:purple}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-ab79d9e{color:purple}',
),
),

Expand Down Expand Up @@ -243,9 +252,9 @@ public function get_link_and_style_test_data() {
'multiple_amp_custom_and_other_styles' => array(
'<html amp><head><meta charset="utf-8"><style amp-custom>b {color:red !important}</style><style amp-custom>i {color:blue}</style><style type="text/css">u {color:green; text-decoration: underline !important}</style></head><body><style>s {color:yellow} /* So !important! */</style><b>1</b><i>i</i><u>u</u><s>s</s></body></html>',
array(
':root:not(#_):not(#_) b{color:red}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) b{color:red}',
'i{color:blue}',
'u{color:green}:root:not(#_):not(#_) u{text-decoration:underline}',
'u{color:green}:root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) u{text-decoration:underline}',
's{color:yellow}',
),
array(),
Expand All @@ -259,7 +268,7 @@ public function get_link_and_style_test_data() {
'strong.before-dashicon',
'.dashicons-dashboard:before',
'strong.after-dashicon',
':root:not(#_):not(#_) s{color:yellow}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_) s{color:yellow}',
),
array(),
),
Expand Down Expand Up @@ -287,7 +296,7 @@ public function get_link_and_style_test_data() {
'style_on_root_element' => array(
'<html amp style="color:red;"><head><meta charset="utf-8"><style amp-custom>html { background-color: blue !important; }</style></head><body>Hi</body></html>',
array(
'html:not(#_):not(#_){background-color:blue}',
'html:not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_):not(#_){background-color:blue}',
':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-10b06ba{color:red}',
),
array(),
Expand Down

0 comments on commit 0ebc97f

Please sign in to comment.