Merge "resourceloader: Condition-wrap the HTML tag instead of JS response"

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index af90ca6..22a6012 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -2769,7 +2769,10 @@ $templates
                                );
                                $context = new ResourceLoaderContext( $resourceLoader, new FauxRequest( $query ) );
 
-                               // Extract modules that know they're empty
+
+                               // Extract modules that know they're empty and see if we have one or more
+                               // raw modules
+                               $isRaw = false;
                                foreach ( $grpModules as $key => $module ) {
                                        // Inline empty modules: since they're empty, just mark them as 'ready' (bug 46857)
                                        // If we're only getting the styles, we don't need to do anything for empty modules.
@@ -2779,6 +2782,8 @@ $templates
                                                        $links['states'][$key] = 'ready';
                                                }
                                        }
+
+                                       $isRaw |= $module->isRaw();
                                }
 
                                // If there are no non-empty modules, skip this group
@@ -2845,6 +2850,17 @@ $templates
                                                );
                                        } else {
                                                $link = Html::linkedScript( $url );
+                                               if ( $context->getOnly() === 'scripts' && !$context->getRaw() && !$isRaw ) {
+                                                       // Wrap only=script requests in a conditional as browsers not supported
+                                                       // by the startup module would unconditionally execute this module.
+                                                       // Otherwise users will get "ReferenceError: mw is undefined" or
+                                                       // "jQuery is undefined" from e.g. a "site" module.
+                                                       $link = Html::inlineScript(
+                                                               ResourceLoader::makeLoaderConditionalScript(
+                                                                       Xml::encodeJsCall( 'document.write', array( $link ) )
+                                                               )
+                                                       );
+                                               }
 
                                                // For modules requested directly in the html via <link> or <script>,
                                                // tell mw.loader they are being loading to prevent duplicate requests.

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index a041ad6..4f1414b 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1002,15 +1002,6 @@ class ResourceLoader {
                        if ( count( $states ) ) {
                                $out .= self::makeLoaderStateScript( $states );
                        }
-
-                       if ( $context->getOnly() === 'scripts' ) {
-                               // In only=script requests for modules that are not raw (e.g. not the startup module)
-                               // ensure the execution is conditional to avoid situations where browsers with an
-                               // unsupported environment do unconditionally execute a module's scripts. Otherwise users
-                               // will get things like "ReferenceError: mw is undefined" or "jQuery is undefined" from
-                               // legacy scripts loaded with only=scripts (such as the 'site' module).
-                               $out = self::makeLoaderConditionalScript( $out );
-                       }
                } else {
                        if ( count( $states ) ) {
                                $exceptions .= self::makeComment(

diff --git a/tests/phpunit/ResourceLoaderTestCase.php b/tests/phpunit/ResourceLoaderTestCase.php
index dc5549b..f5f302e 100644 (file)
--- a/tests/phpunit/ResourceLoaderTestCase.php
+++ b/tests/phpunit/ResourceLoaderTestCase.php
@@ -46,6 +46,7 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        protected $script = '';
        protected $styles = '';
        protected $skipFunction = null;
+       protected $isRaw = false;
        protected $targets = array( 'test' );
 
        public function __construct( $options = array() ) {
@@ -77,6 +78,10 @@ class ResourceLoaderTestModule extends ResourceLoaderModule {
        public function getSkipFunction() {
                return $this->skipFunction;
        }
+
+       public function isRaw() {
+               return $this->isRaw;
+       }
 }
 
 class ResourceLoaderFileModuleTestModule extends ResourceLoaderFileModule {

diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php
index 97fa85c..d7e8cd3 100644 (file)
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -141,7 +141,15 @@ class OutputPageTest extends MediaWikiTestCase {
                        // Load module script only
                        array(
                                array( 'test.foo', ResourceLoaderModule::TYPE_SCRIPTS ),
-                               '<script src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.foo&amp;only=scripts&amp;skin=fallback&amp;*"></script>
+                               '<script>if(window.mw){
+document.write("\u003Cscript src=\"http://127.0.0.1:8080/w/load.php?debug=false\u0026amp;lang=en\u0026amp;modules=test.foo\u0026amp;only=scripts\u0026amp;skin=fallback\u0026amp;*\"\u003E\u003C/script\u003E");
+}</script>
+'
+                       ),
+                       array(
+                               // Don't condition wrap raw modules (like the startup module)
+                               array( 'test.raw', ResourceLoaderModule::TYPE_SCRIPTS ),
+                               '<script src="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.raw&amp;only=scripts&amp;skin=fallback&amp;*"></script>
 '
                        ),
                        // Load module styles only
@@ -152,14 +160,10 @@ class OutputPageTest extends MediaWikiTestCase {
 '
                        ),
                        // Load private module (only=scripts)
-                       // This is asserted for completion (would get two condition wrappers),
-                       // though in practice we'd never embed a module with only=scripts,
-                       // that mode is reserved for hardcoded requests. Embedded modules
-                       // would always be combined.
                        array(
                                array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
                                '<script>if(window.mw){
-if(window.mw){mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});}
+mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});
 
 }</script>
 '
@@ -244,17 +248,21 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
                                'styles' => '/* pref-animate=off */ .mw-icon { transition: none; }',
                                'group' => 'private',
                        )),
+                       'test.raw' => new ResourceLoaderTestModule( array(
+                               'script' => 'mw.test.baz( { token: 123 } );',
+                               'isRaw' => true,
+                       )),
                        'test.noscript' => new ResourceLoaderTestModule( array(
                                'styles' => '.mw-test-noscript { content: "style"; }',
                                'group' => 'noscript',
                        )),
                        'test.group.bar' => new ResourceLoaderTestModule( array(
-                                       'styles' => '.mw-group-bar { content: "style"; }',
-                                       'group' => 'bar',
+                               'styles' => '.mw-group-bar { content: "style"; }',
+                               'group' => 'bar',
                        )),
                        'test.group.foo' => new ResourceLoaderTestModule( array(
-                                       'styles' => '.mw-group-foo { content: "style"; }',
-                                       'group' => 'foo',
+                               'styles' => '.mw-group-foo { content: "style"; }',
+                               'group' => 'foo',
                        )),
                ) );
                $links = $method->invokeArgs( $out, $args );