From: jenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Date: Mon, 6 Apr 2015 06:22:45 +0000 (+0000)
Subject: Merge "resourceloader: Omit empty parameters from mw.loader.implement calls"
X-Git-Tag: 1.31.0-rc.0~11840
X-Git-Url: https://git.cyclocoop.org/admin/?a=commitdiff_plain;h=5d7a5925a4e8c041daf24b928658aa6c8d4f38d8;hp=6db469c225a6c6340e8507474e9ad876388f51d4;p=lhc%2Fweb%2Fwiklou.git

Merge "resourceloader: Omit empty parameters from mw.loader.implement calls"
---

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index b9d1b2bd12..976275bb23 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1095,23 +1095,19 @@ MESSAGE;
 		} elseif ( !is_array( $scripts ) ) {
 			throw new MWException( 'Invalid scripts error. Array of URLs or string of code expected.' );
 		}
-
-		return Xml::encodeJsCall(
-			'mw.loader.implement',
-			array(
-				$name,
-				$scripts,
-				// Force objects. mw.loader.implement requires them to be javascript objects.
-				// Although these variables are associative arrays, which become javascript
-				// objects through json_encode. In many cases they will be empty arrays, and
-				// PHP/json_encode() consider empty arrays to be numerical arrays and
-				// output javascript "[]" instead of "{}". This fixes that.
-				(object)$styles,
-				(object)$messages,
-				(object)$templates,
-			),
-			ResourceLoader::inDebugMode()
+		// mw.loader.implement requires 'styles', 'messages' and 'templates' to be objects (not
+		// arrays). json_encode considers empty arrays to be numerical and outputs "[]" instead
+		// of "{}". Force them to objects.
+		$module = array(
+			$name,
+			$scripts,
+			(object) $styles,
+			(object) $messages,
+			(object) $templates,
 		);
+		self::trimArray( $module );
+
+		return Xml::encodeJsCall( 'mw.loader.implement', $module, ResourceLoader::inDebugMode() );
 	}
 
 	/**
@@ -1218,20 +1214,33 @@ MESSAGE;
 		);
 	}
 
+	private static function isEmptyObject( stdClass $obj ) {
+		foreach ( $obj as $key => &$value ) {
+			return false;
+		}
+		return true;
+	}
+
 	/**
 	 * Remove empty values from the end of an array.
 	 *
 	 * Values considered empty:
 	 *
 	 * - null
-	 * - empty array
+	 * - array()
+	 * - new XmlJsCode( '{}' )
+	 * - new stdClass() // (object) array()
 	 *
 	 * @param Array $array
 	 */
 	private static function trimArray( Array &$array ) {
 		$i = count( $array );
 		while ( $i-- ) {
-			if ( $array[$i] === null || $array[$i] === array() ) {
+			if ( $array[$i] === null
+				|| $array[$i] === array()
+				|| ( $array[$i] instanceof XmlJsCode && $array[$i]->value === '{}' )
+				|| ( $array[$i] instanceof stdClass && self::isEmptyObject( $array[$i] ) )
+			) {
 				unset( $array[$i] );
 			} else {
 				break;
diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index ee6a8cfc22..6c6d95ee8a 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -172,7 +172,7 @@ mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});
 			array(
 				array( 'test.quux', ResourceLoaderModule::TYPE_COMBINED ),
 				'<script>if(window.mw){
-mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{},{});
+mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]});
 
 }</script>
 '
diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
index e43db78c80..ca7307ec9a 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -186,6 +186,106 @@ class ResourceLoaderTest extends ResourceLoaderTestCase {
 		);
 	}
 
+	public static function provideLoaderImplement() {
+		return array(
+			array( array(
+				'title' => 'Implement scripts, styles and messages',
+
+				'name' => 'test.example',
+				'scripts' => 'mw.example();',
+				'styles' => array( 'css' => array( '.mw-example {}' ) ),
+				'messages' => array( 'example' => '' ),
+				'templates' => array(),
+
+				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) {
+mw.example();
+}, {
+    "css": [
+        ".mw-example {}"
+    ]
+}, {
+    "example": ""
+} );',
+			) ),
+			array( array(
+				'title' => 'Implement scripts',
+
+				'name' => 'test.example',
+				'scripts' => 'mw.example();',
+				'styles' => array(),
+				'messages' => new XmlJsCode( '{}' ),
+				'templates' => array(),
+				'title' => 'scripts, styles and messags',
+
+				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) {
+mw.example();
+} );',
+			) ),
+			array( array(
+				'title' => 'Implement styles',
+
+				'name' => 'test.example',
+				'scripts' => array(),
+				'styles' => array( 'css' => array( '.mw-example {}' ) ),
+				'messages' => new XmlJsCode( '{}' ),
+				'templates' => array(),
+
+				'expected' => 'mw.loader.implement( "test.example", [], {
+    "css": [
+        ".mw-example {}"
+    ]
+} );',
+			) ),
+			array( array(
+				'title' => 'Implement scripts and messages',
+
+				'name' => 'test.example',
+				'scripts' => 'mw.example();',
+				'styles' => array(),
+				'messages' => array( 'example' => '' ),
+				'templates' => array(),
+
+				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) {
+mw.example();
+}, {}, {
+    "example": ""
+} );',
+			) ),
+			array( array(
+				'title' => 'Implement scripts and templates',
+
+				'name' => 'test.example',
+				'scripts' => 'mw.example();',
+				'styles' => array(),
+				'messages' => new XmlJsCode( '{}' ),
+				'templates' => array( 'example.html' => '' ),
+
+				'expected' => 'mw.loader.implement( "test.example", function ( $, jQuery ) {
+mw.example();
+}, {}, {}, {
+    "example.html": ""
+} );',
+			) ),
+		);
+	}
+
+	/**
+	 * @dataProvider provideLoaderImplement
+	 * @covers ResourceLoader::makeLoaderImplementScript
+	 */
+	public function testMakeLoaderImplementScript( $case ) {
+		$this->assertEquals(
+			$case['expected'],
+			ResourceLoader::makeLoaderImplementScript(
+				$case['name'],
+				$case['scripts'],
+				$case['styles'],
+				$case['messages'],
+				$case['templates']
+			)
+		);
+	}
+
 	/**
 	 * @covers ResourceLoader::getLoadScript
 	 */