Fix broken value="" stripping for HTML5

The default value for value="" on <input> elements is not always an
empty string.

In particular the default value for type="radio" is "on" and by
stripping value="" out of the attributes a "" becomes "on" and our
cleanup code ends up breaking forms.

Change-Id: Ibe5a3be3f45a2f93ef95dbe42729b8f8c94a41cb

diff --git a/includes/Html.php b/includes/Html.php
index 7b79146..35aa2c0 100644 (file)
--- a/includes/Html.php
+++ b/includes/Html.php
@@ -323,7 +323,6 @@ class Html {
                        'input' => array(
                                'formaction' => 'GET',
                                'type' => 'text',
-                               'value' => '',
                        ),
                        'keygen' => array( 'keytype' => 'rsa' ),
                        'link' => array( 'media' => 'all' ),
@@ -364,6 +363,29 @@ class Html {
                && strval( $attribs['type'] ) == 'text/css' ) {
                        unset( $attribs['type'] );
                }
+               if ( $element === 'input' ) {
+                       $type = isset( $attribs['type'] ) ? $attribs['type'] : null;
+                       $value = isset( $attribs['value'] ) ? $attribs['value'] : null;
+                       if ( $type === 'checkbox' || $type === 'radio' ) {
+                               // The default value for checkboxes and radio buttons is 'on'
+                               // not ''. By stripping value="" we break radio boxes that
+                               // actually wants empty values.
+                               if ( $value === 'on' ) {
+                                       unset( $attribs['value'] );
+                               }
+                       } elseif ( $type === 'submit' ) {
+                               // The default value for submit appears to be "Submit" but
+                               // let's not bother stripping out localized text that matches
+                               // that.
+                       } else {
+                               // The default value for nearly every other field type is ''
+                               // The 'range' and 'color' types use different defaults but
+                               // stripping a value="" does not hurt them.
+                               if ( $value === '' ) {
+                                       unset( $attribs['value'] );
+                               }
+                       }
+               }
                if ( $element === 'select' && isset( $attribs['size'] ) ) {
                        if ( in_array( 'multiple', $attribs )
                                || ( isset( $attribs['multiple'] ) && $attribs['multiple'] !== false )

diff --git a/tests/phpunit/includes/HtmlTest.php b/tests/phpunit/includes/HtmlTest.php
index 02a8328..a4ddf85 100644 (file)
--- a/tests/phpunit/includes/HtmlTest.php
+++ b/tests/phpunit/includes/HtmlTest.php
@@ -403,4 +403,50 @@ class HtmlTest extends MediaWikiTestCase {
                }
                return $cases;
        }
+
+       /**
+        * Test out Html::element drops default value
+        * @cover Html::dropDefaults
+        * @dataProvider provideElementsWithAttributesHavingDefaultValues
+        */
+       function testDropDefaults( $expected, $element, $message = '' ) {
+               $this->enableHTML5();
+               $this->assertEquals( $expected, $element, $message );
+       }
+
+       function provideElementsWithAttributesHavingDefaultValues() {
+               # Use cases in a concise format:
+               # <expected>, <element name>, <array of attributes> [, <message>]
+               # Will be mapped to Html::element()
+               $cases = array();
+               $cases[] = array( '<input type="checkbox" />',
+                       'input', array( 'type' => 'checkbox', 'value' => 'on' ),
+                       'Default value "on" is stripped of checkboxes',
+               );
+               $cases[] = array( '<input type="radio" />',
+                       'input', array( 'type' => 'radio', 'value' => 'on' ),
+                       'Default value "on" is stripped of radio buttons',
+               );
+               $cases[] = array( '<input type="submit" value="Submit" />',
+                       'input', array( 'type' => 'submit', 'value' => 'Submit' ),
+                       'Default value "Submit" is kept on submit buttons (for possible l10n issues)',
+               );
+               $cases[] = array( '<input type="color" />',
+                       'input', array( 'type' => 'color', 'value' => '' ),
+               );
+               $cases[] = array( '<input type="range" />',
+                       'input', array( 'type' => 'range', 'value' => '' ),
+               );
+
+               # Craft the Html elements
+               $ret = array();
+               foreach( $cases as $case ) {
+                       $ret[] = array(
+                               $case[0],
+                               Html::element( $case[1], $case[2] )
+                       );
+               }
+               return $ret;
+       }
+
 }