resourceloader: Fix debug mode for RL-to-RL cross-wiki module loads
The native "foreign module source" feature, as used by the GlobalCssJs extension, did not work correctly in debug mode as the urls returned by the remote wiki were formatted as "/w/load.php...", which would be interpreted by the browser relative to the host document, instead of relative to the parent script. For example: 1. Page view on en.wikipedia.org. 2. Script call to meta.wikimedia.org/w/load.php?debug=true&modules=ext.globalCssJs.user&user This URL is formatted by getScriptURLsForDebug on en.wikipedia.org, when building the article HTML. It knows the modules is on Meta, and formats it as such. So far so good. 3. meta.wikimedia.org responds with an array of urls for sub resources. That array contained URLs like "/w/load.php...only=scripts". These were formatted by getScriptURLsForDebug running on Meta, no longer with a reason to make it a Meta-Wiki URL as it isn't perceived as cross-wiki. It is indistinguishable from debugging a Meta-Wiki page view from its perspective. This patch affects scenario 3 by always expanding it relative to the current-request's wgServer. We still only do this in debug mode. There is not yet a need to do this in non-debug mode, and if there was we'd likely want to find a way to avoid it in the common case to keep embedded URLs short. The ResourceLoader::expandUrl() method is similar to the one in Wikimedia\Minify\CSSMin. Test Plan: * view-source:http://mw.localhost:8080/w/load.php?debug=1&modules=site For Module base class. Before, the array entries were relative. After, they are full. * view-source:http://mw.localhost:8080/w/load.php?debug=1&modules=jquery For FileModule. Before, the array entries were relative. After, they are full. * view-source:http://mw.localhost:8080/wiki/Main_Page?debug=true Unchanged. * view-source:http://mw.localhost:8080/wiki/Main_Page Unchanged. Bug: T255367 Change-Id: I83919744b2677c7fb52b84089ecc60b89957d32a
This commit is contained in:
parent
89259d5a61
commit
b99458ceab
5 changed files with 78 additions and 14 deletions
|
|
@ -1877,6 +1877,34 @@ MESSAGE;
|
|||
return $parser;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve a possibly relative URL against a base URL.
|
||||
*
|
||||
* The base URL must have a server and should have a protocol.
|
||||
* A protocol-relative base expands to HTTPS.
|
||||
*
|
||||
* This is a standalone version of MediaWiki's wfExpandUrl (T32956).
|
||||
*
|
||||
* @internal For use by core ResourceLoader classes only
|
||||
* @param string $base
|
||||
* @param string $url
|
||||
* @return string URL
|
||||
*/
|
||||
public function expandUrl( string $base, string $url ): string {
|
||||
// Net_URL2::resolve() doesn't allow protocol-relative URLs, but we do.
|
||||
$isProtoRelative = strpos( $base, '//' ) === 0;
|
||||
if ( $isProtoRelative ) {
|
||||
$base = "https:$base";
|
||||
}
|
||||
// Net_URL2::resolve() takes care of throwing if $base doesn't have a server.
|
||||
$baseUrl = new Net_URL2( $base );
|
||||
$ret = $baseUrl->resolve( $url );
|
||||
if ( $isProtoRelative ) {
|
||||
$ret->setScheme( false );
|
||||
}
|
||||
return $ret->getURL();
|
||||
}
|
||||
|
||||
/**
|
||||
* Get site configuration settings to expose to JavaScript on all pages via `mw.config`.
|
||||
*
|
||||
|
|
|
|||
|
|
@ -385,12 +385,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
|
|||
* @return string[]
|
||||
*/
|
||||
public function getScriptURLsForDebug( ResourceLoaderContext $context ) {
|
||||
$rl = $context->getResourceLoader();
|
||||
$config = $this->getConfig();
|
||||
$server = $config->get( 'Server' );
|
||||
|
||||
$urls = [];
|
||||
foreach ( $this->getScriptFiles( $context ) as $file ) {
|
||||
$urls[] = OutputPage::transformResourcePath(
|
||||
$this->getConfig(),
|
||||
$this->getRemotePath( $file )
|
||||
);
|
||||
$url = OutputPage::transformResourcePath( $config, $this->getRemotePath( $file ) );
|
||||
// Expand debug URL in case we are another wiki's module source (T255367)
|
||||
$url = $rl->expandUrl( $server, $url );
|
||||
$urls[] = $url;
|
||||
}
|
||||
return $urls;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -286,32 +286,40 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface {
|
|||
}
|
||||
|
||||
/**
|
||||
* Get the URL or URLs to load for this module's JS in debug mode.
|
||||
* The default behavior is to return a load.php?only=scripts URL for
|
||||
* the module, but file-based modules will want to override this to
|
||||
* load the files directly.
|
||||
* Get the URLs to load this module's JS code in debug mode.
|
||||
*
|
||||
* This function is called only when 1) we're in debug mode, 2) there
|
||||
* is no only= parameter and 3) supportsURLLoading() returns true.
|
||||
* #2 is important to prevent an infinite loop, therefore this function
|
||||
* MUST return either an only= URL or a non-load.php URL.
|
||||
* The default behavior is to return a `load.php?only=scripts` URL for
|
||||
* the module. File-based modules may override this to load underlying
|
||||
* files directly.
|
||||
*
|
||||
* This function must only be called when:
|
||||
*
|
||||
* 1. We're in debug mode,
|
||||
* 2. there is no `only=` parameter and,
|
||||
* 3. self::supportsURLLoading() returns true.
|
||||
*
|
||||
* Point 2 is prevents an infinite loop, therefore this function
|
||||
* MUST return either an URL with an `only=` query, or a non-load.php URL.
|
||||
*
|
||||
* @stable to override
|
||||
* @param ResourceLoaderContext $context
|
||||
* @return string[]
|
||||
*/
|
||||
public function getScriptURLsForDebug( ResourceLoaderContext $context ) {
|
||||
$resourceLoader = $context->getResourceLoader();
|
||||
$rl = $context->getResourceLoader();
|
||||
$derivative = new DerivativeResourceLoaderContext( $context );
|
||||
$derivative->setModules( [ $this->getName() ] );
|
||||
$derivative->setOnly( 'scripts' );
|
||||
$derivative->setDebug( true );
|
||||
|
||||
$url = $resourceLoader->createLoaderURL(
|
||||
$url = $rl->createLoaderURL(
|
||||
$this->getSource(),
|
||||
$derivative
|
||||
);
|
||||
|
||||
// Expand debug URL in case we are another wiki's module source (T255367)
|
||||
$url = $rl->expandUrl( $this->getConfig()->get( 'Server' ), $url );
|
||||
|
||||
return [ $url ];
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -71,6 +71,8 @@ abstract class ResourceLoaderTestCase extends MediaWikiIntegrationTestCase {
|
|||
'ResourceBasePath' => '/w',
|
||||
'ParserEnableLegacyMediaDOM' => true,
|
||||
|
||||
// For ResourceLoader::expandUrl()
|
||||
'Server' => 'https://example.org',
|
||||
// For ResourceLoaderStartUpModule and ResourceLoader::__construct()
|
||||
'ScriptPath' => '/w',
|
||||
'Script' => '/w/index.php',
|
||||
|
|
|
|||
|
|
@ -167,6 +167,28 @@ class ResourceLoaderFileModuleTest extends ResourceLoaderTestCase {
|
|||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @covers ResourceLoader::expandUrl
|
||||
* @covers ResourceLoaderFileModule
|
||||
*/
|
||||
public function testGetScriptURLsForDebug() {
|
||||
$ctx = $this->getResourceLoaderContext();
|
||||
$module = new ResourceLoaderFileModule( [
|
||||
'localBasePath' => __DIR__ . '/../../data/resourceloader',
|
||||
'remoteBasePath' => '/w/something',
|
||||
'scripts' => [ 'script-comment.js' ],
|
||||
] );
|
||||
$module->setName( 'testing' );
|
||||
$module->setConfig( $ctx->getResourceLoader()->getConfig() );
|
||||
|
||||
$this->assertEquals(
|
||||
[
|
||||
'https://example.org/w/something/script-comment.js'
|
||||
],
|
||||
$module->getScriptURLsForDebug( $ctx )
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* @covers ResourceLoaderFileModule
|
||||
*/
|
||||
|
|
|
|||
Loading…
Reference in a new issue