resourceloader: Only output ResourceLoaderDynamicStyles when needed
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 26 Jun 2019 23:20:33 +0000 (00:20 +0100)
committerKrinkle <krinklemail@gmail.com>
Thu, 4 Jul 2019 19:14:39 +0000 (19:14 +0000)
In mediawiki.js, this marker has always been optional, falling back to
appending to <head>. 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 <link> for that user styles request
  in the <head>.

Bug: T219342
Change-Id: I2e9657f318088860916823efeb96ae4f1532974c

includes/OutputPage.php
tests/phpunit/includes/OutputPageTest.php

index 28e0a31..58b025a 100644 (file)
@@ -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 );
        }
 
        /**
index 243a90d..448eec8 100644 (file)
@@ -2674,11 +2674,11 @@ class OutputPageTest extends MediaWikiTestCase {
                return [
                        'empty' => [
                                'exemptStyleModules' => [],
-                               '<meta name="ResourceLoaderDynamicStyles" content=""/>',
+                               '',
                        ],
                        'empty sets' => [
                                'exemptStyleModules' => [ 'site' => [], 'noscript' => [], 'private' => [], 'user' => [] ],
-                               '<meta name="ResourceLoaderDynamicStyles" content=""/>',
+                               '',
                        ],
                        'default logged-out' => [
                                'exemptStyleModules' => [ 'site' => [ 'site.styles' ] ],