resourceloader: Replace ResourceLoaderDebug config use with context
authorTimo Tijhof <krinklemail@gmail.com>
Fri, 8 Mar 2019 20:33:04 +0000 (20:33 +0000)
committerTimo Tijhof <krinklemail@gmail.com>
Fri, 8 Mar 2019 20:33:16 +0000 (20:33 +0000)
Reduce our reliance on static state and configuration, and
propagate more state in explicit ways, through context, and
request parameters.

OutputPage creates ResourceLoaderContext and ResourceLoaderClientHtml
based on the configuration (via ResourceLoader::inDebugMode).

Everything within those classes should not need to check it
again.

* ResourceLoaderClientHtml:
  Already doesn't check MW config, but it's test was still
  mocking it. Removed now, and confirmed that it passes both
  with true and false. The individual test cases set
  debug=true/false as needed already.

  It's sets were previously relying on the accidental behaviour
  that within a unit test, we don't serialise over HTTP, which
  meant that a pure PHP boolean would survive. With the new
  raw `=== 'true'` check, this no longer works. Set it as a
  string explicitly instead, which is the only thing we support
  outside unit tests as well.

* ResourceLoaderContext:
  Remove fallback to MW config when 'debug' is unset.
  This is never unset in practice given that all load.php
  urls have it set by OutputPage based on ResourceLoader::inDebugMode.

  This change means that manually constructed ad-hoc load.php
  urls that are missing 'debug=' parameter, will now always be
  read as debug=false. This was the default already, but could
  previously be changed through wgResourceLoaderDebug.

When changing wgResourceLoaderDebug, everything will still have
debug=true as before. The only change is when constructing load.php
urls manually and explicitly not set it.

Bug: T32956
Change-Id: Ie3424be46e2b8311968f3068ca08ba6a1139224a

includes/resourceloader/ResourceLoader.php
includes/resourceloader/ResourceLoaderContext.php
tests/phpunit/ResourceLoaderTestCase.php
tests/phpunit/includes/resourceloader/ResourceLoaderClientHtmlTest.php
tests/phpunit/includes/resourceloader/ResourceLoaderTest.php

index 3d13350..b36b8d0 100644 (file)
@@ -1106,7 +1106,7 @@ MESSAGE;
                                                                // mw.loader.implement will use globalEval if scripts is a string.
                                                                // Minify manually here, because general response minification is
                                                                // not effective due it being a string literal, not a function.
-                                                               if ( !self::inDebugMode() ) {
+                                                               if ( !$context->getDebug() ) {
                                                                        $scripts = self::filter( 'minify-js', $scripts ); // T107377
                                                                }
                                                        } else {
index 67de192..a625970 100644 (file)
@@ -72,10 +72,7 @@ class ResourceLoaderContext implements MessageLocalizer {
 
                // Various parameters
                $this->user = $request->getRawVal( 'user' );
-               $this->debug = $request->getFuzzyBool(
-                       'debug',
-                       $this->getConfig()->get( 'ResourceLoaderDebug' )
-               );
+               $this->debug = $request->getRawVal( 'debug' ) === 'true';
                $this->only = $request->getRawVal( 'only', null );
                $this->version = $request->getRawVal( 'version', null );
                $this->raw = $request->getFuzzyBool( 'raw' );
index cadd0ff..ca5ff6c 100644 (file)
@@ -26,6 +26,7 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
                        $options = [ 'lang' => $options ];
                }
                $options += [
+                       'debug' => 'true',
                        'lang' => 'en',
                        'dir' => 'ltr',
                        'skin' => 'vector',
@@ -35,6 +36,7 @@ abstract class ResourceLoaderTestCase extends MediaWikiTestCase {
                ];
                $resourceLoader = $rl ?: new ResourceLoader();
                $request = new FauxRequest( [
+                               'debug' => $options['debug'],
                                'lang' => $options['lang'],
                                'modules' => $options['modules'],
                                'only' => $options['only'],
index 8fdf5dd..3f9d52b 100644 (file)
@@ -21,7 +21,6 @@ class ResourceLoaderClientHtmlTest extends PHPUnit\Framework\TestCase {
                        'ResourceModuleSkinStyles' => [],
                        'ResourceModules' => [],
                        'EnableJavaScriptTest' => false,
-                       'ResourceLoaderDebug' => false,
                        'LoadScript' => '/w/load.php',
                ] );
                return new ResourceLoaderContext(
@@ -329,7 +328,7 @@ Deprecation message.' ]
                                'output' => '<script async="" src="/w/load.php?debug=false&amp;lang=nl&amp;modules=test.scripts.raw&amp;only=scripts&amp;skin=fallback"></script>',
                        ],
                        [
-                               'context' => [ 'sync' => true ],
+                               'context' => [ 'sync' => '1' ],
                                'modules' => [ 'test.scripts.raw' ],
                                'only' => ResourceLoaderModule::TYPE_SCRIPTS,
                                'output' => '<script src="/w/load.php?debug=false&amp;lang=nl&amp;modules=test.scripts.raw&amp;only=scripts&amp;skin=fallback&amp;sync=1"></script>',
@@ -347,14 +346,14 @@ Deprecation message.' ]
                                'output' => '<script>(window.RLQ=window.RLQ||[]).push(function(){mw.loader.load("/w/load.php?debug=false\u0026lang=nl\u0026modules=test.user\u0026skin=fallback\u0026user=Example\u0026version=0a56zyi");});</script>',
                        ],
                        [
-                               'context' => [ 'debug' => true ],
+                               'context' => [ 'debug' => 'true' ],
                                'modules' => [ 'test.styles.pure', 'test.styles.mixed' ],
                                'only' => ResourceLoaderModule::TYPE_STYLES,
                                'output' => '<link rel="stylesheet" href="/w/load.php?debug=true&amp;lang=nl&amp;modules=test.styles.mixed&amp;only=styles&amp;skin=fallback"/>' . "\n"
                                        . '<link rel="stylesheet" href="/w/load.php?debug=true&amp;lang=nl&amp;modules=test.styles.pure&amp;only=styles&amp;skin=fallback"/>',
                        ],
                        [
-                               'context' => [ 'debug' => false ],
+                               'context' => [ 'debug' => 'false' ],
                                'modules' => [ 'test.styles.pure', 'test.styles.mixed' ],
                                'only' => ResourceLoaderModule::TYPE_STYLES,
                                'output' => '<link rel="stylesheet" href="/w/load.php?debug=false&amp;lang=nl&amp;modules=test.styles.mixed%2Cpure&amp;only=styles&amp;skin=fallback"/>',
index f6bf7f1..5941c6e 100644 (file)
@@ -767,11 +767,11 @@ END
                }, $scripts );
                $rl->register( $modules );
 
-               $this->setMwGlobals( 'wgResourceLoaderDebug', $debug );
                $context = $this->getResourceLoaderContext(
                        [
                                'modules' => implode( '|', array_keys( $modules ) ),
                                'only' => 'scripts',
+                               'debug' => $debug ? 'true' : 'false',
                        ],
                        $rl
                );