OutputPage: Restore ResourceLoader condition wrap for embedded modules
authorTimo Tijhof <krinklemail@gmail.com>
Tue, 2 Sep 2014 01:25:13 +0000 (03:25 +0200)
committerTimo Tijhof <krinklemail@gmail.com>
Tue, 2 Sep 2014 09:05:13 +0000 (11:05 +0200)
Follows-up 9272bc6c4703c503da22.

* In 9272bc6c47, the condition wrap was removed from
  OutputPage for no reason. This went unnnoticed as I had also
  accidentally made the cond wrap in makeModuleResponse apply
  to both only=scripts and regular (faux) responses, such as by
  OutputPage embedding private modules.

* In 03c503da22, the latter bug was fixed, thus exposing the former.

This wrapper belongs in OutputPage, not in ResourceLoader. It's
OutputPage making the loader request. And just like in other places,
it's the "client"'s responsibility to ensure the request is either not
made or wrapped appropriately.

The test for "private module (only=scripts)" could be removed but
I'll keep it so we can see how this changes in the future. It's
a case that can't ever happen, but if it would, it currently gets
a double condition wrapper, which is fine.

Change-Id: Id333e4958ed769831fabca02164c1e8505962d57

includes/OutputPage.php
tests/phpunit/includes/OutputPageTest.php

index a58a79a..af90ca6 100644 (file)
@@ -2797,7 +2797,9 @@ $templates
                                                );
                                        } else {
                                                $links['html'] .= Html::inlineScript(
-                                                       $resourceLoader->makeModuleResponse( $context, $grpModules )
+                                                       ResourceLoader::makeLoaderConditionalScript(
+                                                               $resourceLoader->makeModuleResponse( $context, $grpModules )
+                                                       )
                                                );
                                        }
                                        $links['html'] .= "\n";
index 5c1994b..69a302d 100644 (file)
@@ -151,18 +151,26 @@ class OutputPageTest extends MediaWikiTestCase {
                                '<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)
+                       // 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){mw.test.baz({token:123});mw.loader.state({"test.quux":"ready"});}
-</script>
+                               '<script>if(window.mw){
+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>mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});
-</script>
+                               '<script>if(window.mw){
+mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"css":[".mw-icon{transition:none}\n"]},{});
+
+}</script>
 '
                        ),
                        // Load module script with with ESI