jenkins-bot has submitted this change and it was merged.

Change subject: resourceloader: Only conditional-wrap script responses with 
only=scripts
......................................................................


resourceloader: Only conditional-wrap script responses with only=scripts

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
---
M includes/resourceloader/ResourceLoader.php
M tests/phpunit/includes/OutputPageTest.php
2 files changed, 16 insertions(+), 7 deletions(-)

Approvals:
  Catrope: Looks good to me, approved
  Thiemo Mättig (WMDE): Looks good to me, but someone else must approve
  jenkins-bot: Verified



diff --git a/includes/resourceloader/ResourceLoader.php 
b/includes/resourceloader/ResourceLoader.php
index c200408..5f72d8d 100644
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -1007,12 +1007,14 @@
                                $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(
diff --git a/tests/phpunit/includes/OutputPageTest.php 
b/tests/phpunit/includes/OutputPageTest.php
index 3eb58fc..5c1994b 100644
--- a/tests/phpunit/includes/OutputPageTest.php
+++ b/tests/phpunit/includes/OutputPageTest.php
@@ -151,10 +151,17 @@
                                '<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>
 '
                        ),

-- 
To view, visit https://gerrit.wikimedia.org/r/153839
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I375a7a8039f9b3ad4909e76005725f7d975d8a5e
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Krinkle <[email protected]>
Gerrit-Reviewer: Catrope <[email protected]>
Gerrit-Reviewer: Jforrester <[email protected]>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <[email protected]>
Gerrit-Reviewer: jenkins-bot <>

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to