resourceloader: Only conditional-wrap script responses with only=scripts
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 13 Aug 2014 18:06:10 +0000 (19:06 +0100)
committerKrinkle <krinklemail@gmail.com>
Fri, 22 Aug 2014 01:15:49 +0000 (01:15 +0000)
Follows-up 9272bc6c4721225. The shouldIncludeScripts method returns
true for only=scripts, but also for other responses (except for
only=styles, naturally).

Regular load.php responses that load both scripts and styles don't
need the conditional wrap because those requests should only be
made from the mw.loader client which inherently means the
environment has been provided already.

It's merely unnecessary and shouldn't have caused any issues since
it simply evaluates to true always. It was already correctly being
excluded from raw modules that run before the environment is
provided (such as startup, jquery and mediawiki).

Change-Id: I375a7a8039f9b3ad4909e76005725f7d975d8a5e

includes/resourceloader/ResourceLoader.php
tests/phpunit/includes/OutputPageTest.php

index c200408..5f72d8d 100644 (file)
@@ -1007,12 +1007,14 @@ class ResourceLoader {
                                $out .= self::makeLoaderStateScript( $states );
                        }
 
-                       // 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 );
+                       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(
index 3eb58fc..5c1994b 100644 (file)
@@ -149,12 +149,19 @@ class OutputPageTest extends MediaWikiTestCase {
                        array(
                                array( array( 'test.baz', 'test.foo', 'test.bar' ), ResourceLoaderModule::TYPE_STYLES ),
                                '<link rel=stylesheet href="http://127.0.0.1:8080/w/load.php?debug=false&amp;lang=en&amp;modules=test.bar%2Cbaz%2Cfoo&amp;only=styles&amp;skin=fallback&amp;*">
+'
+                       ),
+                       // Load private module (scripts)
+                       array(
+                               array( 'test.quux', ResourceLoaderModule::TYPE_SCRIPTS ),
+                               '<script>if(window.mw){mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});}
+</script>
 '
                        ),
                        // Load private module (combined)
                        array(
                                array( 'test.quux', ResourceLoaderModule::TYPE_COMBINED ),
-                               '<script>if(window.mw){mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});}
+                               '<script>mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});
 </script>
 '
                        ),