From 1e5689852ba4d49e80e24f214cd501164a0b4ebd Mon Sep 17 00:00:00 2001
From: Krinkle <krinkle@users.mediawiki.org>
Date: Wed, 7 Mar 2012 19:14:20 +0000
Subject: [PATCH] [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)
---
 includes/Html.php                          |  21 ++--
 includes/specials/SpecialContributions.php |   1 -
 includes/specials/SpecialPrefixindex.php   |   2 -
 tests/phpunit/includes/HtmlTest.php        | 116 +++++++--------------
 4 files changed, 48 insertions(+), 92 deletions(-)

diff --git a/includes/Html.php b/includes/Html.php
index 26a40cd80a..3986a7b100 100644
--- 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 2a465739b9..501ccee517 100644
--- 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 5779cadd43..3202c00966 100644
--- 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 9e2deb14a9..8b019d78e6 100644
--- 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
-		);
-	}
-
 }
-- 
2.20.1