resourceloader: Wrap only=script responses in "if(window.mw)"
authorTimo Tijhof <krinklemail@gmail.com>
Wed, 6 Aug 2014 15:54:22 +0000 (15:54 +0000)
committerKrinkle <krinklemail@gmail.com>
Tue, 12 Aug 2014 23:20:42 +0000 (23:20 +0000)
We currently have a few legacy requests to the load.php end point
that bypass the ResourceLoader client by coding a request to
load.php via a "<script src>" directly. Most prominently the
request for the 'site' wiki module (aka MediaWiki:Common.js).

Remove the manual wrapping of embedded private modules as this
is now taken are of by ResourceLoader::makeModuleResponse itself.

Misc:
* Mark "jquery" and "mediawiki" as Raw modules. While the startup
  module had this already, these didn't. Without this, they'd
  get the conditional wrap – which would be a problem since mediawiki.js
  can't be conditional on 'window.mw' for that file defines that
  namespace itself.
* Strip the cache-key comment in the unit tests because the hash
  no longer matches and using the generic 'wiki' dbname was breaking
  DB queries.
* Relates to bug 63587.
* See also 05d0f6fefdcc959d which expands the reach of the non-JS
  environment to IE6 and helped expose this bug.

Change-Id: Icf6ede09b51ce212aa70ff6be4b341762ec75b4d

includes/OutputPage.php
includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderStartUpModule.php
resources/Resources.php
tests/phpunit/includes/OutputPageTest.php

index 87a0809..e91f2af 100644 (file)
@@ -2797,9 +2797,7 @@ $templates
                                                );
                                        } else {
                                                $links['html'] .= Html::inlineScript(
-                                                       ResourceLoader::makeLoaderConditionalScript(
-                                                               $resourceLoader->makeModuleResponse( $context, $grpModules )
-                                                       )
+                                                       $resourceLoader->makeModuleResponse( $context, $grpModules )
                                                );
                                        }
                                        $links['html'] .= "\n";
index 21e6435..53b6d81 100644 (file)
@@ -990,6 +990,13 @@ class ResourceLoader {
                        if ( count( $states ) ) {
                                $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 );
                } else {
                        if ( count( $states ) ) {
                                $exceptions .= self::makeComment(
index 2c0f8df..65d09b0 100644 (file)
@@ -333,6 +333,15 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
                return true;
        }
 
+       /**
+        * Base modules required for the the base environment of ResourceLoader
+        *
+        * @return array
+        */
+       public static function getStartupModules() {
+               return array( 'jquery', 'mediawiki' );
+       }
+
        /**
         * Get the load URL of the startup modules.
         *
@@ -343,8 +352,7 @@ class ResourceLoaderStartUpModule extends ResourceLoaderModule {
         * @return string
         */
        public static function getStartupModulesUrl( ResourceLoaderContext $context ) {
-               // The core modules:
-               $moduleNames = array( 'jquery', 'mediawiki' );
+               $moduleNames = self::getStartupModules();
 
                // Get the latest version
                $loader = $context->getResourceLoader();
index b588cf2..bd85f6e 100644 (file)
@@ -128,7 +128,7 @@ return array(
                        array(
                                'resources/lib/jquery/jquery.js'
                        ) ),
-               'debugRaw' => false,
+               'raw' => true,
                'targets' => array( 'desktop', 'mobile' ),
        ),
 
@@ -771,7 +771,7 @@ return array(
        'mediawiki' => array(
                'scripts' => 'resources/src/mediawiki/mediawiki.js',
                'debugScripts' => 'resources/src/mediawiki/mediawiki.log.js',
-               'debugRaw' => false,
+               'raw' => true,
                'targets' => array( 'desktop', 'mobile' ),
        ),
        'mediawiki.api' => array(
index 2cfdfcd..50fd3ef 100644 (file)
@@ -154,10 +154,8 @@ class OutputPageTest extends MediaWikiTestCase {
                        // 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/* cache key: wiki:resourceloader:filter:minify-css:7:fd8ea20b3336b2bfb230c789d430067a */"]},{});
-/* cache key: wiki:resourceloader:filter:minify-js:7:274ccee17be73cd5f4dda5dc2a819188 */
-}</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
@@ -186,10 +184,6 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
                        'wgLoadScript' => 'http://127.0.0.1:8080/w/load.php',
                        // Affects whether CDATA is inserted
                        'wgWellFormedXml' => false,
-                       // Cache key is based on database name, and affects output;
-                       // this test should not touch the database anyways.
-                       'wgDBname' => 'wiki',
-                       'wgDBprefix' => '',
                ) );
                $class = new ReflectionClass( 'OutputPage' );
                $method = $class->getMethod( 'makeResourceLoaderLink' );
@@ -219,6 +213,8 @@ mw.loader.implement("test.quux",function($,jQuery){mw.test.baz({token:123});},{"
                        )),
                ) );
                $links = $method->invokeArgs( $out, $args );
-               $this->assertEquals( $expectedHtml, $links['html'] );
+               // Strip comments to avoid variation due to wgDBname in WikiID and cache key
+               $actualHtml = preg_replace( '#/\*[^*]+\*/#', '', $links['html'] );
+               $this->assertEquals( $expectedHtml, $actualHtml );
        }
 }