From 3cf2f18bb80593ca3066ed4bf1eab0d341f80df6 Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Tue, 9 Dec 2014 01:17:53 +0000 Subject: [PATCH] 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 --- includes/resourceloader/ResourceLoader.php | 45 ++++---- tests/phpunit/includes/OutputPageTest.php | 2 +- .../resourceloader/ResourceLoaderTest.php | 100 ++++++++++++++++++ 3 files changed, 128 insertions(+), 19 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 5eab3cb66b..ce1fd82694 100644 --- 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 2526fccd83..9825e0db76 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 ), ' ' 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 */ -- 2.20.1