resourceloader: Ensure user.styles and site.styles having their own request

Regardless of whether other modules exist with group=user or group=site,
these two modules in particular must always be in their own request for
legacy reasons.

This has already always been the case because even in the few cases where
an extension uses this group (eg. MobileFrontend's custom site module) it
would load it instead of another module in that group, never at the same
time. There is one notable exception, which is GlobalCssJs. However the
ext.globalCssJs.user.styles module is usually served from another wiki
which is why that went unnoticed as well. This commit fixes that so that
even if you're viewing a page on the central wiki, the modules are still
in separate requests.

Aside from this one existing edge case, there is also need to add
group=site to gadgets by default so that they load after the DynamicStyles
marker instead of before, which is currently causing problems with the
cascading order (gadget apply before core and skin styles due to being
in the same request group and alphabetically sorting before them).

Semantically, the appropiate solution is group=site, but this wasn't
possible due to core putting "all" group=site modules in the same request
(under the assumption there is only one such module). This commit removes
that fragile assumption.

Bug: T147667
Change-Id: I9eb725c083124d22a9af3bf3d075ade6f3b970a3

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index c739b30..e22f42c 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -3644,10 +3644,21 @@ class OutputPage extends ContextSource {
                        [ 'name' => 'ResourceLoaderDynamicStyles', 'content' => '' ]
                );
 
+               $separateReq = [ 'site.styles', 'user.styles' ];
                foreach ( $this->rlExemptStyleModules as $group => $moduleNames ) {
-                       $chunks[] = $this->makeResourceLoaderLink( $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,
+                                       ResourceLoaderModule::TYPE_STYLES
+                               );
+                       }
                }
 
                return self::combineWrappedStrings( array_merge( $chunks, $append ) );

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index cedb623..32fa468 100644 (file)
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -372,8 +372,10 @@ class OutputPageTest extends MediaWikiTestCase {
                                        'user' => [ 'user.styles', 'example.user' ],
                                ],
                                '<meta name="ResourceLoaderDynamicStyles" content=""/>' . "\n" .
-                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=example.site.a%2Cb%7Csite.styles&amp;only=styles&amp;skin=fallback"/>' . "\n" .
-                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=example.user%7Cuser.styles&amp;only=styles&amp;skin=fallback&amp;version=17f1vjw"/>',
+                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=example.site.a%2Cb&amp;only=styles&amp;skin=fallback"/>' . "\n" .
+                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=site.styles&amp;only=styles&amp;skin=fallback"/>' . "\n" .
+                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=example.user&amp;only=styles&amp;skin=fallback&amp;version=0a56zyi"/>' . "\n" .
+                               '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=en&amp;modules=user.styles&amp;only=styles&amp;skin=fallback&amp;version=1e9z0ox"/>',
                        ],
                        // @codingStandardsIgnoreEnd Generic.Files.LineLength
                ];