From: Timo Tijhof <ttijhof@wikimedia.org>
Date: Thu, 1 Nov 2012 18:08:54 +0000 (+0100)
Subject: (bug 40632) Remove CleanupPresentationalAttributes feature
X-Git-Tag: 1.31.0-rc.0~21572^2
X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/journal.php?a=commitdiff_plain;h=5f41c8ba875d6617f512fb236b50627453eed2f7;p=lhc%2Fweb%2Fwiklou.git

(bug 40632) Remove CleanupPresentationalAttributes feature

Removed $wgCleanupPresentationalAttributes, the associated
code it toggles and references to those in src and tests.

Also fixes bug 40329.

This was originally introduced in r94465 (released in REL1_19) but
disabled by default. Then enabled in r98053, after which several
bugs were filed and eventually the decision was made to remove
this feature.

Removed obsolete release-note entry, as this is to be backported
to REL1_20.

Change-Id: I4e86305520a3b22ef88381caab55d24abac932e3
---

diff --git a/RELEASE-NOTES-1.20 b/RELEASE-NOTES-1.20
index 1a9296842a..ea8ed6d327 100644
--- a/RELEASE-NOTES-1.20
+++ b/RELEASE-NOTES-1.20
@@ -230,8 +230,6 @@ milestone in Bugzilla.
 * (bug 36812) Special:ActiveUsers "Hide bots" should hide users from any group
   having the "bot" user right, instead of just the default "bot" user group.
 * (bug 35082) mw.util.addPortletLink incorrectly adds link to mutiple <ul> tags.
-* (bug 36495) Sanitizer::fixDeprecatedAttributes should convert "align"
-  attribute to margin or float instead of text-align (for non-table-cells).
 * (bug 36991) jquery.tablesorter should extract date sort format from date
   string instead of global config. Dates like "April 1 2012" and "1 April 2012"
   now sort correctly regardless of the content language's DefaultDateFormat.
@@ -284,6 +282,7 @@ milestone in Bugzilla.
 * (bug 40500) ResourceLoader should not ignore media-type for urls in debug mode.
 * (bug 40660) ResourceLoaderWikiModule should not convert "&nbsp;" to a space
   for pages from the MediaWiki-namespace.
+* (bug 40329) (bug 40632) Removed CleanupPresentationalAttributes feature.
 
 === API changes in 1.20 ===
 * (bug 34316) Add ability to retrieve maximum upload size from MediaWiki API.
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 9621984bcb..60c3ab8754 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -2535,11 +2535,6 @@ $wgAllowRdfaAttributes = false;
  */
 $wgAllowMicrodataAttributes = false;
 
-/**
- * Cleanup as much presentational html like valign -> css vertical-align as we can
- */
-$wgCleanupPresentationalAttributes = true;
-
 /**
  * Should we try to make our HTML output well-formed XML?  If set to false,
  * output will be a few bytes shorter, and the HTML will arguably be more
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index 7bb3d939e4..0034afefae 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -638,122 +638,15 @@ class Sanitizer {
 	}
 
 	/**
-	 * Take an array of attribute names and values and fix some deprecated values
-	 * for the given element type.
-	 * This does not validate properties, so you should ensure that you call
-	 * validateTagAttributes AFTER this to ensure that the resulting style rule
-	 * this may add is safe.
-	 *
-	 * - Converts most presentational attributes like align into inline css
-	 *
-	 * @param $attribs Array
-	 * @param $element String
-	 * @return Array
-	 */
-	static function fixDeprecatedAttributes( $attribs, $element ) {
-		global $wgHtml5, $wgCleanupPresentationalAttributes;
-
-		// presentational attributes were removed from html5, we can leave them
-		// in when html5 is turned off
-		if ( !$wgHtml5 || !$wgCleanupPresentationalAttributes ) {
-			return $attribs;
-		}
-
-		$table = array( 'table' );
-		$cells = array( 'td', 'th' );
-		$colls = array( 'col', 'colgroup' );
-		$tblocks = array( 'tbody', 'tfoot', 'thead' );
-		$h = array( 'h1', 'h2', 'h3', 'h4', 'h5', 'h6' );
-
-		$presentationalAttribs = array(
-			'align' => array( 'text-align', array_merge( array( 'caption', 'hr', 'div', 'p', 'tr' ), $table, $cells, $colls, $tblocks, $h ) ),
-			'clear' => array( 'clear', array( 'br' ) ),
-			'height' => array( 'height', $cells ),
-			'nowrap' => array( 'white-space', $cells ),
-			'size' => array( 'height', array( 'hr' ) ),
-			'type' => array( 'list-style-type', array( 'li', 'ol', 'ul' ) ),
-			'valign' => array( 'vertical-align', array_merge( $cells, $colls, $tblocks ) ),
-			'width' => array( 'width', array_merge( array( 'hr', 'pre' ), $table, $cells, $colls ) ),
-		);
-
-		// Ensure that any upper case or mixed case attributes are converted to lowercase
-		foreach ( $attribs as $attribute => $value ) {
-			if ( $attribute !== strtolower( $attribute ) && array_key_exists( strtolower( $attribute ), $presentationalAttribs ) ) {
-				$attribs[strtolower( $attribute )] = $value;
-				unset( $attribs[$attribute] );
-			}
-		}
-
-		$style = "";
-		foreach ( $presentationalAttribs as $attribute => $info ) {
-			list( $property, $elements ) = $info;
-
-			// Skip if this attribute is not relevant to this element
-			if ( !in_array( $element, $elements ) ) {
-				continue;
-			}
-
-			// Skip if the attribute is not used
-			if ( !array_key_exists( $attribute, $attribs ) ) {
-				continue;
-			}
-
-			$value = $attribs[$attribute];
-
-			// For nowrap the value should be nowrap instead of whatever text is in the value
-			if ( $attribute === 'nowrap' ) {
-				$value = 'nowrap';
-			}
-
-			// clear="all" is clear: both; in css
-			if ( $attribute === 'clear' && strtolower( $value ) === 'all' ) {
-				$value = 'both';
-			}
-
-			// Size based properties should have px applied to them if they have no unit
-			if ( in_array( $attribute, array( 'height', 'width', 'size' ) ) ) {
-				if ( preg_match( '/^[\d.]+$/', $value ) ) {
-					$value = "{$value}px";
-				}
-			}
-
-			// Table align is special, it's about block alignment instead of
-			// content align (see also bug 40306)
-			if ( $attribute === 'align' && in_array( $element, $table ) ) {
-				if ( $value === 'center' ) {
-					$style .= ' margin-left: auto;';
-					$property = 'margin-right';
-					$value = 'auto';
-				} else {
-					$property = 'float';
-				}
-			}
-
-			$style .= " $property: $value;";
-
-			unset( $attribs[$attribute] );
-		}
-
-		if ( $style ) {
-			// Prepend our style rules so that they can be overridden by user css
-			if ( isset($attribs['style']) ) {
-				$style .= " " . $attribs['style'];
-			}
-			$attribs['style'] = trim($style);
-		}
-
-		return $attribs;
-	}
-
-	/**
-	 * Takes attribute names and values for a tag and the tah name and
+	 * Takes attribute names and values for a tag and the tag name and
 	 * validates that the tag is allowed to be present.
 	 * This DOES NOT validate the attributes, nor does it validate the
 	 * tags themselves. This method only handles the special circumstances
 	 * where we may want to allow a tag within content but ONLY when it has
 	 * specific attributes set.
 	 *
-	 * @param $
+	 * @param $params
+	 * @param $element
 	 */
 	static function validateTag( $params, $element ) {
 		$params = Sanitizer::decodeTagAttributes( $params );
@@ -1024,7 +917,6 @@ class Sanitizer {
 		}
 
 		$decoded = Sanitizer::decodeTagAttributes( $text );
-		$decoded = Sanitizer::fixDeprecatedAttributes( $decoded, $element );
 		$stripped = Sanitizer::validateTagAttributes( $decoded, $element );
 
 		$attribs = array();
diff --git a/tests/parser/parserTest.inc b/tests/parser/parserTest.inc
index b9f1817ac6..ea1b29016d 100644
--- a/tests/parser/parserTest.inc
+++ b/tests/parser/parserTest.inc
@@ -692,7 +692,6 @@ class ParserTest {
 			'wgExternalLinkTarget' => false,
 			'wgAlwaysUseTidy' => false,
 			'wgHtml5' => true,
-			'wgCleanupPresentationalAttributes' => true,
 			'wgWellFormedXml' => true,
 			'wgAllowMicrodataAttributes' => true,
 			'wgAdaptiveMessageCache' => true,
diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index ac3113e9bb..d25f822f34 100644
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -11497,22 +11497,6 @@ Bug 31098 Template which includes system messages which includes the template
 </p>
 !! end
 
-!! test
-Deprecated presentational attributes are converted to css
-!! input
-{|
-| valign=top align=left width=100 height=25% | Asdf
-|}
-<ul type="disc"></ul>
-!! result
-<table>
-<tr>
-<td style="text-align: left; height: 25%; vertical-align: top; width: 100px;"> Asdf
-</td></tr></table>
-<ul style="list-style-type: disc;"></ul>
-
-!! end
-
 !! test
 Bug31490 Turkish: ucfirst 'blah'
 !! options
diff --git a/tests/phpunit/includes/SanitizerTest.php b/tests/phpunit/includes/SanitizerTest.php
index 6e618e392d..b6786473aa 100644
--- a/tests/phpunit/includes/SanitizerTest.php
+++ b/tests/phpunit/includes/SanitizerTest.php
@@ -5,8 +5,6 @@ class SanitizerTest extends MediaWikiTestCase {
 	protected function setUp() {
 		parent::setUp();
 
-		$this->setMwGlobals( 'wgCleanupPresentationalAttributes', true );
-
 		AutoLoader::loadClass( 'Sanitizer' );
 	}
 
@@ -157,51 +155,24 @@ class SanitizerTest extends MediaWikiTestCase {
 	/**
 	 * @dataProvider provideDeprecatedAttributes
 	 */
-	function testDeprecatedAttributes( $input, $tag, $expected, $message = null ) {
-		$this->assertEquals( $expected, Sanitizer::fixTagAttributes( $input, $tag ), $message );
-	}
-
-	function testDeprecatedAttributesDisabled() {
-		global $wgCleanupPresentationalAttributes;
-
-		$wgCleanupPresentationalAttributes = false;
+	function testDeprecatedAttributesUnaltered( $inputAttr, $inputEl ) {
 
-		$this->assertEquals( ' clear="left"', Sanitizer::fixTagAttributes( 'clear="left"', 'br' ), 'Deprecated attributes are not converted to styles when enabled.' );
+		$this->assertEquals( " $inputAttr", Sanitizer::fixTagAttributes( $inputAttr, $inputEl ) );
 	}
 
 	public static function provideDeprecatedAttributes() {
 		return array(
-			array( 'clear="left"', 'br', ' style="clear: left;"', 'Deprecated attributes are converted to styles when enabled.' ),
-			array( 'clear="all"', 'br', ' style="clear: both;"', 'clear=all is converted to clear: both; not clear: all;' ),
-			array( 'CLEAR="ALL"', 'br', ' style="clear: both;"', 'clear=ALL is not treated differently from clear=all' ),
-			array( 'width="100"', 'td', ' style="width: 100px;"', 'Numeric sizes use pixels instead of numbers.' ),
-			array( 'width="100%"', 'td', ' style="width: 100%;"', 'Units are allowed in sizes.' ),
-			array( 'WIDTH="100%"', 'td', ' style="width: 100%;"', 'Uppercase WIDTH is treated as lowercase width.' ),
-			array( 'WiDTh="100%"', 'td', ' style="width: 100%;"', 'Mixed case does not break WiDTh.' ),
-			array( 'nowrap="true"', 'td', ' style="white-space: nowrap;"', 'nowrap attribute is output as white-space: nowrap; not something else.' ),
-			array( 'nowrap=""', 'td', ' style="white-space: nowrap;"', 'nowrap="" is considered true, not false' ),
-			array( 'NOWRAP="true"', 'td', ' style="white-space: nowrap;"', 'nowrap attribute works when uppercase.' ),
-			array( 'NoWrAp="true"', 'td', ' style="white-space: nowrap;"', 'nowrap attribute works when mixed-case.' ),
-			array( 'align="right"', 'td', ' style="text-align: right;"'   , 'align on table cells gets converted to text-align' ),
-			array( 'align="center"', 'td', ' style="text-align: center;"' , 'align on table cells gets converted to text-align' ),
-			array( 'align="left"'  , 'div', ' style="text-align: left;"'  , 'align=(left|right) on div elements gets converted to text-align' ),
-			array( 'align="center"', 'div', ' style="text-align: center;"', 'align="center" on div elements gets converted to text-align' ),
-			array( 'align="left"'  , 'p',   ' style="text-align: left;"'  , 'align on p elements gets converted to text-align' ),
-			array( 'align="left"'  , 'h1',  ' style="text-align: left;"'  , 'align on h1 elements gets converted to text-align' ),
-			array( 'align="left"'  , 'h1',  ' style="text-align: left;"'  , 'align on h1 elements gets converted to text-align' ),
-			array( 'align="left"'  , 'caption',' style="text-align: left;"','align on caption elements gets converted to text-align' ),
-			array( 'align="left"'  , 'tfoot',' style="text-align: left;"' , 'align on tfoot elements gets converted to text-align' ),
-			array( 'align="left"'  , 'tbody',' style="text-align: left;"' , 'align on tbody elements gets converted to text-align' ),
-
-			# <tr>
-			array( 'align="right"' , 'tr', ' style="text-align: right;"' , 'align on table row get converted to text-align' ),
-			array( 'align="center"', 'tr', ' style="text-align: center;"', 'align on table row get converted to text-align' ),
-			array( 'align="left"'  , 'tr', ' style="text-align: left;"'  , 'align on table row get converted to text-align' ),
-
-			#table
-			array( 'align="left"'  , 'table', ' style="float: left;"'    , 'align on table converted to float' ),
-			array( 'align="center"', 'table', ' style="margin-left: auto; margin-right: auto;"', 'align center on table converted to margins' ),
-			array( 'align="right"' , 'table', ' style="float: right;"'   , 'align on table converted to float' ),
+			array( 'clear="left"', 'br' ),
+			array( 'clear="all"', 'br' ),
+			array( 'width="100"', 'td' ),
+			array( 'nowrap="true"', 'td' ),
+			array( 'nowrap=""', 'td' ),
+			array( 'align="right"', 'td' ),
+			array( 'align="center"', 'table' ),
+			array( 'align="left"', 'tr' ),
+			array( 'align="center"', 'div' ),
+			array( 'align="left"', 'h1' ),
+			array( 'align="left"', 'span' ),
 		);
 	}
 
diff --git a/tests/phpunit/includes/parser/NewParserTest.php b/tests/phpunit/includes/parser/NewParserTest.php
index 401962fba8..3476ce39ee 100644
--- a/tests/phpunit/includes/parser/NewParserTest.php
+++ b/tests/phpunit/includes/parser/NewParserTest.php
@@ -329,7 +329,6 @@ class NewParserTest extends MediaWikiTestCase {
 			'wgExternalLinkTarget' => false,
 			'wgAlwaysUseTidy' => false,
 			'wgHtml5' => true,
-			'wgCleanupPresentationalAttributes' => true,
 			'wgWellFormedXml' => true,
 			'wgAllowMicrodataAttributes' => true,
 			'wgAdaptiveMessageCache' => true,