Revert changes to convert width to max-width automatically

Reverts PR #495 for Issue #494.
ampproject · Apr 9, 2018 · 1d9f9b8 · 1d9f9b8
1 parent c0f7550
commit 1d9f9b8
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 37 deletions.
diff --git 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 <https://www.ampproject.org/docs/design/responsive/style_pages#disallowed-styles>.
 					'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 <https://github.com/Automattic/amp-wp/issues/494>.
-		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
@@ -49,14 +49,6 @@ public function get_body_style_attribute_data() {
 				),
 			),
 
-			'width_to_max-width' => array(
-				'<figure style="width: 300px"></figure>',
-				'<figure class="amp-wp-343bce0"></figure>',
-				array(
-					':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-343bce0{max-width:300px;}',
-				),
-			),
-
 			'span_display_none' => array(
 				'<span style="display: none;">Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.</span>',
 				'<span class="amp-wp-224b51a">Kses-banned properties are allowed since Kses will have already applied if user does not have unfiltered_html.</span>',
@@ -161,15 +153,15 @@ public function get_body_style_attribute_data() {
 				'<table><colgroup><col width="253"/></colgroup></table>',
 				'<table><colgroup><col class="amp-wp-cbcb5c2"></colgroup></table>',
 				array(
-					':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cbcb5c2{max-width:253px;}',
+					':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cbcb5c2{width:253px;}',
 				),
 			),
 
 			'col_with_percent_width_attribute' => array(
 				'<table><colgroup><col width="50%"/></colgroup></table>',
 				'<table><colgroup><col class="amp-wp-cd7753e"></colgroup></table>',
 				array(
-					':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cd7753e{max-width:50%;}',
+					':root:not(#_):not(#_):not(#_):not(#_):not(#_) .amp-wp-cd7753e{width:50%;}',
 				),
 			),