[Html::namespaceSelector] Remove default id/name attributes

* Remove default id/name attributes
* Remove now redundant tests introduced in r110274
* Add tests to make sure label has no 'for' attribute when label isn't null but name/id are unset
* Update tests to not include id="" and name="" when calling with no arguments
* Updating calls to add name/id if needed, and while at it remove useless 'null' params

* Context:
-- Introduced in r109990, r111376, r111315
-- No callers exist that assume these defaults. Forcing an ID that should be unique is annoying and redundant. The name used in the query when submitting a form should be mentioned in the same file where it is used from the submission, never assume what name="" is from unrelated code
-- Same for ID, this is often used in CSS or JavaScript, shouldn't be assumed. (It should be simple to but two simple namespace selectors on a page without getting DOM conflicts)

diff --git a/includes/Html.php b/includes/Html.php
index 26a40cd..3986a7b 100644 (file)
--- a/includes/Html.php
+++ b/includes/Html.php
@@ -718,11 +718,6 @@ class Html {
        public static function namespaceSelector( Array $params = array(), Array $selectAttribs = array() ) {
                global $wgContLang;
 
-               // Default 'id' & 'name' <select> attributes
-               $selectAttribs = $selectAttribs + array(
-                       'id'   => 'namespace',
-                       'name' => 'namespace',
-               );
                ksort( $selectAttribs );
 
                // Is a namespace selected?
@@ -768,14 +763,22 @@ class Html {
                                // main we don't use "" but the user message descripting it (e.g. "(Main)" or "(Article)")
                                $nsName = wfMsg( 'blanknamespace' );
                        }
-                       $optionsHtml[] = Xml::option( $nsName, $nsId, $nsId === $params['selected'], array(
-                               'disabled' => in_array( $nsId, $params['disable'] ),
-                       ) );
+                       $optionsHtml[] = Html::element(
+                               'option', array(
+                                       'disabled' => in_array( $nsId, $params['disable'] ),
+                                       'value' => $nsId,
+                                       'selected' => $nsId === $params['selected'],
+                               ), $nsName
+                       );
                }
 
                $ret = '';
                if ( isset( $params['label'] ) ) {
-                       $ret .= Xml::label( $params['label'], $selectAttribs['id'] ) . '&#160;';
+                       $ret .= Html::element(
+                               'label', array(
+                                       'for' => isset( $selectAttribs['id'] ) ? $selectAttribs['id'] : null,
+                               ), $params['label']
+                       ) . '&#160;';
                }
 
                // Wrap options in a <select>

diff --git a/includes/specials/SpecialContributions.php b/includes/specials/SpecialContributions.php
index 2a46573..501ccee 100644 (file)
--- a/includes/specials/SpecialContributions.php
+++ b/includes/specials/SpecialContributions.php
@@ -447,7 +447,6 @@ class SpecialContributions extends SpecialPage {
                                Html::namespaceSelector( array(
                                        'selected' => $this->opts['namespace'],
                                        'all'      => '',
-                                       'label'    => null,
                                ), array(
                                        'name'  => 'namespace',
                                        'id'    => 'namespace',

diff --git a/includes/specials/SpecialPrefixindex.php b/includes/specials/SpecialPrefixindex.php
index 5779cad..3202c00 100644 (file)
--- a/includes/specials/SpecialPrefixindex.php
+++ b/includes/specials/SpecialPrefixindex.php
@@ -109,8 +109,6 @@ class SpecialPrefixindex extends SpecialAllpages {
                                <td class='mw-input'>" .
                                        Html::namespaceSelector( array(
                                                'selected' => $namespace,
-                                               'all'      => null,
-                                               'label'    => null,
                                        ), array(
                                                'name'  => 'namespace',
                                                'id'    => 'namespace',

diff --git a/tests/phpunit/includes/HtmlTest.php b/tests/phpunit/includes/HtmlTest.php
index 9e2deb1..8b019d7 100644 (file)
--- a/tests/phpunit/includes/HtmlTest.php
+++ b/tests/phpunit/includes/HtmlTest.php
@@ -215,7 +215,7 @@ class HtmlTest extends MediaWikiTestCase {
 
        function testNamespaceSelector() {
                $this->assertEquals(
-                       '<select id="namespace" name="namespace">' . "\n" .
+                       '<select>' . "\n" .
 '<option value="0">(Main)</option>' . "\n" .
 '<option value="1">Talk</option>' . "\n" .
 '<option value="2">User</option>' . "\n" .
@@ -236,6 +236,7 @@ class HtmlTest extends MediaWikiTestCase {
                        Html::namespaceSelector(),
                        'Basic namespace selector without custom options'
                );
+
                $this->assertEquals(
                        '<label for="mw-test-namespace">Select a namespace:</label>&#160;' .
 '<select id="mw-test-namespace" name="wpNamespace">' . "\n" .
@@ -263,11 +264,37 @@ class HtmlTest extends MediaWikiTestCase {
                        ),
                        'Basic namespace selector with custom values'
                );
-               }
 
-               function testCanFilterOutNamespaces() {
-                       $this->assertEquals(
-'<select id="namespace" name="namespace">' . "\n" .
+               $this->assertEquals(
+                       '<label>Select a namespace:</label>&#160;' .
+'<select>' . "\n" .
+'<option value="0">(Main)</option>' . "\n" .
+'<option value="1">Talk</option>' . "\n" .
+'<option value="2">User</option>' . "\n" .
+'<option value="3">User talk</option>' . "\n" .
+'<option value="4">MyWiki</option>' . "\n" .
+'<option value="5">MyWiki Talk</option>' . "\n" .
+'<option value="6">File</option>' . "\n" .
+'<option value="7">File talk</option>' . "\n" .
+'<option value="8">MediaWiki</option>' . "\n" .
+'<option value="9">MediaWiki talk</option>' . "\n" .
+'<option value="10">Template</option>' . "\n" .
+'<option value="11">Template talk</option>' . "\n" .
+'<option value="14">Category</option>' . "\n" .
+'<option value="15">Category talk</option>' . "\n" .
+'<option value="100">Custom</option>' . "\n" .
+'<option value="101">Custom talk</option>' . "\n" .
+'</select>',
+                       Html::namespaceSelector(
+                               array( 'label' => 'Select a namespace:' )
+                       ),
+                       'Basic namespace selector with a custom label but no id attribtue for the <select>'
+               );
+       }
+
+       function testCanFilterOutNamespaces() {
+               $this->assertEquals(
+'<select>' . "\n" .
 '<option value="2">User</option>' . "\n" .
 '<option value="4">MyWiki</option>' . "\n" .
 '<option value="5">MyWiki Talk</option>' . "\n" .
@@ -285,11 +312,11 @@ class HtmlTest extends MediaWikiTestCase {
                        ),
                        'Namespace selector namespace filtering.'
                );
-               }
+       }
 
-               function testCanDisableANamespaces() {
-                       $this->assertEquals(
-'<select id="namespace" name="namespace">' . "\n" .
+       function testCanDisableANamespaces() {
+               $this->assertEquals(
+'<select>' . "\n" .
 '<option disabled="" value="0">(Main)</option>' . "\n" .
 '<option disabled="" value="1">Talk</option>' . "\n" .
 '<option disabled="" value="2">User</option>' . "\n" .
@@ -314,75 +341,4 @@ class HtmlTest extends MediaWikiTestCase {
                );
        }
 
-       function testNamespaceSelectorIdAndNameDefaultsAttributes() {
-
-               $this->assertNsSelectorIdAndName(
-                       'namespace', 'namespace',
-                       Html::namespaceSelector( array(), array(
-                               # neither 'id' nor 'name' key given
-                       )),
-                       "Neither 'id' nor 'name' key given"
-               );
-
-               $this->assertNsSelectorIdAndName(
-                       'namespace', 'select_name',
-                       Html::namespaceSelector( array(), array(
-                               'name' => 'select_name',
-                               # no 'id' key given
-                       )),
-                       "No 'id' key given, 'name' given"
-               );
-
-               $this->assertNsSelectorIdAndName(
-                       'select_id', 'namespace',
-                       Html::namespaceSelector( array(), array(
-                               'id' => 'select_id',
-                               # no 'name' key given
-                       )),
-                       "'id' given, no 'name' key given"
-               );
-
-               $this->assertNsSelectorIdAndName(
-                       'select_id', 'select_name',
-                       Html::namespaceSelector( array(), array(
-                               'id'   => 'select_id',
-                               'name' => 'select_name',
-                       )),
-                       "Both 'id' and 'name' given"
-               );
-       }
-
-       /**
-        * Helper to verify <select> attributes generated by Html::namespaceSelector()
-        * This helper expect the Html method to use 'namespace' as a default value for
-        * both 'id' and 'name' attributes.
-        *
-        * @param String $expectedId <select> id attribute value
-        * @param String $expectedName <select> name attribute value
-        * @param String $html Output of a call to Html::namespaceSelector()
-        * @param String $msg Optional message (default: '')
-       */
-       function assertNsSelectorIdAndName( $expectedId, $expectedName, $html, $msg = '' ) {
-               $actualId = 'namespace';
-               if( 1 === preg_match( '/id="(.+?)"/', $html, $m ) ) {
-                       $actualId = $m[1];
-               }
-
-               $actualName = 'namespace';
-               if( 1 === preg_match( '/name="(.+?)"/', $html, $m ) ) {
-                       $actualName = $m[1];
-               }
-               $this->assertEquals(
-                       array( #expected
-                               'id'   => $expectedId,
-                               'name' => $expectedName,
-                       ),
-                       array( #actual
-                               'id'   => $actualId,
-                               'name' => $actualName,
-                       ),
-                       'Html::namespaceSelector() got wrong id and/or name attribute(s). ' . $msg
-               );
-       }
-
 }