From a69880e05cfd5197862af2c248177a5b3bef9aee Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Thu, 27 Jun 2019 00:20:33 +0100 Subject: [PATCH] resourceloader: Only output ResourceLoaderDynamicStyles when needed In mediawiki.js, this marker has always been optional, falling back to appending to . When no stylesheets need to be after the marker (e.g. no site styles on the wiki, and user is not logged-in), then there is no need for the marker to exist. In a previous refactor, I was going to do this and created an "$append" variable in the function to do what this commit does, but I forgot to actually use it for anything. Test Plan: * Local wiki, with no MediaWiki:Common/{Skinname}.css pages existing. * When logged-out, before this change, there is a marker, now there is not. * When creating "MediaWiki:Group-user.css" and logging in, there is still a marker, and it is still above the for that user styles request in the . Bug: T219342 Change-Id: I2e9657f318088860916823efeb96ae4f1532974c --- includes/OutputPage.php | 61 +++++++++++++---------- tests/phpunit/includes/OutputPageTest.php | 4 +- 2 files changed, 38 insertions(+), 27 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index 28e0a31352..58b025a655 100644 --- a/includes/OutputPage.php +++ b/includes/OutputPage.php @@ -3712,43 +3712,54 @@ class OutputPage extends ContextSource { */ protected function buildExemptModules() { $chunks = []; - // Things that go after the ResourceLoaderDynamicStyles marker - $append = []; - // We want site, private and user styles to override dynamically added styles from - // general modules, but we want dynamically added styles to override statically added - // style modules. So the order has to be: - // - page style modules (formatted by ResourceLoaderClientHtml::getHeadHtml()) - // - dynamically loaded styles (added by mw.loader before ResourceLoaderDynamicStyles) - // - ResourceLoaderDynamicStyles marker - // - site/private/user styles + // Requirements: + // - Within modules provided by the software (core, skin, extensions), + // styles from skin stylesheets should be overridden by styles + // from modules dynamically loaded with JavaScript. + // - Styles from site-specific, private, and user modules should override + // both of the above. + // + // The effective order for stylesheets must thus be: + // 1. Page style modules, formatted server-side by ResourceLoaderClientHtml. + // 2. Dynamically-loaded styles, inserted client-side by mw.loader. + // 3. Styles that are site-specific, private or from the user, formatted + // server-side by this function. + // + // The 'ResourceLoaderDynamicStyles' marker helps JavaScript know where + // point #2 is. // Add legacy styles added through addStyle()/addInlineStyle() here $chunks[] = implode( '', $this->buildCssLinksArray() ) . $this->mInlineStyles; - $chunks[] = Html::element( - 'meta', - [ 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ] - ); - + // Things that go after the ResourceLoaderDynamicStyles marker + $append = []; $separateReq = [ 'site.styles', 'user.styles' ]; foreach ( $this->rlExemptStyleModules as $group => $moduleNames ) { - // Combinable modules - $chunks[] = $this->makeResourceLoaderLink( - array_diff( $moduleNames, $separateReq ), - ResourceLoaderModule::TYPE_STYLES - ); - - foreach ( array_intersect( $moduleNames, $separateReq ) as $name ) { - // These require their own dedicated request in order to support "@import" - // syntax, which is incompatible with concatenation. (T147667, T37562) - $chunks[] = $this->makeResourceLoaderLink( $name, + if ( $moduleNames ) { + $append[] = $this->makeResourceLoaderLink( + array_diff( $moduleNames, $separateReq ), ResourceLoaderModule::TYPE_STYLES ); + + foreach ( array_intersect( $moduleNames, $separateReq ) as $name ) { + // These require their own dedicated request in order to support "@import" + // syntax, which is incompatible with concatenation. (T147667, T37562) + $append[] = $this->makeResourceLoaderLink( $name, + ResourceLoaderModule::TYPE_STYLES + ); + } } } + if ( $append ) { + $chunks[] = Html::element( + 'meta', + [ 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ] + ); + $chunks = array_merge( $chunks, $append ); + } - return self::combineWrappedStrings( array_merge( $chunks, $append ) ); + return self::combineWrappedStrings( $chunks ); } /** diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index 243a90d741..448eec8fd0 100644 --- a/tests/phpunit/includes/OutputPageTest.php +++ b/tests/phpunit/includes/OutputPageTest.php @@ -2674,11 +2674,11 @@ class OutputPageTest extends MediaWikiTestCase { return [ 'empty' => [ 'exemptStyleModules' => [], - '', + '', ], 'empty sets' => [ 'exemptStyleModules' => [ 'site' => [], 'noscript' => [], 'private' => [], 'user' => [] ], - '', + '', ], 'default logged-out' => [ 'exemptStyleModules' => [ 'site' => [ 'site.styles' ] ], -- 2.20.1