(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 1a92968..ea8ed6d 100644 (file)
--- 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 9621984..60c3ab8 100644 (file)
--- 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 7bb3d93..0034afe 100644 (file)
--- 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 b9f1817..ea1b290 100644 (file)
--- 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 ac3113e..d25f822 100644 (file)
--- 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 6e618e3..b678647 100644 (file)
--- 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 401962f..3476ce3 100644 (file)
--- 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,