From 4ce0c0da42acfbcc5c68527834f85436efd0ebc1 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 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 --- 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 15bb13f4cb..08a854cb39 100644 --- 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 4d63ea61c8..66e5d68421 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 4fc7378a8a..a8cf991b16 100644 --- 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 */ -- 2.20.1