Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/186109
Change subject: Revert "resourceloader: Omit empty parameters from
mw.loader.implement calls"
......................................................................
Revert "resourceloader: Omit empty parameters from mw.loader.implement calls"
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
---
M includes/resourceloader/ResourceLoader.php
M tests/phpunit/includes/OutputPageTest.php
M tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
3 files changed, 19 insertions(+), 128 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/09/186109/1
diff --git a/includes/resourceloader/ResourceLoader.php
b/includes/resourceloader/ResourceLoader.php
index 08a854c..15bb13f 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1065,19 +1065,23 @@
} 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 @@
);
}
- 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;
diff --git a/tests/phpunit/includes/OutputPageTest.php
b/tests/phpunit/includes/OutputPageTest.php
index 66e5d68..4d63ea6 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 a8cf991..4fc7378 100644
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderTest.php
@@ -221,106 +221,6 @@
);
}
- 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/186109
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I36c1619991663c0303636d1d3f037b0021ac79bf
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
