Krinkle has uploaded a new change for review.
https://gerrit.wikimedia.org/r/112837
Change subject: [WIP] resourceloader: Fix broken 'skinStyles' loop in file
module getModifiedTime
......................................................................
[WIP] resourceloader: Fix broken 'skinStyles' loop in file module
getModifiedTime
Applying the patch from Ie6b47f5d192953366911 in VisualEditor
did not result in a module update locally. I kept refreshing and
purging but nothing happened.
Turns out the way we fetch files in #getModifiedTime is broken
for 'skinStyles'. For comparision, look at the loop in #getStyleFiles
which we use for the creation of the actual stylesheets and that
one does work.
FIXME / DONT MERGE:
Need to find a way to unit test this, hacked up for now as proof
of concept to the reviewer (try the hack and unit test without
the patch to see it fail).
Change-Id: I2e772a13183e66e4bfbf95057ebfd7f5e0d817ec
---
M includes/resourceloader/ResourceLoaderFileModule.php
M tests/phpunit/includes/ResourceLoaderModuleTest.php
2 files changed, 75 insertions(+), 6 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/37/112837/1
diff --git a/includes/resourceloader/ResourceLoaderFileModule.php
b/includes/resourceloader/ResourceLoaderFileModule.php
index eaff86f..c6fe2d9 100644
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -429,10 +429,11 @@
foreach ( $styles as $styleFiles ) {
$files = array_merge( $files, $styleFiles );
}
- $skinFiles = self::tryForKey(
- self::collateFilePathListByOption( $this->skinStyles,
'media', 'all' ),
- $context->getSkin(),
- 'default'
+
+ $skinFiles = self::collateFilePathListByOption(
+ self::tryForKey( $this->skinStyles,
$context->getSkin(), 'default' ),
+ 'media',
+ 'all'
);
foreach ( $skinFiles as $styleFiles ) {
$files = array_merge( $files, $styleFiles );
@@ -447,6 +448,7 @@
self::tryForKey( $this->skinScripts,
$context->getSkin(), 'default' ),
$this->loaderScripts
);
+ $this->_modifiedTime_files = $files;
$files = array_map( array( $this, 'getLocalPath' ), $files );
// File deps need to be treated separately because they're
already prefixed
$files = array_merge( $files, $this->getFileDependencies(
$context->getSkin() ) );
@@ -614,7 +616,9 @@
return array_merge_recursive(
self::collateFilePathListByOption( $this->styles,
'media', 'all' ),
self::collateFilePathListByOption(
- self::tryForKey( $this->skinStyles,
$context->getSkin(), 'default' ), 'media', 'all'
+ self::tryForKey( $this->skinStyles,
$context->getSkin(), 'default' ),
+ 'media',
+ 'all'
)
);
}
diff --git a/tests/phpunit/includes/ResourceLoaderModuleTest.php
b/tests/phpunit/includes/ResourceLoaderModuleTest.php
index 4643319..9783b20 100644
--- a/tests/phpunit/includes/ResourceLoaderModuleTest.php
+++ b/tests/phpunit/includes/ResourceLoaderModuleTest.php
@@ -82,6 +82,71 @@
'Class is significant'
);
}
+
+ /**
+ */
+ public function testF() {
+ $context = self::getResourceLoaderContext();
+
+ $params = array(
+ 'scripts' => array( 'foo.js', 'bar.js' ),
+ 'dependencies' => array( 'jquery', 'mediawiki' ),
+ 'messages' => array( 'hello', 'world' ),
+ 'styles' => array(
+ 'foo.css',
+ ),
+ 'skinStyles' => array(
+ 'vector' => array(
+ 'foo-vector.css',
+ 'foo-vector-hd.css' => array(
+ 'media' => 'screen and
(min-width: 982px)'
+ ),
+ ),
+ 'monobook' => array(
+ 'foo-monobook.css',
+ ),
+ 'default' => 'foo-other.css'
+ ),
+ );
+ $module = new ResourceLoaderFileModuleTestModule( $params );
+
+
+ $this->assertEquals(
+ $module->test_getStyleFiles( $context ),
+ array(
+ 'all' => array(
+ 'foo.css',
+ 'foo-vector.css',
+ ),
+ 'screen and (min-width: 982px)' => array(
+ 'foo-vector-hd.css',
+ ),
+ ),
+ 'Style files'
+ );
+ $module->getModifiedTime( $context );
+ $this->assertEquals(
+ $module->test_getModifiedTime_files( $context ),
+ array(
+ 'foo.css',
+ 'foo-vector.css',
+ 'foo-vector-hd.css',
+ 'foo.js',
+ 'bar.js',
+ ),
+ 'Files'
+ );
+ }
}
-class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {}
+class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {
+
+ // Make public
+ public function test_getStyleFiles( $context ) {
+ return $this->getStyleFiles( $context );
+ }
+
+ public function test_getModifiedTime_files() {
+ return $this->_modifiedTime_files;
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/112837
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2e772a13183e66e4bfbf95057ebfd7f5e0d817ec
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
