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

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index b9d1b2b..976275b 100644 (file)
--- 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 ee6a8cf..6c6d95e 100644 (file)
--- 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 e43db78..ca7307e 100644 (file)
--- 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
         */