From: Timo Tijhof Date: Tue, 6 Mar 2018 00:52:28 +0000 (-0800) Subject: resourceloader: Support loading group=user modules with addModules() X-Git-Tag: 1.34.0-rc.0~5671^2 X-Git-Url: https://git.cyclocoop.org/%27.%24link.%27?a=commitdiff_plain;h=5ab0dd088861c0c01b5221b109a4e0c54941cbb6;p=lhc%2Fweb%2Fwiklou.git 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 545fd3bda7..6c4a5d0664 100644 --- 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 07956f1d1c..ea3d199efe 100644 --- 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' => '', ], + [ + 'context' => [], + 'modules' => [ 'test.user' ], + 'only' => ResourceLoaderModule::TYPE_COMBINED, + 'output' => '', + ], [ 'context' => [ 'debug' => true ], 'modules' => [ 'test.styles.pure', 'test.styles.mixed' ],