resourceloader: Allow skins to provide additional styles for any module
authorBartosz Dziewoński <matma.rex@gmail.com>
Thu, 26 Jun 2014 14:29:31 +0000 (16:29 +0200)
committerBartosz Dziewoński <matma.rex@gmail.com>
Mon, 28 Jul 2014 22:53:41 +0000 (00:53 +0200)
The newly introduced $wgResourceModuleSkinStyles global enables skins to
provide additional stylesheets to existing ResourceLoader module.

This both makes it easier (or at all possible) to override default
styles and lowers the style footprint by making it possible not to
load styles unused on most pages.

----

Example:

Use the file 'foo-styles.css' for the 'mediawiki.foo' module when using
the MySkin skin:

  $wgResourceModuleSkinStyles['myskin'] = array(
    'mediawiki.foo' => 'foo-styles.css',
    'remoteSkinPath' => 'MySkin',
    'localBasePath' => __DIR__,
  );

For detailed documentation, see the doc comment in DefaultSettings.php.
For a practical usage example, see Vector.php.

----

Implementation notes:

* The values defined in $wgResourceModuleSkinStyles are embedded into
  the modules as late as possible (in ResourceLoader::register()).
* Only plain file modules are supported, setting module skin styles
  for other module types has no effect.
* ResourceLoader and ResourceLoaderFileModule now support loading
  files from arbitrary paths to make this possible, defined using
  ResourceLoaderFilePath objects.
  * This required some adjustments in seemingly unrelated places for
    code which didn't handle the paths fully correctly before.
* ResourceLoader and ResourceLoaderFileModule are now a bit more
  tightly coupled than before :(
* Included a tiny example change for the Vector skin, a lot more of
  similar cleanup is possible and planned for the future.
* Many of the non-essential mediawiki.* modules defined in
  Resources.php should be using `'skinStyles' => array( 'default' => … )`
  instead of `'styles' => …` to allow more customizations, this is
  also planned for the future after auditing which ones would actually
  benefit from this.

Change-Id: Ica4ff9696b490e35f60288d7ce1295766c427e87

RELEASE-NOTES-1.24
includes/AutoLoader.php
includes/DefaultSettings.php
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderFileModule.php
includes/resourceloader/ResourceLoaderFilePath.php [new file with mode: 0644]
resources/Resources.php
skins/Vector/Vector.php
tests/phpunit/structure/ResourcesTest.php

index fa8ed00..8e6e8f5 100644 (file)
@@ -138,6 +138,8 @@ production.
 * MediaWiki now supports multiple password types, including bcrypt and PBKDF2.
   The default type can be changed with $wgPasswordDefault and the type
   configurations can be changed with $wgPasswordConfig.
+* Skins can now define custom styles for default ResourceLoader modules using
+  the $wgResourceModuleSkinStyles global. See the Vector skin for examples.
 
 === Bug fixes in 1.24 ===
 * (bug 49116) Footer copyright notice is now always displayed in user language
index 1d76e71..905050d 100644 (file)
@@ -869,6 +869,7 @@ $wgAutoloadLocalClasses = array(
        'ResourceLoaderContext' => 'includes/resourceloader/ResourceLoaderContext.php',
        'ResourceLoaderFileModule' => 'includes/resourceloader/ResourceLoaderFileModule.php',
        'ResourceLoaderFilePageModule' => 'includes/resourceloader/ResourceLoaderFilePageModule.php',
+       'ResourceLoaderFilePath' => 'includes/resourceloader/ResourceLoaderFilePath.php',
        'ResourceLoaderLESSFunctions' => 'includes/resourceloader/ResourceLoaderLESSFunctions.php',
        'ResourceLoaderModule' => 'includes/resourceloader/ResourceLoaderModule.php',
        'ResourceLoaderNoscriptModule' => 'includes/resourceloader/ResourceLoaderNoscriptModule.php',
index 9df920b..e6dd544 100644 (file)
@@ -3186,6 +3186,111 @@ $wgEnableCanonicalServerLink = false;
  */
 $wgResourceModules = array();
 
+/**
+ * Skin-specific styles for resource modules.
+ *
+ * These are later added to the 'skinStyles' list of the existing module. The 'styles' list can
+ * not be modified or disabled.
+ *
+ * For example, here is a module "bar" and how skin Foo would provide additional styles for it.
+ *
+ * @par Example:
+ * @code
+ *   $wgResourceModules['bar'] = array(
+ *     'scripts' => 'resources/bar/bar.js',
+ *     'styles' => 'resources/bar/main.css',
+ *   );
+ *
+ *   $wgResourceModuleSkinStyles['foo'] = array(
+ *     'bar' => 'skins/Foo/bar.css',
+ *   );
+ * @endcode
+ *
+ * This is mostly equivalent to:
+ *
+ * @par Equivalent:
+ * @code
+ *   $wgResourceModules['bar'] = array(
+ *     'scripts' => 'resources/bar/bar.js',
+ *     'styles' => 'resources/bar/main.css',
+ *     'skinStyles' => array(
+ *       'foo' => skins/Foo/bar.css',
+ *     ),
+ *   );
+ * @endcode
+ *
+ * If the module already defines its own entry in `skinStyles` for a given skin, then
+ * $wgResourceModuleSkinStyles is ignored.
+ *
+ * If a module defines a `skinStyles['default']` the skin may want to extend that instead
+ * of replacing them. This can be done using the `+` prefix.
+ *
+ * @par Example:
+ * @code
+ *   $wgResourceModules['bar'] = array(
+ *     'scripts' => 'resources/bar/bar.js',
+ *     'styles' => 'resources/bar/basic.css',
+ *     'skinStyles' => array(
+ *       'default' => 'resources/bar/additional.css',
+ *     ),
+ *   );
+ *   // Note the '+' character:
+ *   $wgResourceModuleSkinStyles['+foo'] = array(
+ *     'bar' => 'skins/Foo/bar.css',
+ *   );
+ * @endcode
+ *
+ * This is mostly equivalent to:
+ *
+ * @par Equivalent:
+ * @code
+ *   $wgResourceModules['bar'] = array(
+ *     'scripts' => 'resources/bar/bar.js',
+ *     'styles' => 'resources/bar/basic.css',
+ *     'skinStyles' => array(
+ *       'default' => 'resources/bar/additional.css',
+ *       'foo' => array(
+ *         'resources/bar/additional.css',
+ *         'skins/Foo/bar.css',
+ *       ),
+ *     ),
+ *   );
+ * @endcode
+ *
+ * In other words, as a module author, use the `styles` list for stylesheets that may not be
+ * disabled by a skin. To provide default styles that may be extended or replaced,
+ * use `skinStyles['default']`.
+ *
+ * As with $wgResourceModules, paths default to being relative to the MediaWiki root.
+ * You should always provide a localBasePath and remoteBasePath (or remoteExtPath/remoteSkinPath).
+ * Either for all skin styles at once (first example below) or for each module separately (second
+ * example).
+ *
+ * @par Example:
+ * @code
+ *   $wgResourceModuleSkinStyles['foo'] = array(
+ *     'bar' => 'bar.css',
+ *     'quux' => 'quux.css',
+ *     'remoteSkinPath' => 'Foo',
+ *     'localBasePath' => __DIR__,
+ *   );
+ *
+ *   $wgResourceModuleSkinStyles['foo'] = array(
+ *     'bar' => array(
+ *       'bar.css',
+ *       'remoteSkinPath' => 'Foo',
+ *       'localBasePath' => __DIR__,
+ *     ),
+ *     'quux' => array(
+ *       'quux.css',
+ *       'remoteSkinPath' => 'Foo',
+ *       'localBasePath' => __DIR__,
+ *     ),
+ *   );
+ * @endcode
+ */
+$wgResourceModuleSkinStyles = array();
+
 /**
  * Extensions should register foreign module sources here. 'local' is a
  * built-in source that is not in this array, but defined by
index a89b45f..bcb3842 100644 (file)
@@ -293,6 +293,47 @@ class ResourceLoader {
                                        '\': expected ResourceLoaderModule or array (got: ' . gettype( $info ) . ')'
                                );
                        }
+
+                       // Last-minute changes
+
+                       // Apply custom skin-defined styles to existing modules.
+                       if ( $this->isFileModule( $name ) ) {
+                               global $wgResourceModuleSkinStyles;
+                               foreach ( $wgResourceModuleSkinStyles as $skinName => $skinStyles ) {
+                                       // If this module already defines skinStyles for this skin, ignore $wgResourceModuleSkinStyles.
+                                       if ( isset( $this->moduleInfos[$name]['skinStyles'][$skinName] ) ) {
+                                               continue;
+                                       }
+
+                                       // If $name is preceded with a '+', the defined style files will be added to 'default'
+                                       // skinStyles, otherwise 'default' will be ignored as it normally would be.
+                                       if ( isset( $skinStyles[ $name ] ) ) {
+                                               $paths = (array)$skinStyles[ $name ];
+                                               $styleFiles = array();
+                                       } else if ( isset( $skinStyles[ '+' . $name ] ) ) {
+                                               $paths = (array)$skinStyles[ '+' . $name ];
+                                               $styleFiles = isset( $this->moduleInfos[$name]['skinStyles']['default'] ) ?
+                                                       $this->moduleInfos[$name]['skinStyles']['default'] :
+                                                       array();
+                                       } else {
+                                               continue;
+                                       }
+
+                                       // Add new file paths, remapping them to refer to our directories and not use settings
+                                       // from the module we're modifying. These can come from the base definition or be defined
+                                       // for each module.
+                                       list( $localBasePath, $remoteBasePath ) =
+                                               ResourceLoaderFileModule::extractBasePaths( $skinStyles );
+                                       list( $localBasePath, $remoteBasePath ) =
+                                               ResourceLoaderFileModule::extractBasePaths( $paths, $localBasePath, $remoteBasePath );
+
+                                       foreach ( $paths as $path ) {
+                                               $styleFiles[] = new ResourceLoaderFilePath( $path, $localBasePath, $remoteBasePath );
+                                       }
+
+                                       $this->moduleInfos[$name]['skinStyles'][$skinName] = $styleFiles;
+                               }
+                       }
                }
 
                wfProfileOut( __METHOD__ );
@@ -448,6 +489,23 @@ class ResourceLoader {
                return $this->modules[$name];
        }
 
+       /**
+        * Return whether the definition of a module corresponds to a simple ResourceLoaderFileModule.
+        *
+        * @param string $name Module name
+        * @return boolean
+        */
+       protected function isFileModule( $name ) {
+               if ( !isset( $this->moduleInfos[$name] ) ) {
+                       return false;
+               }
+               $info = $this->moduleInfos[$name];
+               if ( isset( $info['object'] ) || isset( $info['class'] ) ) {
+                       return false;
+               }
+               return true;
+       }
+
        /**
         * Get the list of sources.
         *
index 43bd562..edde9bc 100644 (file)
@@ -218,27 +218,17 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         *     )
         * @endcode
         */
-       public function __construct( $options = array(), $localBasePath = null,
+       public function __construct(
+               $options = array(),
+               $localBasePath = null,
                $remoteBasePath = null
        ) {
-               global $IP, $wgScriptPath, $wgResourceBasePath;
-               $this->localBasePath = $localBasePath === null ? $IP : $localBasePath;
-               if ( $remoteBasePath !== null ) {
-                       $this->remoteBasePath = $remoteBasePath;
-               } else {
-                       $this->remoteBasePath = $wgResourceBasePath === null ? $wgScriptPath : $wgResourceBasePath;
-               }
-
-               if ( isset( $options['remoteExtPath'] ) ) {
-                       global $wgExtensionAssetsPath;
-                       $this->remoteBasePath = $wgExtensionAssetsPath . '/' . $options['remoteExtPath'];
-               }
-
-               if ( isset( $options['remoteSkinPath'] ) ) {
-                       global $wgStylePath;
-                       $this->remoteBasePath = $wgStylePath . '/' . $options['remoteSkinPath'];
-               }
+               // localBasePath and remoteBasePath both have unbelievably long fallback chains
+               // and need to be handled separately.
+               list( $this->localBasePath, $this->remoteBasePath ) =
+                       self::extractBasePaths( $options, $localBasePath, $remoteBasePath );
 
+               // Extract, validate and normalise remaining options
                foreach ( $options as $member => $option ) {
                        switch ( $member ) {
                                // Lists of file paths
@@ -281,8 +271,6 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                                // Single strings
                                case 'group':
                                case 'position':
-                               case 'localBasePath':
-                               case 'remoteBasePath':
                                case 'skipFunction':
                                        $this->{$member} = (string)$option;
                                        break;
@@ -293,9 +281,59 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                                        break;
                        }
                }
+       }
+
+       /**
+        * Extract a pair of local and remote base paths from module definition information.
+        * Implementation note: the amount of global state used in this function is staggering.
+        *
+        * @param array $options Module definition
+        * @param string $localBasePath Path to use if not provided in module definition. Defaults
+        *     to $IP
+        * @param string $remoteBasePath Path to use if not provided in module definition. Defaults
+        *     to $wgScriptPath
+        * @return array array( localBasePath, remoteBasePath )
+        */
+       public static function extractBasePaths(
+               $options = array(),
+               $localBasePath = null,
+               $remoteBasePath = null
+       ) {
+               global $IP, $wgScriptPath, $wgResourceBasePath;
+
+               // The different ways these checks are done, and their ordering, look very silly,
+               // but were preserved for backwards-compatibility just in case. Tread lightly.
+
+               $localBasePath = $localBasePath === null ? $IP : $localBasePath;
+               if ( $remoteBasePath !== null ) {
+                       $remoteBasePath = $remoteBasePath;
+               } else {
+                       $remoteBasePath = $wgResourceBasePath === null ? $wgScriptPath : $wgResourceBasePath;
+               }
+
+               if ( isset( $options['remoteExtPath'] ) ) {
+                       global $wgExtensionAssetsPath;
+                       $remoteBasePath = $wgExtensionAssetsPath . '/' . $options['remoteExtPath'];
+               }
+
+               if ( isset( $options['remoteSkinPath'] ) ) {
+                       global $wgStylePath;
+                       $remoteBasePath = $wgStylePath . '/' . $options['remoteSkinPath'];
+               }
+
+               if ( array_key_exists( 'localBasePath', $options ) ) {
+                       $localBasePath = (string)$options['localBasePath'];
+               }
+
+               if ( array_key_exists( 'remoteBasePath', $options ) ) {
+                       $remoteBasePath = (string)$options['remoteBasePath'];
+               }
+
                // Make sure the remote base path is a complete valid URL,
                // but possibly protocol-relative to avoid cache pollution
-               $this->remoteBasePath = wfExpandUrl( $this->remoteBasePath, PROTO_RELATIVE );
+               $remoteBasePath = wfExpandUrl( $remoteBasePath, PROTO_RELATIVE );
+
+               return array( $localBasePath, $remoteBasePath );
        }
 
        /**
@@ -567,18 +605,26 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
        /* Protected Methods */
 
        /**
-        * @param string $path
+        * @param string|ResourceLoaderFilePath $path
         * @return string
         */
        protected function getLocalPath( $path ) {
+               if ( $path instanceof ResourceLoaderFilePath ) {
+                       return $path->getLocalPath();
+               }
+
                return "{$this->localBasePath}/$path";
        }
 
        /**
-        * @param string $path
+        * @param string|ResourceLoaderFilePath $path
         * @return string
         */
        protected function getRemotePath( $path ) {
+               if ( $path instanceof ResourceLoaderFilePath ) {
+                       return $path->getRemotePath();
+               }
+
                return "{$this->remoteBasePath}/$path";
        }
 
@@ -660,7 +706,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        $files = array_merge( $files, $this->debugScripts );
                }
 
-               return array_unique( $files );
+               return array_unique( $files, SORT_REGULAR );
        }
 
        /**
@@ -751,7 +797,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        return '';
                }
                $js = '';
-               foreach ( array_unique( $scripts ) as $fileName ) {
+               foreach ( array_unique( $scripts, SORT_REGULAR ) as $fileName ) {
                        $localPath = $this->getLocalPath( $fileName );
                        if ( !file_exists( $localPath ) ) {
                                throw new MWException( __METHOD__ . ": script file not found: \"$localPath\"" );
@@ -785,7 +831,7 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                        return array();
                }
                foreach ( $styles as $media => $files ) {
-                       $uniqueFiles = array_unique( $files );
+                       $uniqueFiles = array_unique( $files, SORT_REGULAR );
                        $styleFiles = array();
                        foreach ( $uniqueFiles as $file ) {
                                $styleFiles[] = $this->readStyleFile( $file, $flip );
@@ -808,13 +854,14 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
         */
        protected function readStyleFile( $path, $flip ) {
                $localPath = $this->getLocalPath( $path );
+               $remotePath = $this->getRemotePath( $path );
                if ( !file_exists( $localPath ) ) {
                        $msg = __METHOD__ . ": style file not found: \"$localPath\"";
                        wfDebugLog( 'resourceloader', $msg );
                        throw new MWException( $msg );
                }
 
-               if ( $this->getStyleSheetLang( $path ) === 'less' ) {
+               if ( $this->getStyleSheetLang( $localPath ) === 'less' ) {
                        $style = $this->compileLESSFile( $localPath );
                        $this->hasGeneratedStyles = true;
                } else {
@@ -824,20 +871,15 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
                if ( $flip ) {
                        $style = CSSJanus::transform( $style, true, false );
                }
-               $dirname = dirname( $path );
-               if ( $dirname == '.' ) {
-                       // If $path doesn't have a directory component, don't prepend a dot
-                       $dirname = '';
-               }
-               $dir = $this->getLocalPath( $dirname );
-               $remoteDir = $this->getRemotePath( $dirname );
+               $localDir = dirname( $localPath );
+               $remoteDir = dirname( $remotePath );
                // Get and register local file references
                $this->localFileRefs = array_merge(
                        $this->localFileRefs,
-                       CSSMin::getLocalFileReferences( $style, $dir )
+                       CSSMin::getLocalFileReferences( $style, $localDir )
                );
                return CSSMin::remap(
-                       $style, $dir, $remoteDir, true
+                       $style, $localDir, $remoteDir, true
                );
        }
 
diff --git a/includes/resourceloader/ResourceLoaderFilePath.php b/includes/resourceloader/ResourceLoaderFilePath.php
new file mode 100644 (file)
index 0000000..dd239d0
--- /dev/null
@@ -0,0 +1,74 @@
+<?php
+/**
+ * An object to represent a path to a JavaScript/CSS file, along with a remote
+ * and local base path, for use with ResourceLoaderFileModule.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * An object to represent a path to a JavaScript/CSS file, along with a remote
+ * and local base path, for use with ResourceLoaderFileModule.
+ */
+class ResourceLoaderFilePath {
+       /* Protected Members */
+
+       /** @var string Local base path */
+       protected $localBasePath;
+
+       /** @var string Remote base path */
+       protected $remoteBasePath;
+
+       /**
+        * @var string Path to the file */
+       protected $path;
+
+       /* Methods */
+
+       /**
+        * @param string $path Path to the file.
+        * @param string $localBasePath Base path to prepend when generating a local path.
+        * @param string $remoteBasePath Base path to prepend when generating a remote path.
+        */
+       public function __construct( $path, $localBasePath, $remoteBasePath ) {
+               $this->path = $path;
+               $this->localBasePath = $localBasePath;
+               $this->remoteBasePath = $remoteBasePath;
+       }
+
+       /**
+        * @return string
+        */
+       public function getLocalPath() {
+               return "{$this->localBasePath}/{$this->path}";
+       }
+
+       /**
+        * @return string
+        */
+       public function getRemotePath() {
+               return "{$this->remoteBasePath}/{$this->path}";
+       }
+
+       /**
+        * @return string
+        */
+       public function getPath() {
+               return $this->path;
+       }
+}
index 942b332..0982b29 100644 (file)
@@ -1205,9 +1205,6 @@ return array(
        'mediawiki.special' => array(
                'scripts' => 'resources/src/mediawiki.special/mediawiki.special.js',
                'styles' => 'resources/src/mediawiki.special/mediawiki.special.css',
-               'skinStyles' => array(
-                       'vector' => 'skins/Vector/special.less', // FIXME this should use $wgStyleDirectory
-               ),
        ),
        'mediawiki.special.block' => array(
                'scripts' => 'resources/src/mediawiki.special/mediawiki.special.block.js',
@@ -1257,9 +1254,6 @@ return array(
                'scripts' => 'resources/src/mediawiki.special/mediawiki.special.preferences.js',
                'styles' => 'resources/src/mediawiki.special/mediawiki.special.preferences.css',
                'position' => 'top',
-               'skinStyles' => array(
-                       'vector' => 'skins/Vector/special.preferences.less', // FIXME this should use $wgStyleDirectory
-               ),
                'messages' => array(
                        'prefs-tabs-navigation-hint',
                ),
index be8719a..abcc65d 100644 (file)
@@ -62,3 +62,11 @@ $wgResourceModules['skins.vector.js'] = array(
        'remoteSkinPath' => 'Vector',
        'localBasePath' => __DIR__,
 );
+
+// Apply module customizations
+$wgResourceModuleSkinStyles['vector'] = array(
+       'mediawiki.special' => 'special.less',
+       'mediawiki.special.preferences' => 'special.preferences.less',
+       'remoteSkinPath' => 'Vector',
+       'localBasePath' => __DIR__,
+);
index bcd57a0..647386d 100644 (file)
@@ -255,7 +255,7 @@ class ResourcesTest extends MediaWikiTestCase {
                                $cases[] = array(
                                        $method->invoke( $module, $file ),
                                        $moduleName,
-                                       $file,
+                                       ( $file instanceof ResourceLoaderFilePath ? $file->getPath() : $file ),
                                );
                        }
                }