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

Follows-up ebeb29723, 1f393b6da, 0e719ce23.

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

This commit changes the load.php response to omit empty parameters.

These parameters were required until recently. The client has been
updated (1f393b6da and 0e719ce23) to make these optional, thus supporting
both the old server format and the change this commit makes

Clients with a tab open from before 0e719ce23 are naturally not
compatible with load.php responses from after this commit. Ensure
this is deployed several days after 0e719ce23 to reduce race
conditions of this nature.

(This is a re-submitted version of 4ce0c0da4)

Bug: T88879
Change-Id: I9e998261ee9b0b745e3339bc3493755c0cb04b6a

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index 5eab3cb..ce1fd82 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1081,23 +1081,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() );
        }
 
        /**
@@ -1204,20 +1200,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 2526fcc..9825e0d 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
         */