From: Brad Jorsch Date: Fri, 30 Jun 2017 18:19:38 +0000 (-0400) Subject: resourceloader: Add ResourceLoaderModule::shouldEmbedModule and use it X-Git-Tag: 1.31.0-rc.0~2642^2 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/?a=commitdiff_plain;h=f6466732bbdc289da6c81efcb551d53ccc04d1ef;p=lhc%2Fweb%2Fwiklou.git resourceloader: Add ResourceLoaderModule::shouldEmbedModule and use it Rather than only the 'private' group triggering embedding, allow modules to explicitly specify if they should be embedded. The default is still to only embed when the group is 'private', and the 'private' group is still special in that ResourceLoader::respond() will still refuse to serve it from load.php. Change-Id: Ib9a043c566822e278baecc15e87f9c5cebc2eb98 --- diff --git a/includes/resourceloader/ResourceLoaderClientHtml.php b/includes/resourceloader/ResourceLoaderClientHtml.php index 197ac511d1..06f9841d44 100644 --- a/includes/resourceloader/ResourceLoaderClientHtml.php +++ b/includes/resourceloader/ResourceLoaderClientHtml.php @@ -149,9 +149,7 @@ class ResourceLoaderClientHtml { continue; } - $group = $module->getGroup(); - - if ( $group === 'private' ) { + if ( $module->shouldEmbedModule( $this->context ) ) { // Embed via mw.loader.implement per T36907. $data['embed']['general'][] = $name; // Avoid duplicate request from mw.loader @@ -186,7 +184,7 @@ class ResourceLoaderClientHtml { // Avoid needless request for empty module $data['states'][$name] = 'ready'; } else { - if ( $group === 'private' ) { + if ( $module->shouldEmbedModule( $this->context ) ) { // Embed via style element $data['embed']['styles'][] = $name; // Avoid duplicate request from mw.loader @@ -392,62 +390,75 @@ class ResourceLoaderClientHtml { foreach ( $sortedModules as $source => $groups ) { foreach ( $groups as $group => $grpModules ) { $context = self::makeContext( $mainContext, $group, $only, $extraQuery ); - $context->setModules( array_keys( $grpModules ) ); - - if ( $group === 'private' ) { - // Decide whether to use style or script element - if ( $only == ResourceLoaderModule::TYPE_STYLES ) { - $chunks[] = Html::inlineStyle( - $rl->makeModuleResponse( $context, $grpModules ) - ); - } else { - $chunks[] = ResourceLoader::makeInlineScript( - $rl->makeModuleResponse( $context, $grpModules ) - ); - } - continue; - } - - // See if we have one or more raw modules - $isRaw = false; - foreach ( $grpModules as $key => $module ) { - $isRaw |= $module->isRaw(); - } - // Special handling for the user group; because users might change their stuff - // on-wiki like user pages, or user preferences; we need to find the highest - // timestamp of these user-changeable modules so we can ensure cache misses on change - // This should NOT be done for the site group (T29564) because anons get that too - // and we shouldn't be putting timestamps in CDN-cached HTML - if ( $group === 'user' ) { - // Must setModules() before makeVersionQuery() - $context->setVersion( $rl->makeVersionQuery( $context ) ); + // Separate sets of linked and embedded modules while preserving order + $moduleSets = []; + $idx = -1; + foreach ( $grpModules as $name => $module ) { + $shouldEmbed = $module->shouldEmbedModule( $context ); + if ( !$moduleSets || $moduleSets[$idx][0] !== $shouldEmbed ) { + $moduleSets[++$idx] = [ $shouldEmbed, [] ]; + } + $moduleSets[$idx][1][$name] = $module; } - $url = $rl->createLoaderURL( $source, $context, $extraQuery ); - - // Decide whether to use 'style' or 'script' element - if ( $only === ResourceLoaderModule::TYPE_STYLES ) { - $chunk = Html::linkedStyle( $url ); - } else { - if ( $context->getRaw() || $isRaw ) { - $chunk = Html::element( 'script', [ - // In SpecialJavaScriptTest, QUnit must load synchronous - 'async' => !isset( $extraQuery['sync'] ), - 'src' => $url - ] ); + // Link/embed each set + foreach ( $moduleSets as list( $embed, $moduleSet ) ) { + $context->setModules( array_keys( $moduleSet ) ); + if ( $embed ) { + // Decide whether to use style or script element + if ( $only == ResourceLoaderModule::TYPE_STYLES ) { + $chunks[] = Html::inlineStyle( + $rl->makeModuleResponse( $context, $moduleSet ) + ); + } else { + $chunks[] = ResourceLoader::makeInlineScript( + $rl->makeModuleResponse( $context, $moduleSet ) + ); + } } else { - $chunk = ResourceLoader::makeInlineScript( - Xml::encodeJsCall( 'mw.loader.load', [ $url ] ) - ); + // See if we have one or more raw modules + $isRaw = false; + foreach ( $moduleSet as $key => $module ) { + $isRaw |= $module->isRaw(); + } + + // Special handling for the user group; because users might change their stuff + // on-wiki like user pages, or user preferences; we need to find the highest + // timestamp of these user-changeable modules so we can ensure cache misses on change + // This should NOT be done for the site group (T29564) because anons get that too + // and we shouldn't be putting timestamps in CDN-cached HTML + if ( $group === 'user' ) { + // Must setModules() before makeVersionQuery() + $context->setVersion( $rl->makeVersionQuery( $context ) ); + } + + $url = $rl->createLoaderURL( $source, $context, $extraQuery ); + + // Decide whether to use 'style' or 'script' element + if ( $only === ResourceLoaderModule::TYPE_STYLES ) { + $chunk = Html::linkedStyle( $url ); + } else { + if ( $context->getRaw() || $isRaw ) { + $chunk = Html::element( 'script', [ + // In SpecialJavaScriptTest, QUnit must load synchronous + 'async' => !isset( $extraQuery['sync'] ), + 'src' => $url + ] ); + } else { + $chunk = ResourceLoader::makeInlineScript( + Xml::encodeJsCall( 'mw.loader.load', [ $url ] ) + ); + } + } + + if ( $group == 'noscript' ) { + $chunks[] = Html::rawElement( 'noscript', [], $chunk ); + } else { + $chunks[] = $chunk; + } } } - - if ( $group == 'noscript' ) { - $chunks[] = Html::rawElement( 'noscript', [], $chunk ); - } else { - $chunks[] = $chunk; - } } } diff --git a/includes/resourceloader/ResourceLoaderModule.php b/includes/resourceloader/ResourceLoaderModule.php index 1c767fa5b8..16089019dc 100644 --- a/includes/resourceloader/ResourceLoaderModule.php +++ b/includes/resourceloader/ResourceLoaderModule.php @@ -918,6 +918,20 @@ abstract class ResourceLoaderModule implements LoggerAwareInterface { return false; } + /** + * Check whether this module should be embeded rather than linked + * + * Modules returning true here will be embedded rather than loaded by + * ResourceLoaderClientHtml. + * + * @since 1.30 + * @param ResourceLoaderContext $context + * @return bool + */ + public function shouldEmbedModule( ResourceLoaderContext $context ) { + return $this->getGroup() === 'private'; + } + /** @var JSParser Lazy-initialized; use self::javaScriptParser() */ private static $jsParser; private static $parseCacheVersion = 1; diff --git a/tests/phpunit/ResourceLoaderTestCase.php b/tests/phpunit/ResourceLoaderTestCase.php index a8a8f4d730..d8f89fb6bc 100644 --- a/tests/phpunit/ResourceLoaderTestCase.php +++ b/tests/phpunit/ResourceLoaderTestCase.php @@ -94,6 +94,7 @@ class ResourceLoaderTestModule extends ResourceLoaderModule { protected $isKnownEmpty = false; protected $type = ResourceLoaderModule::LOAD_GENERAL; protected $targets = [ 'phpunit' ]; + protected $shouldEmbed = null; public function __construct( $options = [] ) { foreach ( $options as $key => $value ) { @@ -143,6 +144,10 @@ class ResourceLoaderTestModule extends ResourceLoaderModule { return $this->isKnownEmpty; } + public function shouldEmbedModule( ResourceLoaderContext $context ) { + return $this->shouldEmbed !== null ? $this->shouldEmbed : parent::shouldEmbedModule( $context ); + } + public function enableModuleContentVersion() { return true; } diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php index 3e0d883c60..3530d3c1d3 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php @@ -43,6 +43,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'test.top' => [ 'position' => 'top' ], 'test.private.top' => [ 'group' => 'private', 'position' => 'top' ], 'test.private.bottom' => [ 'group' => 'private', 'position' => 'bottom' ], + 'test.shouldembed' => [ 'shouldEmbed' => true ], 'test.styles.pure' => [ 'type' => ResourceLoaderModule::LOAD_STYLES ], 'test.styles.mixed' => [], @@ -64,12 +65,24 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'group' => 'private', 'styles' => '.private{}', ], + 'test.styles.shouldembed' => [ + 'type' => ResourceLoaderModule::LOAD_STYLES, + 'shouldEmbed' => true, + 'styles' => '.shouldembed{}', + ], 'test.scripts' => [], 'test.scripts.top' => [ 'position' => 'top' ], 'test.scripts.user' => [ 'group' => 'user' ], 'test.scripts.user.empty' => [ 'group' => 'user', 'isKnownEmpty' => true ], 'test.scripts.raw' => [ 'isRaw' => true ], + 'test.scripts.shouldembed' => [ 'shouldEmbed' => true ], + + 'test.ordering.a' => [ 'shouldEmbed' => false ], + 'test.ordering.b' => [ 'shouldEmbed' => false ], + 'test.ordering.c' => [ 'shouldEmbed' => true, 'styles' => '.orderingC{}' ], + 'test.ordering.d' => [ 'shouldEmbed' => true, 'styles' => '.orderingD{}' ], + 'test.ordering.e' => [ 'shouldEmbed' => false ], ]; return array_map( function ( $options ) { return self::makeModule( $options ); @@ -102,6 +115,7 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'test.private.bottom', 'test.private.top', 'test.top', + 'test.shouldembed', 'test.unregistered', ] ); $client->setModuleStyles( [ @@ -109,12 +123,14 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'test.styles.user.empty', 'test.styles.private', 'test.styles.pure', + 'test.styles.shouldembed', 'test.unregistered.styles', ] ); $client->setModuleScripts( [ 'test.scripts', 'test.scripts.user.empty', 'test.scripts.top', + 'test.scripts.shouldembed', 'test.unregistered.scripts', ] ); @@ -122,12 +138,15 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'states' => [ 'test.private.top' => 'loading', 'test.private.bottom' => 'loading', + 'test.shouldembed' => 'loading', 'test.styles.pure' => 'ready', 'test.styles.user.empty' => 'ready', 'test.styles.private' => 'ready', + 'test.styles.shouldembed' => 'ready', 'test.scripts' => 'loading', 'test.scripts.top' => 'loading', 'test.scripts.user.empty' => 'ready', + 'test.scripts.shouldembed' => 'loading', ], 'general' => [ 'test', @@ -139,12 +158,14 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'scripts' => [ 'test.scripts', 'test.scripts.top', + 'test.scripts.shouldembed', ], 'embed' => [ - 'styles' => [ 'test.styles.private' ], + 'styles' => [ 'test.styles.private', 'test.styles.shouldembed' ], 'general' => [ 'test.private.bottom', 'test.private.top', + 'test.shouldembed', ], ], ]; @@ -276,6 +297,47 @@ class ResourceLoaderClientHtmlTest extends PHPUnit_Framework_TestCase { 'only' => ResourceLoaderModule::TYPE_STYLES, 'output' => '', ], + [ + 'context' => [], + 'modules' => [ 'test.shouldembed' ], + 'only' => ResourceLoaderModule::TYPE_COMBINED, + 'output' => '', + ], + [ + 'context' => [], + 'modules' => [ 'test.styles.shouldembed' ], + 'only' => ResourceLoaderModule::TYPE_STYLES, + 'output' => '', + ], + [ + 'context' => [], + 'modules' => [ 'test.scripts.shouldembed' ], + 'only' => ResourceLoaderModule::TYPE_SCRIPTS, + 'output' => '', + ], + [ + 'context' => [], + 'modules' => [ 'test', 'test.shouldembed' ], + 'only' => ResourceLoaderModule::TYPE_COMBINED, + 'output' => '', + ], + [ + 'context' => [], + 'modules' => [ 'test.styles.pure', 'test.styles.shouldembed' ], + 'only' => ResourceLoaderModule::TYPE_STYLES, + 'output' => + '' . "\n" + . '' + ], + [ + 'context' => [], + 'modules' => [ 'test.ordering.a', 'test.ordering.e', 'test.ordering.b', 'test.ordering.d', 'test.ordering.c' ], + 'only' => ResourceLoaderModule::TYPE_STYLES, + 'output' => + '' . "\n" + . '' . "\n" + . '' + ], // @codingStandardsIgnoreEnd ]; }