OOUIHTMLForm: Make boolean form field parameters actually work everywhere
authorBartosz Dziewoński <matma.rex@gmail.com>
Tue, 1 Sep 2015 13:03:07 +0000 (15:03 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Tue, 1 Sep 2015 13:16:52 +0000 (13:16 +0000)
HTMLFormField::getAttributes() is unfortunately reused both for
generating HTML tag attributes and generating OOUI widget
configuration. For boolean ones passing '' for true (empty string)
works for HTML (where the attribute only has to be present), but not
for OOUI (where the configuration option has to be truthy).

It would be cleanest to pass true/false, which is the expected input
for OOUI widgets and which the Html class handles intuitively, but it
seems that these values often end up in the Xml class's methods
instead (somebody remind me why do we even have that?). So let's play
it safe and pass the name of the parameter instead, which is okay for
both HTML/XML (both disabled="" and disabled="disabled" work the same)
and OOUI widgets.

(Note also that the whole thing relies on the default value of these
boolean parameters/attributes being false.)

This was not spotted before because we had hacks for this problem in
all the important places. This commit reverts three such hacky
patches that missed the underlying problem:

e25eb30ea81eda82b57a4e3c84f4e1afdf3b6080
70910cd13cdb5d67f1dda2d9ccffbc8e30787352
8a164ff9f9f5b4691e40f1219358ca45c8b61c3d

Change-Id: Ic6a1f3758cba62147f7fe8127cc0a83c695b0212

includes/htmlform/HTMLFormField.php
includes/htmlform/HTMLTextAreaField.php
includes/htmlform/HTMLTextField.php

index 0fab033..13756e3 100644 (file)
@@ -920,7 +920,7 @@ abstract class HTMLFormField {
         * @return array Attributes
         */
        public function getAttributes( array $list, array $mappings = null ) {
-               static $boolAttribs = array( 'disabled', 'required', 'multiple', 'readonly' );
+               static $boolAttribs = array( 'disabled', 'required', 'autofocus', 'multiple', 'readonly' );
 
                $ret = array();
                foreach ( $list as $key ) {
@@ -928,7 +928,7 @@ abstract class HTMLFormField {
 
                        if ( in_array( $key, $boolAttribs ) ) {
                                if ( !empty( $this->mParams[$key] ) ) {
-                                       $ret[$mappedKey] = '';
+                                       $ret[$mappedKey] = $mappedKey;
                                }
                        } elseif ( isset( $this->mParams[$key] ) ) {
                                $ret[$mappedKey] = $this->mParams[$key];
index 5cfea63..aeb4b7c 100644 (file)
@@ -71,11 +71,6 @@ class HTMLTextAreaField extends HTMLFormField {
                        'readonly' => 'readOnly',
                ) );
 
-               if ( isset( $attribs['readOnly'] ) ) {
-                       // this needs to be set to a boolean value - hack??
-                       $attribs['readOnly'] = true;
-               }
-
                return new OOUI\TextInputWidget( array(
                        'id' => $this->mID,
                        'name' => $this->mName,
index 40cff47..157116d 100644 (file)
@@ -113,11 +113,6 @@ class HTMLTextField extends HTMLFormField {
                        'tabindex' => 'tabIndex',
                ) );
 
-               if ( isset( $attribs['readOnly'] ) ) {
-                       // This needs to be set to a boolean value
-                       $attribs['readOnly'] = true;
-               }
-
                $type = $this->getType( $attribs );
 
                return $this->getInputWidget( array(