Merge "mediawiki.api: Prevent misusing #saveOptions"

diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33
index cacfa51..e545af8 100644 (file)
--- a/RELEASE-NOTES-1.33
+++ b/RELEASE-NOTES-1.33
@@ -247,6 +247,8 @@ because of Phabricator reports.
 * The mw.user.stickyRandomId() method, deprecated in 1.32, was removed.
   Use mw.user.getPageviewToken() instead.
 * Removed deprecated class property WikiRevision::$importer.
+* ResourceLoaderFileModule::readStyleFiles() now requires its $context
+  parameter.
 
 === Deprecations in 1.33 ===
 * The configuration option $wgUseESI has been deprecated, and is expected

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index 4abd729..292e8df 100644 (file)
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -44,6 +44,7 @@ use ParserCache;
 use ParserFactory;
 use PasswordFactory;
 use ProxyLookup;
+use ResourceLoader;
 use SearchEngine;
 use SearchEngineConfig;
 use SearchEngineFactory;
@@ -744,6 +745,14 @@ class MediaWikiServices extends ServiceContainer {
                return $this->getService( 'ReadOnlyMode' );
        }
 
+       /**
+        * @since 1.33
+        * @return ResourceLoader
+        */
+       public function getResourceLoader() {
+               return $this->getService( 'ResourceLoader' );
+       }
+
        /**
         * @since 1.31
         * @return RevisionFactory

diff --git a/includes/OutputPage.php b/includes/OutputPage.php
index 61a1ef2..9b766bb 100644 (file)
--- a/includes/OutputPage.php
+++ b/includes/OutputPage.php
@@ -21,7 +21,6 @@
  */
 
 use MediaWiki\Linker\LinkTarget;
-use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 use MediaWiki\Session\SessionManager;
 use Wikimedia\Rdbms\IResultWrapper;
@@ -3276,10 +3275,8 @@ class OutputPage extends ContextSource {
         */
        public function getResourceLoader() {
                if ( is_null( $this->mResourceLoader ) ) {
-                       $this->mResourceLoader = new ResourceLoader(
-                               $this->getConfig(),
-                               LoggerFactory::getInstance( 'resourceloader' )
-                       );
+                       // Lazy-initialise as needed
+                       $this->mResourceLoader = MediaWikiServices::getInstance()->getResourceLoader();
                }
                return $this->mResourceLoader;
        }

diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 44ca502..46dd913 100644 (file)
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -413,6 +413,13 @@ return [
                );
        },
 
+       'ResourceLoader' => function ( MediaWikiServices $services ) : ResourceLoader {
+               return new ResourceLoader(
+                       $services->getMainConfig(),
+                       LoggerFactory::getInstance( 'resourceloader' )
+               );
+       },
+
        'RevisionFactory' => function ( MediaWikiServices $services ) : RevisionFactory {
                return $services->getRevisionStore();
        },

diff --git a/includes/cache/MessageBlobStore.php b/includes/cache/MessageBlobStore.php
index 65e659d..19c4997 100644 (file)
--- a/includes/cache/MessageBlobStore.php
+++ b/includes/cache/MessageBlobStore.php
@@ -23,6 +23,7 @@
  * @author Timo Tijhof
  */
 
+use MediaWiki\MediaWikiServices;
 use Psr\Log\LoggerAwareInterface;
 use Psr\Log\LoggerInterface;
 use Psr\Log\NullLogger;
@@ -194,7 +195,7 @@ class MessageBlobStore implements LoggerAwareInterface {
                // Lazy-initialise this property because most callers don't need it.
                if ( $this->resourceloader === null ) {
                        $this->logger->warning( __CLASS__ . ' created without a ResourceLoader instance' );
-                       $this->resourceloader = new ResourceLoader();
+                       $this->resourceloader = MediaWikiServices::getInstance()->getResourceLoader();
                }
                return $this->resourceloader;
        }

diff --git a/includes/installer/WebInstallerOutput.php b/includes/installer/WebInstallerOutput.php
index ae07d0c..b061d0d 100644 (file)
--- a/includes/installer/WebInstallerOutput.php
+++ b/includes/installer/WebInstallerOutput.php
@@ -21,6 +21,8 @@
  * @ingroup Deployment
  */
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * Output class modelled on OutputPage.
  *
@@ -147,7 +149,7 @@ class WebInstallerOutput {
                        'mediawiki.skinning.interface',
                ];
 
-               $resourceLoader = new ResourceLoader();
+               $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader();
 
                if ( file_exists( "$wgStyleDirectory/Vector/skin.json" ) ) {
                        // Force loading Vector skin if available as a fallback skin

diff --git a/includes/resourceloader/ResourceLoader.php b/includes/resourceloader/ResourceLoader.php
index b648260..1a23258 100644 (file)
--- a/includes/resourceloader/ResourceLoader.php
+++ b/includes/resourceloader/ResourceLoader.php
@@ -245,6 +245,7 @@ class ResourceLoader implements LoggerAwareInterface {
                $this->logger = $logger ?: new NullLogger();
 
                if ( !$config ) {
+                       // TODO: Deprecate and remove.
                        $this->logger->debug( __METHOD__ . ' was called without providing a Config instance' );
                        $config = MediaWikiServices::getInstance()->getMainConfig();
                }

diff --git a/includes/resourceloader/ResourceLoaderContext.php b/includes/resourceloader/ResourceLoaderContext.php
index 57392b9..67de192 100644 (file)
--- a/includes/resourceloader/ResourceLoaderContext.php
+++ b/includes/resourceloader/ResourceLoaderContext.php
@@ -133,9 +133,19 @@ class ResourceLoaderContext implements MessageLocalizer {
        /**
         * Return a dummy ResourceLoaderContext object suitable for passing into
         * things that don't "really" need a context.
+        *
+        * Use cases:
+        * - Creating html5shiv script tag in OutputPage.
+        * - Unit tests (deprecated, create empty instance directly or use RLTestCase).
+        *
         * @return ResourceLoaderContext
         */
        public static function newDummyContext() {
+               // This currently creates a non-empty instance of ResourceLoader (all modules registered),
+               // but that's probably not needed. So once that moves into ServiceWiring, this'll
+               // become more like the EmptyResourceLoader class we have in PHPUnit tests, which
+               // is what this should've had originally. If this turns out to be untrue, change to:
+               // `MediaWikiServices::getInstance()->getResourceLoader()` instead.
                return new self( new ResourceLoader(
                        MediaWikiServices::getInstance()->getMainConfig(),
                        LoggerFactory::getInstance( 'resourceloader' )

diff --git a/includes/resourceloader/ResourceLoaderFileModule.php b/includes/resourceloader/ResourceLoaderFileModule.php
index 0e53e5e..4444b13 100644 (file)
--- a/includes/resourceloader/ResourceLoaderFileModule.php
+++ b/includes/resourceloader/ResourceLoaderFileModule.php
@@ -878,25 +878,16 @@ class ResourceLoaderFileModule extends ResourceLoaderModule {
        /**
         * Get the contents of a list of CSS files.
         *
-        * This is considered a private method. Exposed for internal use by WebInstallerOutput.
-        *
-        * @private
+        * @internal This is considered a private method. Exposed for internal use by WebInstallerOutput.
         * @param array $styles Map of media type to file paths to read, remap, and concatenate
         * @param bool $flip
-        * @param ResourceLoaderContext|null $context
+        * @param ResourceLoaderContext $context
         * @return array List of concatenated and remapped CSS data from $styles,
         *     keyed by media type
         * @throws MWException
-        * @since 1.27 Calling this method without a ResourceLoaderContext instance
-        *   is deprecated.
         */
-       public function readStyleFiles( array $styles, $flip, $context = null ) {
-               if ( $context === null ) {
-                       wfDeprecated( __METHOD__ . ' without a ResourceLoader context', '1.27' );
-                       $context = ResourceLoaderContext::newDummyContext();
-               }
-
-               if ( empty( $styles ) ) {
+       public function readStyleFiles( array $styles, $flip, $context ) {
+               if ( !$styles ) {
                        return [];
                }
                foreach ( $styles as $media => $files ) {

diff --git a/load.php b/load.php
index 1997fe7..4d34e5d 100644 (file)
--- a/load.php
+++ b/load.php
@@ -22,7 +22,6 @@
  * @author Trevor Parscal
  */
 
-use MediaWiki\Logger\LoggerFactory;
 use MediaWiki\MediaWikiServices;
 
 // This endpoint is supposed to be independent of request cookies and other
@@ -40,11 +39,7 @@ if ( !$wgRequest->checkUrlExtension() ) {
 // writes when getting database connections for ResourceLoader. (T192611)
 MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->disableChronologyProtection();
 
-// Set up ResourceLoader
-$resourceLoader = new ResourceLoader(
-       ConfigFactory::getDefaultInstance()->makeConfig( 'main' ),
-       LoggerFactory::getInstance( 'resourceloader' )
-);
+$resourceLoader = MediaWikiServices::getInstance()->getResourceLoader();
 $context = new ResourceLoaderContext( $resourceLoader, $wgRequest );
 
 // Respond to ResourceLoader request

diff --git a/maintenance/cleanupRemovedModules.php b/maintenance/cleanupRemovedModules.php
index 63838d2..cec640b 100644 (file)
--- a/maintenance/cleanupRemovedModules.php
+++ b/maintenance/cleanupRemovedModules.php
@@ -46,7 +46,7 @@ class CleanupRemovedModules extends Maintenance {
                $this->output( "Cleaning up module_deps table...\n" );
 
                $dbw = $this->getDB( DB_MASTER );
-               $rl = new ResourceLoader( MediaWikiServices::getInstance()->getMainConfig() );
+               $rl = MediaWikiServices::getInstance()->getResourceLoader();
                $moduleNames = $rl->getModuleNames();
                $res = $dbw->select( 'module_deps',
                        [ 'md_module', 'md_skin' ],

diff --git a/resources/src/mediawiki.Title/Title.js b/resources/src/mediawiki.Title/Title.js
index ff7a40f..6bb3bce 100644 (file)
--- a/resources/src/mediawiki.Title/Title.js
+++ b/resources/src/mediawiki.Title/Title.js
@@ -687,7 +687,7 @@
         * @return {boolean} Namespace is a signature namespace
         */
        Title.wantSignaturesNamespace = function ( namespaceId ) {
-               return this.isTalkNamespace( namespaceId ) ||
+               return Title.isTalkNamespace( namespaceId ) ||
                        mw.config.get( 'wgExtraSignatureNamespaces' ).indexOf( namespaceId ) !== -1;
        };
 

diff --git a/tests/phpunit/structure/ResourcesTest.php b/tests/phpunit/structure/ResourcesTest.php
index 8a08181..776dee1 100644 (file)
--- a/tests/phpunit/structure/ResourcesTest.php
+++ b/tests/phpunit/structure/ResourcesTest.php
@@ -1,8 +1,11 @@
 <?php
+
+use MediaWiki\MediaWikiServices;
+use Wikimedia\TestingAccessWrapper;
+
 /**
  * Sanity checks for making sure registered resources are sane.
  *
- * @file
  * @author Antoine Musso
  * @author Niklas Laxström
  * @author Santhosh Thottingal
@@ -171,8 +174,8 @@ class ResourcesTest extends MediaWikiTestCase {
                $org_wgEnableJavaScriptTest = $wgEnableJavaScriptTest;
                $wgEnableJavaScriptTest = true;
 
-               // Initialize ResourceLoader
-               $rl = new ResourceLoader();
+               // Get main ResourceLoader
+               $rl = MediaWikiServices::getInstance()->getResourceLoader();
 
                $modules = [];
 
@@ -243,9 +246,6 @@ class ResourcesTest extends MediaWikiTestCase {
        /**
         * Get all resource files from modules that are an instance of
         * ResourceLoaderFileModule (or one of its subclasses).
-        *
-        * Since the raw data is stored in protected properties, we have to
-        * overrride this through ReflectionObject methods.
         */
        public static function provideResourceFiles() {
                $data = self::getAllModules();
@@ -273,14 +273,12 @@ class ResourcesTest extends MediaWikiTestCase {
                                continue;
                        }
 
-                       $reflectedModule = new ReflectionObject( $module );
+                       $moduleProxy = TestingAccessWrapper::newFromObject( $module );
 
                        $files = [];
 
                        foreach ( $filePathProps['lists'] as $propName ) {
-                               $property = $reflectedModule->getProperty( $propName );
-                               $property->setAccessible( true );
-                               $list = $property->getValue( $module );
+                               $list = $moduleProxy->$propName;
                                foreach ( $list as $key => $value ) {
                                        // 'scripts' are numeral arrays.
                                        // 'styles' can be numeral or associative.
@@ -295,9 +293,7 @@ class ResourcesTest extends MediaWikiTestCase {
                        }
 
                        foreach ( $filePathProps['nested-lists'] as $propName ) {
-                               $property = $reflectedModule->getProperty( $propName );
-                               $property->setAccessible( true );
-                               $lists = $property->getValue( $module );
+                               $lists = $moduleProxy->$propName;
                                foreach ( $lists as $list ) {
                                        foreach ( $list as $key => $value ) {
                                                // We need the same filter as for 'lists',
@@ -311,29 +307,23 @@ class ResourcesTest extends MediaWikiTestCase {
                                }
                        }
 
-                       // Get method for resolving the paths to full paths
-                       $method = $reflectedModule->getMethod( 'getLocalPath' );
-                       $method->setAccessible( true );
-
                        // Populate cases
                        foreach ( $files as $file ) {
                                $cases[] = [
-                                       $method->invoke( $module, $file ),
+                                       $moduleProxy->getLocalPath( $file ),
                                        $moduleName,
                                        ( $file instanceof ResourceLoaderFilePath ? $file->getPath() : $file ),
                                ];
                        }
 
                        // To populate missingLocalFileRefs. Not sure how sane this is inside this test...
-                       $module->readStyleFiles(
+                       $moduleProxy->readStyleFiles(
                                $module->getStyleFiles( $data['context'] ),
                                $module->getFlip( $data['context'] ),
                                $data['context']
                        );
 
-                       $property = $reflectedModule->getProperty( 'missingLocalFileRefs' );
-                       $property->setAccessible( true );
-                       $missingLocalFileRefs = $property->getValue( $module );
+                       $missingLocalFileRefs = $moduleProxy->missingLocalFileRefs;
 
                        foreach ( $missingLocalFileRefs as $file ) {
                                $cases[] = [

diff --git a/tests/phpunit/suites/LessTestSuite.php b/tests/phpunit/suites/LessTestSuite.php
index 26a784a..b5bd882 100644 (file)
--- a/tests/phpunit/suites/LessTestSuite.php
+++ b/tests/phpunit/suites/LessTestSuite.php
@@ -1,5 +1,7 @@
 <?php
 
+use MediaWiki\MediaWikiServices;
+
 /**
  * @author Sam Smith <samsmith@wikimedia.org>
  */
@@ -7,7 +9,7 @@ class LessTestSuite extends PHPUnit_Framework_TestSuite {
        public function __construct() {
                parent::__construct();
 
-               $resourceLoader = new ResourceLoader();
+               $resourceLoader = MediaWikiServices::getInstance()->getResourceLoader();
 
                foreach ( $resourceLoader->getModuleNames() as $name ) {
                        $module = $resourceLoader->getModule( $name );

diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.Title.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.Title.test.js
index 84e1d4e..fca1f7d 100644 (file)
--- a/tests/qunit/suites/resources/mediawiki/mediawiki.Title.test.js
+++ b/tests/qunit/suites/resources/mediawiki/mediawiki.Title.test.js
@@ -324,19 +324,14 @@
        } );
 
        QUnit.test( 'wantSignaturesNamespace', function ( assert ) {
-               var namespaces = mw.config.values.wgExtraSignatureNamespaces;
-
-               mw.config.values.wgExtraSignatureNamespaces = [];
+               mw.config.set( 'wgExtraSignatureNamespaces', [] );
                assert.strictEqual( mw.Title.wantSignaturesNamespace( 0 ), false, 'Main namespace has no signatures' );
                assert.strictEqual( mw.Title.wantSignaturesNamespace( 1 ), true, 'Talk namespace has signatures' );
                assert.strictEqual( mw.Title.wantSignaturesNamespace( 2 ), false, 'NS2 has no signatures' );
                assert.strictEqual( mw.Title.wantSignaturesNamespace( 3 ), true, 'NS3 has signatures' );
 
-               mw.config.values.wgExtraSignatureNamespaces = [ 0 ];
+               mw.config.set( 'wgExtraSignatureNamespaces', [ 0 ] );
                assert.strictEqual( mw.Title.wantSignaturesNamespace( 0 ), true, 'Main namespace has signatures when explicitly defined' );
-
-               // Restore
-               mw.config.values.wgExtraSignatureNamespaces = namespaces;
        } );
 
        QUnit.test( 'Throw error on invalid title', function ( assert ) {