jenkins-bot has submitted this change and it was merged.
Change subject: resourceloader: Omit empty parameters from mw.loader.implement
calls
......................................................................
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
---
M includes/resourceloader/ResourceLoader.php
M tests/phpunit/includes/OutputPageTest.php
M tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
3 files changed, 128 insertions(+), 19 deletions(-)
Approvals:
Catrope: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 15bb13f..08a854c 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1065,23 +1065,19 @@
} 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 @@
);
}
+ 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
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -172,7 +172,7 @@
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
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -221,6 +221,106 @@
);
}
+ 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
*/
--
To view, visit https://gerrit.wikimedia.org/r/178395
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic8d7efe49b5d45e3f95a2f04e3a26a014b10af16
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: Legoktm <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: Trevor Parscal <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
