From 06b2b1bd6624bb5e722f89123bc88e372df35a31 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Wed, 3 Nov 2010 07:58:03 +0000 Subject: [PATCH] Resource loader minor changes. Fix for r73668 etc. * Break long lines. * Convert long or unnecessary ternary operator usages to if/else. * Fixed excessively clever assignment expressions. * Rename $cache to $cacheEntry. * Removed unnecessary web invocation guards. Their perlish form was making me uncomfortable. BTW, unlike in Perl, die() is not a function, it's a special case in the PHP grammar which very roughly simulates the Perl syntax: die "x"; // works 0 || die("x"); // works 0 || (die); // works 0 || (die "x"); // fail! --- includes/resourceloader/ResourceLoader.php | 88 +++++++++++++------ .../resourceloader/ResourceLoaderContext.php | 2 - .../ResourceLoaderFileModule.php | 2 - .../resourceloader/ResourceLoaderModule.php | 8 +- .../ResourceLoaderSiteModule.php | 7 +- .../ResourceLoaderStartUpModule.php | 27 +++--- .../ResourceLoaderUserModule.php | 8 +- .../ResourceLoaderUserOptionsModule.php | 5 +- 8 files changed, 90 insertions(+), 57 deletions(-) diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php index 52520d605b..93f2b1bdb3 100644 --- a/includes/resourceloader/ResourceLoader.php +++ b/includes/resourceloader/ResourceLoader.php @@ -20,8 +20,6 @@ * @author Trevor Parscal */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Dynamic JavaScript and CSS resource loading system. * @@ -40,10 +38,12 @@ class ResourceLoader { /** * Loads information stored in the database about modules. * - * This method grabs modules dependencies from the database and updates modules objects. + * This method grabs modules dependencies from the database and updates modules + * objects. * - * This is not inside the module code because it's so much more performant to request all of the information at once - * than it is to have each module requests its own information. This sacrifice of modularity yields a profound + * This is not inside the module code because it's so much more performant to + * request all of the information at once than it is to have each module + * requests its own information. This sacrifice of modularity yields a profound * performance improvement. * * @param $modules Array: list of module names to preload information for @@ -111,7 +111,8 @@ class ResourceLoader { * - minify-css \see CSSMin::minify * - flip-css \see CSSJanus::transform * - * If $data is empty, only contains whitespace or the filter was unknown, $data is returned unmodified. + * If $data is empty, only contains whitespace or the filter was unknown, + * $data is returned unmodified. * * @param $filter String: name of filter to run * @param $data String: text to filter, such as JavaScript or CSS text @@ -122,17 +123,21 @@ class ResourceLoader { wfProfileIn( __METHOD__ ); - // For empty/whitespace-only data or for unknown filters, don't perform any caching or processing - if ( trim( $data ) === '' || !in_array( $filter, array( 'minify-js', 'minify-css', 'flip-css' ) ) ) { + // For empty/whitespace-only data or for unknown filters, don't perform + // any caching or processing + if ( trim( $data ) === '' + || !in_array( $filter, array( 'minify-js', 'minify-css', 'flip-css' ) ) ) + { wfProfileOut( __METHOD__ ); return $data; } // Try for Memcached hit $key = wfMemcKey( 'resourceloader', 'filter', $filter, md5( $data ) ); - if ( is_string( $cache = $wgMemc->get( $key ) ) ) { + $cacheEntry = $wgMemc->get( $key ); + if ( is_string( $cacheEntry ) ) { wfProfileOut( __METHOD__ ); - return $cache; + return $cacheEntry; } // Run the filter - we've already verified one of these will work @@ -149,7 +154,8 @@ class ResourceLoader { break; } } catch ( Exception $exception ) { - throw new MWException( 'ResourceLoader filter error. Exception was thrown: ' . $exception->getMessage() ); + throw new MWException( 'ResourceLoader filter error. ' . + 'Exception was thrown: ' . $exception->getMessage() ); } // Save filtered text to Memcached @@ -182,10 +188,13 @@ class ResourceLoader { * Registers a module with the ResourceLoader system. * * @param $name Mixed: string of name of module or array of name/object pairs - * @param $object ResourceLoaderModule: module object (optional when using multiple-registration calling style) + * @param $object ResourceLoaderModule: module object (optional when using + * multiple-registration calling style) * @throws MWException If a duplicate module registration is attempted - * @throws MWException If something other than a ResourceLoaderModule is being registered - * @return Boolean: false if there were any errors, in which case one or more modules were not registered + * @throws MWException If something other than a ResourceLoaderModule is being + * registered + * @return Boolean: false if there were any errors, in which case one or more + * modules were not registered */ public function register( $name, ResourceLoaderModule $object = null ) { @@ -206,13 +215,15 @@ class ResourceLoader { if ( isset( $this->modules[$name] ) ) { // A module has already been registered by this name throw new MWException( - 'ResourceLoader duplicate registration error. Another module has already been registered as ' . $name + 'ResourceLoader duplicate registration error. ' . + 'Another module has already been registered as ' . $name ); } // Validate the input (type hinting lets null through) if ( !( $object instanceof ResourceLoaderModule ) ) { - throw new MWException( 'ResourceLoader invalid module error. Instances of ResourceLoaderModule expected.' ); + throw new MWException( 'ResourceLoader invalid module error. ' . + 'Instances of ResourceLoaderModule expected.' ); } // Attach module @@ -262,12 +273,14 @@ class ResourceLoader { } } - // If a version wasn't specified we need a shorter expiry time for updates to propagate to clients quickly + // If a version wasn't specified we need a shorter expiry time for updates + // to propagate to clients quickly if ( is_null( $context->getVersion() ) ) { $maxage = $wgResourceLoaderMaxage['unversioned']['client']; $smaxage = $wgResourceLoaderMaxage['unversioned']['server']; } - // If a version was specified we can use a longer expiry time since changing version numbers causes cache misses + // If a version was specified we can use a longer expiry time since changing + // version numbers causes cache misses else { $maxage = $wgResourceLoaderMaxage['versioned']['client']; $smaxage = $wgResourceLoaderMaxage['versioned']['server']; @@ -278,7 +291,8 @@ class ResourceLoader { wfProfileIn( __METHOD__.'-getModifiedTime' ); - // To send Last-Modified and support If-Modified-Since, we need to detect the last modified time + // To send Last-Modified and support If-Modified-Since, we need to detect + // the last modified time $mtime = wfTimestamp( TS_UNIX, $wgCacheEpoch ); foreach ( $modules as $module ) { // Bypass squid cache if the request includes any private modules @@ -291,7 +305,11 @@ class ResourceLoader { wfProfileOut( __METHOD__.'-getModifiedTime' ); - header( 'Content-Type: ' . ( $context->getOnly() === 'styles' ? 'text/css' : 'text/javascript' ) ); + if ( $context->getOnly() === 'styles' ) { + header( 'Content-Type: text/css' ); + } else { + header( 'Content-Type: text/javascript' ); + } header( 'Last-Modified: ' . wfTimestamp( TS_RFC2822, $mtime ) ); if ( $context->getDebug() ) { header( 'Cache-Control: must-revalidate' ); @@ -332,10 +350,15 @@ class ResourceLoader { * @param $missing Array: list of unavailable modules (optional) * @return String: response data */ - public function makeModuleResponse( ResourceLoaderContext $context, array $modules, $missing = array() ) { + public function makeModuleResponse( ResourceLoaderContext $context, + array $modules, $missing = array() ) + { // Pre-fetch blobs - $blobs = $context->shouldIncludeMessages() ? - MessageBlobStore::get( $this, $modules, $context->getLanguage() ) : array(); + if ( $context->shouldIncludeMessages() ) { + $blobs = MessageBlobStore::get( $this, $modules, $context->getLanguage() ); + } else { + $blobs = array(); + } // Generate output $out = ''; @@ -351,9 +374,10 @@ class ResourceLoader { // Styles $styles = array(); - if ( $context->shouldIncludeStyles() && ( count( $styles = $module->getStyles( $context ) ) ) ) { + if ( $context->shouldIncludeStyles() ) { + $styles = $module->getStyles( $context ); // Flip CSS on a per-module basis - if ( $this->modules[$name]->getFlip( $context ) ) { + if ( $styles && $this->modules[$name]->getFlip( $context ) ) { foreach ( $styles as $media => $style ) { $styles[$media] = $this->filter( 'flip-css', $style ); } @@ -375,7 +399,8 @@ class ResourceLoader { $out .= self::makeMessageSetScript( $messages ); break; default: - // Minify CSS before embedding in mediaWiki.loader.implement call (unless in debug mode) + // Minify CSS before embedding in mediaWiki.loader.implement call + // (unless in debug mode) if ( !$context->getDebug() ) { foreach ( $styles as $media => $style ) { $styles[$media] = $this->filter( 'minify-css', $style ); @@ -391,8 +416,11 @@ class ResourceLoader { // Update module states if ( $context->shouldIncludeScripts() ) { // Set the state of modules loaded as only scripts to ready - if ( count( $modules ) && $context->getOnly() === 'scripts' && !isset( $modules['startup'] ) ) { - $out .= self::makeLoaderStateScript( array_fill_keys( array_keys( $modules ), 'ready' ) ); + if ( count( $modules ) && $context->getOnly() === 'scripts' + && !isset( $modules['startup'] ) ) + { + $out .= self::makeLoaderStateScript( + array_fill_keys( array_keys( $modules ), 'ready' ) ); } // Set the state of modules which were requested but unavailable as missing if ( is_array( $missing ) && count( $missing ) ) { @@ -462,7 +490,9 @@ class ResourceLoader { "( '$name', $version, $dependencies, $group );\n"; } - public static function makeLoaderRegisterScript( $name, $version = null, $dependencies = null, $group = null ) { + public static function makeLoaderRegisterScript( $name, $version = null, + $dependencies = null, $group = null ) + { if ( is_array( $name ) ) { $registrations = FormatJson::encode( $name ); return "mediaWiki.loader.register( $registrations );\n"; diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php index 594d4c07b5..8a009e2277 100644 --- a/includes/resourceloader/ResourceLoaderContext.php +++ b/includes/resourceloader/ResourceLoaderContext.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Object passed around to modules which contains information about the state * of a specific loader request diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php index 81c0f42b26..bff4812175 100644 --- a/includes/resourceloader/ResourceLoaderFileModule.php +++ b/includes/resourceloader/ResourceLoaderFileModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * ResourceLoader module based on local JavaScript/CSS files. */ diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 30af8f08f2..797fca986a 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Abstraction for resource loader modules, with name registration and maxage functionality. */ @@ -160,9 +158,11 @@ abstract class ResourceLoaderModule { ), __METHOD__ ); if ( !is_null( $deps ) ) { - return $this->fileDeps[$skin] = (array) FormatJson::decode( $deps, true ); + $this->fileDeps[$skin] = (array) FormatJson::decode( $deps, true ); + } else { + $this->fileDeps[$skin] = array(); } - return $this->fileDeps[$skin] = array(); + return $this->fileDeps[$skin]; } /** diff --git a/includes/resourceloader/ResourceLoaderSiteModule.php b/includes/resourceloader/ResourceLoaderSiteModule.php index 3225670293..44e018a0a9 100644 --- a/includes/resourceloader/ResourceLoaderSiteModule.php +++ b/includes/resourceloader/ResourceLoaderSiteModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Module for site customizations */ @@ -45,7 +43,10 @@ class ResourceLoaderSiteModule extends ResourceLoaderWikiModule { 'Print.css' => array( 'ns' => NS_MEDIAWIKI, 'type' => 'style', 'media' => 'print' ), ); if ( $wgHandheldStyle ) { - $pages['Handheld.css'] = array( 'ns' => NS_MEDIAWIKI, 'type' => 'style', 'media' => 'handheld' ); + $pages['Handheld.css'] = array( + 'ns' => NS_MEDIAWIKI, + 'type' => 'style', + 'media' => 'handheld' ); } return $pages; } diff --git a/includes/resourceloader/ResourceLoaderStartUpModule.php b/includes/resourceloader/ResourceLoaderStartUpModule.php index 2f52f42700..0229f61440 100644 --- a/includes/resourceloader/ResourceLoaderStartUpModule.php +++ b/includes/resourceloader/ResourceLoaderStartUpModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - class ResourceLoaderStartUpModule extends ResourceLoaderModule { /* Protected Members */ @@ -107,7 +105,8 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { if ( ( $loader = $module->getLoaderScript() ) !== false ) { $deps = $module->getDependencies(); $group = $module->getGroup(); - $version = wfTimestamp( TS_ISO_8601_BASIC, round( $module->getModifiedTime( $context ), -2 ) ); + $version = wfTimestamp( TS_ISO_8601_BASIC, + round( $module->getModifiedTime( $context ), -2 ) ); $out .= ResourceLoader::makeCustomLoaderScript( $name, $version, $deps, $group, $loader ); } // Automatically register module @@ -118,8 +117,8 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { if ( !count( $module->getDependencies() && $module->getGroup() === null ) ) { $registrations[] = array( $name, $mtime ); } - // Modules with dependencies but no group pass three arguments (name, timestamp, dependencies) - // to mediaWiki.loader.register() + // Modules with dependencies but no group pass three arguments + // (name, timestamp, dependencies) to mediaWiki.loader.register() else if ( $module->getGroup() === null ) { $registrations[] = array( $name, $mtime, $module->getDependencies() ); @@ -163,11 +162,18 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { // Startup function $configuration = FormatJson::encode( $this->getConfig( $context ) ); $registrations = self::getModuleRegistrations( $context ); - $out .= "var startUp = function() {\n\t$registrations\n\tmediaWiki.config.set( $configuration );\n};"; + $out .= "var startUp = function() {\n" . + "\t$registrations\n" . + "\tmediaWiki.config.set( $configuration );" . + "\n};"; // Conditional script injection - $scriptTag = Xml::escapeJsString( Html::linkedScript( $wgLoadScript . '?' . wfArrayToCGI( $query ) ) ); - $out .= "if ( isCompatible() ) {\n\tdocument.write( '$scriptTag' );\n}\ndelete isCompatible;"; + $scriptTag = Xml::escapeJsString( + Html::linkedScript( $wgLoadScript . '?' . wfArrayToCGI( $query ) ) ); + $out .= "if ( isCompatible() ) {\n" . + "\tdocument.write( '$scriptTag' );\n" . + "}\n" . + "delete isCompatible;"; } return $out; @@ -182,8 +188,9 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule { } $this->modifiedTime[$hash] = filemtime( "$IP/resources/startup.js" ); - // ATTENTION!: Because of the line above, this is not going to cause infinite recursion - think carefully - // before making changes to this code! + // ATTENTION!: Because of the line above, this is not going to cause + // infinite recursion - think carefully before making changes to this + // code! $time = wfTimestamp( TS_UNIX, $wgCacheEpoch ); foreach ( $context->getResourceLoader()->getModules() as $module ) { $time = max( $time, $module->getModifiedTime( $context ) ); diff --git a/includes/resourceloader/ResourceLoaderUserModule.php b/includes/resourceloader/ResourceLoaderUserModule.php index 2c82f75b32..cb974f74c2 100644 --- a/includes/resourceloader/ResourceLoaderUserModule.php +++ b/includes/resourceloader/ResourceLoaderUserModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Module for user customizations */ @@ -36,9 +34,11 @@ class ResourceLoaderUserModule extends ResourceLoaderWikiModule { $username = $context->getUser(); return array( "$username/common.js" => array( 'ns' => NS_USER, 'type' => 'script' ), - "$username/" . $context->getSkin() . '.js' => array( 'ns' => NS_USER, 'type' => 'script' ), + "$username/" . $context->getSkin() . '.js' => + array( 'ns' => NS_USER, 'type' => 'script' ), "$username/common.css" => array( 'ns' => NS_USER, 'type' => 'style' ), - "$username/" . $context->getSkin() . '.css' => array( 'ns' => NS_USER, 'type' => 'style' ), + "$username/" . $context->getSkin() . '.css' => + array( 'ns' => NS_USER, 'type' => 'style' ), ); } return array(); diff --git a/includes/resourceloader/ResourceLoaderUserOptionsModule.php b/includes/resourceloader/ResourceLoaderUserOptionsModule.php index 3b13ccc489..ceae2240d2 100644 --- a/includes/resourceloader/ResourceLoaderUserOptionsModule.php +++ b/includes/resourceloader/ResourceLoaderUserOptionsModule.php @@ -20,8 +20,6 @@ * @author Roan Kattouw */ -defined( 'MEDIAWIKI' ) || die( 1 ); - /** * Module for user preference customizations */ @@ -80,7 +78,8 @@ class ResourceLoaderUserOptionsModule extends ResourceLoaderModule { // Build CSS rules $rules = array(); if ( $options['underline'] < 2 ) { - $rules[] = "a { text-decoration: " . ( $options['underline'] ? 'underline' : 'none' ) . "; }"; + $rules[] = "a { text-decoration: " . + ( $options['underline'] ? 'underline' : 'none' ) . "; }"; } if ( $options['highlightbroken'] ) { $rules[] = "a.new, #quickbar a.new { color: #ba0000; }\n"; -- 2.20.1