From 1d9f9b8c7f2aebd9a270b9094de122e2f46ccffd Mon Sep 17 00:00:00 2001 From: Alberto Medina Date: Mon, 9 Apr 2018 15:46:09 -0700 Subject: [PATCH] Revert changes to convert width to max-width automatically Reverts PR #495 for Issue #494. --- .../sanitizers/class-amp-style-sanitizer.php | 42 +++++++------------ tests/test-amp-style-sanitizer.php | 12 +----- 2 files changed, 17 insertions(+), 37 deletions(-) diff --git a/includes/sanitizers/class-amp-style-sanitizer.php b/includes/sanitizers/class-amp-style-sanitizer.php index 93198929a84..d7a780fca05 100644 --- a/includes/sanitizers/class-amp-style-sanitizer.php +++ b/includes/sanitizers/class-amp-style-sanitizer.php @@ -261,10 +261,12 @@ public function sanitize() { // If 'width' attribute is present for 'col' tag, convert to proper CSS rule. foreach ( $this->dom->getElementsByTagName( 'col' ) as $col ) { $width_attr = $col->getAttribute( 'width' ); - if ( ! empty( $width_attr ) && ! strpos( $width_attr, '*' ) ) { - strpos( $width_attr, '%' ) - ? $col->setAttribute( 'style', 'width: ' . $width_attr ) - : $col->setAttribute( 'style', 'width: ' . $width_attr . 'px' ); + if ( ! empty( $width_attr ) && ( false === strpos( $width_attr, '*' ) ) ) { + $width_style = 'width: ' . $width_attr; + if ( is_numeric( $width_attr ) ) { + $width_style .= 'px'; + } + $col->setAttribute( 'style', $width_style ); $col->removeAttribute( 'width' ); } } @@ -470,7 +472,6 @@ private function process_link_element( DOMElement $element ) { * @type bool $class_selector_tree_shaking Whether to perform tree shaking to delete rules that reference class names not extant in the current document. * @type string[] $property_whitelist Exclusively-allowed properties. * @type string[] $property_blacklist Disallowed properties. - * @type bool $convert_width_to_max_width Convert width to max-width. * @type string $stylesheet_url Original URL for stylesheet when originating via link (or @import?). * @type string $stylesheet_path Original filesystem path for stylesheet when originating via link (or @import?). * @type array $allowed_at_rules Allowed @-rules. @@ -481,7 +482,7 @@ private function process_link_element( DOMElement $element ) { private function process_stylesheet( $stylesheet, $node, $options = array() ) { $cache_impacting_options = wp_array_slice_assoc( $options, - array( 'property_whitelist', 'property_blacklist', 'convert_width_to_max_width', 'stylesheet_url', 'allowed_at_rules' ) + array( 'property_whitelist', 'property_blacklist', 'stylesheet_url', 'allowed_at_rules' ) ); $cache_key = md5( $stylesheet . serialize( $cache_impacting_options ) ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.serialize_serialize @@ -530,17 +531,16 @@ private function parse_stylesheet( $stylesheet_string, $options = array() ) { $options = array_merge( array( - 'allowed_at_rules' => array(), - 'convert_width_to_max_width' => false, - 'property_blacklist' => array( + 'allowed_at_rules' => array(), + 'property_blacklist' => array( // See . 'behavior', '-moz-binding', ), - 'property_whitelist' => array(), - 'validate_keyframes' => false, - 'stylesheet_url' => null, - 'stylesheet_path' => null, + 'property_whitelist' => array(), + 'validate_keyframes' => false, + 'stylesheet_url' => null, + 'stylesheet_path' => null, ), $options ); @@ -800,17 +800,6 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l $this->transform_important_qualifiers( $ruleset, $css_list ) ); - // Convert width to max-width when requested. See . - if ( $options['convert_width_to_max_width'] ) { - $properties = $ruleset->getRules( 'width' ); - foreach ( $properties as $property ) { - $width_property = new Rule( 'max-width' ); - $width_property->setValue( $property->getValue() ); - $ruleset->removeRule( $property ); - $ruleset->addRule( $width_property, $property ); - } - } - // Remove the ruleset if it is now empty. if ( 0 === count( $ruleset->getRules() ) ) { $css_list->remove( $ruleset ); @@ -1083,9 +1072,8 @@ private function collect_inline_styles( $element ) { $rule = sprintf( '%s .%s { %s }', $root, $class, $style_attribute->nodeValue ); $stylesheet = $this->process_stylesheet( $rule, $style_attribute, array( - 'convert_width_to_max_width' => true, - 'allowed_at_rules' => array(), - 'property_whitelist' => $this->style_custom_cdata_spec['css_spec']['allowed_declarations'], + 'allowed_at_rules' => array(), + 'property_whitelist' => $this->style_custom_cdata_spec['css_spec']['allowed_declarations'], ) ); if ( empty( $stylesheet ) ) { diff --git a/tests/test-amp-style-sanitizer.php b/tests/test-amp-style-sanitizer.php index 756a4a39393..b0ad11899bf 100644 --- a/tests/test-amp-style-sanitizer.php +++ b/tests/test-amp-style-sanitizer.php @@ -49,14 +49,6 @@ public function get_body_style_attribute_data() { ), ), - 'width_to_max-width' => array( - '
', - '
', - array( - ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-343bce0{max-width:300px;}', - ), - ), - 'span_display_none' => array( 'Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.', 'Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.', @@ -161,7 +153,7 @@ public function get_body_style_attribute_data() { '
', '
', array( - ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cbcb5c2{max-width:253px;}', + ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cbcb5c2{width:253px;}', ), ), @@ -169,7 +161,7 @@ public function get_body_style_attribute_data() { '
', '
', array( - ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cd7753e{max-width:50%;}', + ':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cd7753e{width:50%;}', ), ),