Revert "resourceloader: Omit empty parameters from mw.loader.implement calls"
authorKrinkle <krinklemail@gmail.com>
Tue, 20 Jan 2015 23:56:17 +0000 (23:56 +0000)
committerKrinkle <krinklemail@gmail.com>
Tue, 20 Jan 2015 23:56:17 +0000 (23:56 +0000)
Works as intended, but didn't account for the first implement() parameter
being omitted client-side. Revert until that is accounted for, then re-try after
that fix is rolled out for > 1 week.

This reverts commit 4ce0c0da42acfbcc5c68527834f85436efd0ebc1.

Change-Id: I36c1619991663c0303636d1d3f037b0021ac79bf

includes/resourceloader/ResourceLoader.php
tests/phpunit/includes/OutputPageTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderTest.php

index 08a854c..15bb13f 100644 (file)
@@ -1065,19 +1065,23 @@ class ResourceLoader {
                } elseif ( !is_array( $scripts ) ) {
                        throw new MWException( 'Invalid scripts error. Array of URLs or string of code expected.' );
                }
-               // 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() );
+               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()
+               );
        }
 
        /**
@@ -1184,33 +1188,20 @@ 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
-        * - array()
-        * - new XmlJsCode( '{}' )
-        * - new stdClass() // (object) array()
+        * - empty array
         *
         * @param Array $array
         */
        private static function trimArray( Array &$array ) {
                $i = count( $array );
                while ( $i-- ) {
-                       if ( $array[$i] === null
-                               || $array[$i] === array()
-                               || ( $array[$i] instanceof XmlJsCode && $array[$i]->value === '{}' )
-                               || ( $array[$i] instanceof stdClass && self::isEmptyObject( $array[$i] ) )
-                       ) {
+                       if ( $array[$i] === null || $array[$i] === array() ) {
                                unset( $array[$i] );
                        } else {
                                break;
index 66e5d68..4d63ea6 100644 (file)
@@ -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>
 '
index a8cf991..4fc7378 100644 (file)
@@ -221,106 +221,6 @@ 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
         */