resourceloader: Support loading group=user modules with addModules()

When a module has group=user specified, it means that its module contents
can vary by user. These kinds of requests have two special needs:
1) They need an additional "user" parameter in their load.php request,
   so that the response knows which user-context to use.
2) They need to have their 'version' hash pre-computed based on which assets
   will be loaded for this user. The general 'version' hash associated with
   this module name in the main registry (modules=startup) will be "wrong"
   as that is computed based on logged-out status.

We do this by omitting the module name from the `mw.load.load(Array modules)`
call in the HTML, and instead output a request for the full url.

This currently works fine for most cases, such as the 'user' module loaded
by MediaWiki core. The branch in getData() dealing with legacy 'only=scripts'
behaviour also covers this case.

But the case of an extension registering a group=user module and loading it the
general way (e.g. not with legacy only=scripts behaviour), would currently end
up in the Array-queue and dynamically loaded by the client-side without knowing
the correct version hash. Fortunately, no code exists that I know of that meets
these three critera (extension registered, group=user, non-legacy). However,
for the GlobalCssJs extension to migrate from legacy to non-legacy, they will
need to start doing this. This commit makes sure that that will work.

The makeLoad() method in ClientHtml has code ensuring the full-url form (with
pre-computed 'version' hash) is used for any modules with group=user. Before
this patch, we didn't get to call makeLoad() because getData() was assuming
that we only need makeLoad() when either the module should be embedded (group=private),
or when it is a style/scripts-only module. It didn't consider group=user.

Bug: T188689
Change-Id: Iaab15e5f5c12e7e28b8c81beab90948cd07cd352

diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php
index 545fd3b..6c4a5d0 100644 (file)
--- a/includes/resourceloader/ResourceLoaderClientHtml.php
+++ b/includes/resourceloader/ResourceLoaderClientHtml.php
@@ -148,15 +148,22 @@ class ResourceLoaderClientHtml {
                                continue;
                        }
 
-                       $context = $this->getContext( $module->getGroup(), ResourceLoaderModule::TYPE_COMBINED );
+                       $group = $module->getGroup();
+                       $context = $this->getContext( $group, ResourceLoaderModule::TYPE_COMBINED );
                        if ( $module->isKnownEmpty( $context ) ) {
                                // Avoid needless request or embed for empty module
                                $data['states'][$name] = 'ready';
                                continue;
                        }
 
-                       if ( $module->shouldEmbedModule( $this->context ) ) {
-                               // Embed via mw.loader.implement per T36907.
+                       if ( $group === 'user' || $module->shouldEmbedModule( $this->context ) ) {
+                               // Call makeLoad() to decide how to load these, instead of
+                               // loading via mw.loader.load().
+                               // - For group=user: We need to provide a pre-generated load.php
+                               //   url to the client that has the 'user' and 'version' parameters
+                               //   filled in. Without this, the client would wrongly use the static
+                               //   version hash, per T64602.
+                               // - For shouldEmbed=true:  Embed via mw.loader.implement, per T36907.
                                $data['embed']['general'][] = $name;
                                // Avoid duplicate request from mw.loader
                                $data['states'][$name] = 'loading';

diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php
index 07956f1..ea3d199 100644 (file)
--- a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php
+++ b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php
@@ -45,6 +45,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                        'test.private' => [ 'group' => 'private' ],
                        'test.shouldembed.empty' => [ 'shouldEmbed' => true, 'isKnownEmpty' => true ],
                        'test.shouldembed' => [ 'shouldEmbed' => true ],
+                       'test.user' => [ 'group' => 'user' ],
 
                        'test.styles.pure' => [ 'type' => ResourceLoaderModule::LOAD_STYLES ],
                        'test.styles.mixed' => [],
@@ -115,6 +116,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                        'test.private',
                        'test.shouldembed.empty',
                        'test.shouldembed',
+                       'test.user',
                        'test.unregistered',
                ] );
                $client->setModuleStyles( [
@@ -138,6 +140,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                                'test.private' => 'loading',
                                'test.shouldembed.empty' => 'ready',
                                'test.shouldembed' => 'loading',
+                               'test.user' => 'loading',
                                'test.styles.pure' => 'ready',
                                'test.styles.user.empty' => 'ready',
                                'test.styles.private' => 'ready',
@@ -163,6 +166,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                                'general' => [
                                        'test.private',
                                        'test.shouldembed',
+                                       'test.user',
                                ],
                        ],
                ];
@@ -319,6 +323,12 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                                'only' => ResourceLoaderModule::TYPE_SCRIPTS,
                                'output' => '<script>(window.RLQ=window.RLQ||[]).push(function(){mw.loader.load("/w/load.php?debug=false\u0026lang=nl\u0026modules=test.scripts.user\u0026only=scripts\u0026skin=fallback\u0026user=Example\u0026version=0a56zyi");});</script>',
                        ],
+                       [
+                               'context' => [],
+                               'modules' => [ 'test.user' ],
+                               'only' => ResourceLoaderModule::TYPE_COMBINED,
+                               'output' => '<script>(window.RLQ=window.RLQ||[]).push(function(){mw.loader.load("/w/load.php?debug=false\u0026lang=nl\u0026modules=test.user\u0026skin=fallback\u0026user=Example\u0026version=0a56zyi");});</script>',
+                       ],
                        [
                                'context' => [ 'debug' => true ],
                                'modules' => [ 'test.styles.pure', 'test.styles.mixed' ],