Reduce code duplication for parsing messages into dropdown menus

The same behavior was implemented by Xml::listDropDown(),
Article::confirmDelete() and HTMLFormField::getOptions().
A new function Xml::listDropDownOptions() is added and
all three are changed to use it.

Additionally:
* Xml::listDropDown() now uses XmlSelect internally to generate the
  dropdown HTML.
* Xml::listDropDownOptionsOoui() is introduced to handle converting
  the existing format to the OOUI format, which was previously
  duplicated in Article::confirmDelete() and HTMLFormField::getOptionsOOUI().
* This change allows HTMLForm 'select' fields (HTMLSelectField) to
  support nested options (optgroups) in OOUI mode. This is a
  prerequisite for T117781.

Bug: T117781
Change-Id: I0a088f61eb32ec59677113583c7ecdcbc3fd2af0

diff --git a/includes/Xml.php b/includes/Xml.php
index 16a5a9d..0091513 100644 (file)
--- a/includes/Xml.php
+++ b/includes/Xml.php
@@ -493,7 +493,8 @@ class Xml {
        }
 
        /**
-        * Build a drop-down box from a textual list.
+        * Build a drop-down box from a textual list. This is a wrapper
+        * for Xml::listDropDownOptions() plus the XmlSelect class.
         *
         * @param string $name Name and id for the drop-down
         * @param string $list Correctly formatted text (newline delimited) to be
@@ -507,60 +508,91 @@ class Xml {
        public static function listDropDown( $name = '', $list = '', $other = '',
                $selected = '', $class = '', $tabindex = null
        ) {
-               $optgroup = false;
+               $options = self::listDropDownOptions( $list, [ 'other' => $other ] );
+
+               $xmlSelect = new XmlSelect( $name, $name, $selected );
+               $xmlSelect->addOptions( $options );
+
+               if ( $class ) {
+                       $xmlSelect->setAttribute( 'class', $class );
+               }
+               if ( $tabindex ) {
+                       $xmlSelect->setAttribute( 'tabindex', $tabindex );
+               }
 
-               $options = self::option( $other, 'other', $selected === 'other' );
+               return $xmlSelect->getHTML();
+       }
 
+       /**
+        * Build options for a drop-down box from a textual list.
+        *
+        * The result of this function can be passed to XmlSelect::addOptions()
+        * (to render a plain `<select>` dropdown box) or to Xml::listDropDownOptionsOoui()
+        * and then OOUI\DropdownInputWidget() (to render a pretty one).
+        *
+        * @param string $list Correctly formatted text (newline delimited) to be
+        *   used to generate the options.
+        * @param array $params Extra parameters
+        * @param string $params['other'] If set, add an option with this as text and a value of 'other'
+        * @return array Array keys are textual labels, values are internal values
+        */
+       public static function listDropDownOptions( $list, $params = [] ) {
+               $options = [];
+
+               if ( isset( $params['other'] ) ) {
+                       $options[ $params['other'] ] = 'other';
+               }
+
+               $optgroup = false;
                foreach ( explode( "\n", $list ) as $option ) {
                        $value = trim( $option );
                        if ( $value == '' ) {
                                continue;
                        } elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-                               // A new group is starting ...
+                               # A new group is starting...
                                $value = trim( substr( $value, 1 ) );
-                               if ( $optgroup ) {
-                                       $options .= self::closeElement( 'optgroup' );
-                               }
-                               $options .= self::openElement( 'optgroup', [ 'label' => $value ] );
-                               $optgroup = true;
+                               $optgroup = $value;
                        } elseif ( substr( $value, 0, 2 ) == '**' ) {
-                               // groupmember
-                               $value = trim( substr( $value, 2 ) );
-                               $options .= self::option( $value, $value, $selected === $value );
-                       } else {
-                               // groupless reason list
-                               if ( $optgroup ) {
-                                       $options .= self::closeElement( 'optgroup' );
+                               # groupmember
+                               $opt = trim( substr( $value, 2 ) );
+                               if ( $optgroup === false ) {
+                                       $options[$opt] = $opt;
+                               } else {
+                                       $options[$optgroup][$opt] = $opt;
                                }
-                               $options .= self::option( $value, $value, $selected === $value );
+                       } else {
+                               # groupless reason list
                                $optgroup = false;
+                               $options[$option] = $option;
                        }
                }
 
-               if ( $optgroup ) {
-                       $options .= self::closeElement( 'optgroup' );
-               }
-
-               $attribs = [];
-
-               if ( $name ) {
-                       $attribs['id'] = $name;
-                       $attribs['name'] = $name;
-               }
+               return $options;
+       }
 
-               if ( $class ) {
-                       $attribs['class'] = $class;
-               }
+       /**
+        * Convert options for a drop-down box into a format accepted by OOUI\DropdownInputWidget etc.
+        *
+        * TODO Find a better home for this function.
+        *
+        * @param array $options Options, as returned e.g. by Xml::listDropDownOptions()
+        * @return array
+        */
+       public static function listDropDownOptionsOoui( $options ) {
+               $optionsOoui = [];
 
-               if ( $tabindex ) {
-                       $attribs['tabindex'] = $tabindex;
+               foreach ( $options as $text => $value ) {
+                       if ( is_array( $value ) ) {
+                               $optionsOoui[] = [ 'optgroup' => (string)$text ];
+                               foreach ( $value as $text2 => $value2 ) {
+                                       $optionsOoui[] = [ 'data' => (string)$value2, 'label' => (string)$text2 ];
+                               }
+                       } else {
+                               $optionsOoui[] = [ 'data' => (string)$value, 'label' => (string)$text ];
+                       }
                }
 
-               return self::openElement( 'select', $attribs )
-                       . "\n"
-                       . $options
-                       . "\n"
-                       . self::closeElement( 'select' );
+               return $optionsOoui;
        }
 
        /**

diff --git a/includes/htmlform/HTMLFormField.php b/includes/htmlform/HTMLFormField.php
index dfd18ba..e642c2c 100644 (file)
--- a/includes/htmlform/HTMLFormField.php
+++ b/includes/htmlform/HTMLFormField.php
@@ -1076,33 +1076,8 @@ abstract class HTMLFormField {
                                $this->mOptionsLabelsNotFromMessage = true;
                                $this->mOptions = self::forceToStringRecursive( $this->mParams['options'] );
                        } elseif ( array_key_exists( 'options-message', $this->mParams ) ) {
-                               /** @todo This is copied from Xml::listDropDown(), deprecate/avoid duplication? */
                                $message = $this->getMessage( $this->mParams['options-message'] )->inContentLanguage()->plain();
-
-                               $optgroup = false;
-                               $this->mOptions = [];
-                               foreach ( explode( "\n", $message ) as $option ) {
-                                       $value = trim( $option );
-                                       if ( $value == '' ) {
-                                               continue;
-                                       } elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-                                               # A new group is starting...
-                                               $value = trim( substr( $value, 1 ) );
-                                               $optgroup = $value;
-                                       } elseif ( substr( $value, 0, 2 ) == '**' ) {
-                                               # groupmember
-                                               $opt = trim( substr( $value, 2 ) );
-                                               if ( $optgroup === false ) {
-                                                       $this->mOptions[$opt] = $opt;
-                                               } else {
-                                                       $this->mOptions[$optgroup][$opt] = $opt;
-                                               }
-                                       } else {
-                                               # groupless reason list
-                                               $optgroup = false;
-                                               $this->mOptions[$option] = $option;
-                                       }
-                               }
+                               $this->mOptions = Xml::listDropDownOptions( $message );
                        } else {
                                $this->mOptions = null;
                        }
@@ -1122,16 +1097,7 @@ abstract class HTMLFormField {
                        return null;
                }
 
-               $options = [];
-
-               foreach ( $oldoptions as $text => $data ) {
-                       $options[] = [
-                               'data' => (string)$data,
-                               'label' => (string)$text,
-                       ];
-               }
-
-               return $options;
+               return Xml::listDropDownOptionsOoui( $oldoptions );
        }
 
        /**

diff --git a/includes/page/Article.php b/includes/page/Article.php
index 5b03a22..b91bd9a 100644 (file)
--- a/includes/page/Article.php
+++ b/includes/page/Article.php
@@ -1695,24 +1695,11 @@ class Article implements Page {
 
                $outputPage->enableOOUI();
 
-               $options = [];
-               $options[] = [
-                       'data' => 'other',
-                       'label' => $ctx->msg( 'deletereasonotherlist' )->inContentLanguage()->text(),
-               ];
-               $list = $ctx->msg( 'deletereason-dropdown' )->inContentLanguage()->text();
-               foreach ( explode( "\n", $list ) as $option ) {
-                       $value = trim( $option );
-                       if ( $value == '' ) {
-                               continue;
-                       } elseif ( substr( $value, 0, 1 ) == '*' && substr( $value, 1, 1 ) != '*' ) {
-                               $options[] = [ 'optgroup' => trim( substr( $value, 1 ) ) ];
-                       } elseif ( substr( $value, 0, 2 ) == '**' ) {
-                               $options[] = [ 'data' => trim( substr( $value, 2 ) ) ];
-                       } else {
-                               $options[] = [ 'data' => trim( $value ) ];
-                       }
-               }
+               $options = Xml::listDropDownOptions(
+                       $ctx->msg( 'deletereason-dropdown' )->inContentLanguage()->text(),
+                       [ 'other' => $ctx->msg( 'deletereasonotherlist' )->inContentLanguage()->text() ]
+               );
+               $options = Xml::listDropDownOptionsOoui( $options );
 
                $fields[] = new OOUI\FieldLayout(
                        new OOUI\DropdownInputWidget( [

diff --git a/tests/phpunit/includes/XmlTest.php b/tests/phpunit/includes/XmlTest.php
index 6c059ec..25b754d 100644 (file)
--- a/tests/phpunit/includes/XmlTest.php
+++ b/tests/phpunit/includes/XmlTest.php
@@ -403,12 +403,15 @@ class XmlTest extends MediaWikiTestCase {
         */
        public function testListDropDown() {
                $this->assertEquals(
-                       '<select id="test-name" name="test-name" class="test-css" tabindex="2">' . "\n" .
-                               '<option value="other">other reasons</option>' .
-                               '<optgroup label="Foo"><option value="Foo 1">Foo 1</option>' .
-                               '<option value="Example" selected="">Example</option>' .
-                               '</optgroup><optgroup label="Bar">' .
-                               '<option value="Bar 1">Bar 1</option></optgroup>' . "\n" .
+                       '<select name="test-name" id="test-name" class="test-css" tabindex="2">' .
+                               '<option value="other">other reasons</option>' . "\n" .
+                               '<optgroup label="Foo">' .
+                               '<option value="Foo 1">Foo 1</option>' . "\n" .
+                               '<option value="Example" selected="">Example</option>' . "\n" .
+                               '</optgroup>' . "\n" .
+                               '<optgroup label="Bar">' .
+                               '<option value="Bar 1">Bar 1</option>' . "\n" .
+                               '</optgroup>' .
                                '</select>',
                        Xml::listDropDown(
                                // name
@@ -426,4 +429,52 @@ class XmlTest extends MediaWikiTestCase {
                        )
                );
        }
+
+       /**
+        * @covers Xml::listDropDownOptions
+        */
+       public function testListDropDownOptions() {
+               $this->assertEquals(
+                       [
+                               'other reasons' => 'other',
+                               'Foo' => [
+                                       'Foo 1' => 'Foo 1',
+                                       'Example' => 'Example',
+                               ],
+                               'Bar' => [
+                                       'Bar 1' => 'Bar 1',
+                               ],
+                       ],
+                       Xml::listDropDownOptions(
+                               "* Foo\n** Foo 1\n** Example\n* Bar\n** Bar 1",
+                               [ 'other' => 'other reasons' ]
+                       )
+               );
+       }
+
+       /**
+        * @covers Xml::listDropDownOptionsOoui
+        */
+       public function testListDropDownOptionsOoui() {
+               $this->assertEquals(
+                       [
+                               [ 'data' => 'other', 'label' => 'other reasons' ],
+                               [ 'optgroup' => 'Foo' ],
+                               [ 'data' => 'Foo 1', 'label' => 'Foo 1' ],
+                               [ 'data' => 'Example', 'label' => 'Example' ],
+                               [ 'optgroup' => 'Bar' ],
+                               [ 'data' => 'Bar 1', 'label' => 'Bar 1' ],
+                       ],
+                       Xml::listDropDownOptionsOoui( [
+                               'other reasons' => 'other',
+                               'Foo' => [
+                                       'Foo 1' => 'Foo 1',
+                                       'Example' => 'Example',
+                               ],
+                               'Bar' => [
+                                       'Bar 1' => 'Bar 1',
+                               ],
+                       ] )
+               );
+       }
 }