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

Follows-up ebeb297236.

Also:
* Add tests for ResourceLoader::makeLoaderImplementScript().
* Apply ResourceLoader::trimArray to makeLoaderImplementScript (new in c0c221bf).

As always, the client (updated in Ie32e7d6a3c) is backward-compatible with old
(cached) load.php module responses. However, the old client is not compatible
new load.php responses after this commit.

That's generally not an issue, as we don't cache the client very long (~ 5 min).
However people with their browser open and mw.loader clients initialised can
still make new module requests (e.g. modules loaded on-demand, such as when
previewing edits, clicking buttons etc.). That can easily be several hours after
initial page load. As such, client/server bound changes should always be
back-compat and deployed a reasonable time apart to reduce chances of active
sessions making subsequent requests. Ideally we'd find some solution to this in
the long-term, but handling this at all is better than what we usually do...

For deployment: Ensure this is deployed several days after Ie32e7d6a3c09f.

Change-Id: Ic8d7efe49b5d45e3f95a2f04e3a26a014b10af16

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 15bb13f..08a854c 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1065,23 +1065,19 @@ class ResourceLoader {
                } 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() );
        }
 
        /**
@@ -1188,20 +1184,33 @@ class ResourceLoader {
                );
        }
 
+       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 4d63ea6..66e5d68 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 4fc7378..a8cf991 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -221,6 +221,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
         */