From 15ca48adf291e1d0ab7d4c6d5c28fd54c4c994dd Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Fri, 12 May 2017 23:26:36 +0100 Subject: [PATCH] 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 --- includes/OutputPage.php | 13 ++++++++++++- tests/phpunit/includes/OutputPageTest.php | 6 ++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/includes/OutputPage.php b/includes/OutputPage.php index c739b3098f..e22f42ce21 100644 --- 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 cedb6232d0..32fa46887d 100644 --- 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' ], ], '' . "\n" . - '' . "\n" . - '', + '' . "\n" . + '' . "\n" . + '' . "\n" . + '', ], // @codingStandardsIgnoreEnd Generic.Files.LineLength ]; -- 2.20.1