r82518 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r82517‎ | r82518 | r82519 >
Date:17:24, 20 February 2011
Author:catrope
Status:ok
Tags:
Comment:
(bug 27468) Move private styles out to their own section, so they override everything except user styles. Moved from one makeResourceLoaderLink() call with array_merge() to multiple calls, because makeResourceLoaderLink() orders the modules, then groups them, so the input order is not preserved. Also fixed an issue in makeResourceLoaderLink() where the $uery variable was reused in each iteration but not cleared, allowing pollution to occur in theory (doesn't seem to have happened in practice, though).
Modified paths:
  • /trunk/phase3/includes/OutputPage.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/OutputPage.php
@@ -2404,7 +2404,7 @@
24052405 // Lazy-load ResourceLoader
24062406 // TODO: Should this be a static function of ResourceLoader instead?
24072407 // TODO: Divide off modules starting with "user", and add the user parameter to them
2408 - $query = array(
 2408+ $baseQuery = array(
24092409 'lang' => $wgLang->getCode(),
24102410 'debug' => ResourceLoader::inDebugMode() ? 'true' : 'false',
24112411 'skin' => $skin->getSkinName(),
@@ -2412,10 +2412,10 @@
24132413 );
24142414 // Propagate printable and handheld parameters if present
24152415 if ( $this->isPrintable() ) {
2416 - $query['printable'] = 1;
 2416+ $baseQuery['printable'] = 1;
24172417 }
24182418 if ( $wgRequest->getBool( 'handheld' ) ) {
2419 - $query['handheld'] = 1;
 2419+ $baseQuery['handheld'] = 1;
24202420 }
24212421
24222422 if ( !count( $modules ) ) {
@@ -2444,7 +2444,7 @@
24452445 foreach ( (array) $modules as $name ) {
24462446 $module = $resourceLoader->getModule( $name );
24472447 # Check that we're allowed to include this module on this page
2448 - if( ( $module->getOrigin() > $this->getAllowedModules( ResourceLoaderModule::TYPE_SCRIPTS )
 2448+ if ( ( $module->getOrigin() > $this->getAllowedModules( ResourceLoaderModule::TYPE_SCRIPTS )
24492449 && $only == ResourceLoaderModule::TYPE_SCRIPTS )
24502450 || ( $module->getOrigin() > $this->getAllowedModules( ResourceLoaderModule::TYPE_STYLES )
24512451 && $only == ResourceLoaderModule::TYPE_STYLES )
@@ -2462,6 +2462,7 @@
24632463
24642464 $links = '';
24652465 foreach ( $groups as $group => $modules ) {
 2466+ $query = $baseQuery;
24662467 // Special handling for user-specific groups
24672468 if ( ( $group === 'user' || $group === 'private' ) && $wgUser->isLoggedIn() ) {
24682469 $query['user'] = $wgUser->getName();
@@ -2816,19 +2817,19 @@
28172818 public function buildCssLinks( $sk ) {
28182819 $ret = '';
28192820 // Add ResourceLoader styles
2820 - // Split the styles into three groups
2821 - $styles = array( 'other' => array(), 'user' => array(), 'site' => array() );
 2821+ // Split the styles into four groups
 2822+ $styles = array( 'other' => array(), 'user' => array(), 'site' => array(), 'private' => array() );
28222823 $resourceLoader = $this->getResourceLoader();
28232824 foreach ( $this->getModuleStyles() as $name ) {
28242825 $group = $resourceLoader->getModule( $name )->getGroup();
2825 - // Modules in groups named "other" or anything different than "user" or "site" will
2826 - // be placed in the "other" group
 2826+ // Modules in groups named "other" or anything different than "user", "site" or "private"
 2827+ // will be placed in the "other" group
28272828 $styles[isset( $styles[$group] ) ? $group : 'other'][] = $name;
28282829 }
28292830
2830 - // We want site and user styles to override dynamically added styles from modules, but we want
 2831+ // We want site, private and user styles to override dynamically added styles from modules, but we want
28312832 // dynamically added styles to override statically added styles from other modules. So the order
2832 - // has to be other, dynamic, site, user
 2833+ // has to be other, dynamic, site, private, user
28332834 // Add statically added styles for other modules
28342835 $ret .= $this->makeResourceLoaderLink( $sk, $styles['other'], ResourceLoaderModule::TYPE_STYLES );
28352836 // Add normal styles added through addStyle()/addInlineStyle() here
@@ -2836,10 +2837,15 @@
28372838 // Add marker tag to mark the place where the client-side loader should inject dynamic styles
28382839 // We use a <meta> tag with a made-up name for this because that's valid HTML
28392840 $ret .= Html::element( 'meta', array( 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ) );
2840 - // Add site and user styles
2841 - $ret .= $this->makeResourceLoaderLink(
2842 - $sk, array_merge( $styles['site'], $styles['user'] ), ResourceLoaderModule::TYPE_STYLES
2843 - );
 2841+
 2842+ // Add site, private and user styles
 2843+ // 'private' at present only contains user.options, so put that before 'user'
 2844+ // Any future private modules will likely have a similar user-specific character
 2845+ foreach ( array( 'site', 'private', 'user' ) as $group ) {
 2846+ $ret .= $this->makeResourceLoaderLink( $sk, $styles[$group],
 2847+ ResourceLoaderModule::TYPE_STYLES
 2848+ );
 2849+ }
28442850 return $ret;
28452851 }
28462852

Sign-offs

UserFlagDate
Happy-meloninspected13:22, 10 August 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r825331.17wmf1: MFT r78893, r78897, r78909, r82404, r82408, r82409, r82453, r82456,...catrope20:13, 20 February 2011
r85354MFT r82518, r82530, r82538, r82547, r82550, r82565, r82572, r82608, r82696, r...demon18:25, 4 April 2011

Status & tagging log