skins: Move default style modules to getDefaultModules

This advances T140664 as well, because it encourages module
loading logic from the skin to be in this single method.

Update the tests for setupSkinUserCss(), to now target
getDefaultModules() instead, given setupSkinUserCss() no longer
provides these behaviours. Had to move where the mock object
was created so that it can be injected in the skin (previously
it could be passed as parameter). Besides, its generally best-
practice to create mocks and stubs within the test anyhow, not in
the data provider.

Bug: T140664
Depends-On: Ib2b19ba165a9d646a770702cdf1724509156b93e
Change-Id: I3404c1c2a7e8eae0b803b31f333cb9b837f43d4a

diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32
index a437eb9..72f877f 100644 (file)
--- a/RELEASE-NOTES-1.32
+++ b/RELEASE-NOTES-1.32
@@ -76,6 +76,9 @@ changes to languages because of Phabricator reports.
   configuration in LocalSettings.php.
 * HTMLForm::setSubmitProgressive() is deprecated. No need to call it. Submit
   button is already marked as progressive.
+* Skin::setupSkinUserCss() is deprecated. Adding of modules to load
+  has been centralised to Skin::getDefaultModules(), which is now capable
+  of queueing style modules as well.
 
 === Other changes in 1.32 ===
 * …

diff --git a/includes/installer/WebInstallerOutput.php b/includes/installer/WebInstallerOutput.php
index 6a55d69..cb0092d 100644 (file)
--- a/includes/installer/WebInstallerOutput.php
+++ b/includes/installer/WebInstallerOutput.php
@@ -130,9 +130,9 @@ class WebInstallerOutput {
                global $wgStyleDirectory;
 
                $moduleNames = [
-                       // See SkinTemplate::setupSkinUserCss
+                       // Based on Skin::getDefaultModules
                        'mediawiki.legacy.shared',
-                       // See Vector::setupSkinUserCss
+                       // Based on Vector::setupSkinUserCss
                        'mediawiki.skinning.interface',
                ];
 

diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php
index 2247cc2..340bc2f 100644 (file)
--- a/includes/skins/Skin.php
+++ b/includes/skins/Skin.php
@@ -174,23 +174,29 @@ abstract class Skin extends ContextSource {
        public function getDefaultModules() {
                $out = $this->getOutput();
                $config = $this->getConfig();
-               $user = $out->getUser();
+               $user = $this->getUser();
+
+               // Modules declared in the $modules literal are loaded
+               // for ALL users, on ALL pages, in ALL skins.
+               // Keep this list as small as possible!
                $modules = [
-                       // Styles key sets render blocking styles
-                       // Unlike other keys in this definition it is an associative array
-                       // where each key is the group name and points to a list of modules
                        'styles' => [
+                               // The 'styles' key sets render-blocking style modules
+                               // Unlike other keys in $modules, this is an associative array
+                               // where each key is its own group pointing to a list of modules
+                               'core' => [
+                                       'mediawiki.legacy.shared',
+                                       'mediawiki.legacy.commonPrint',
+                               ],
                                'content' => [],
+                               'syndicate' => [],
                        ],
-                       // modules not specific to any specific skin or page
                        'core' => [
-                               // Enforce various default modules for all pages and all skins
-                               // Keep this list as small as possible
                                'site',
                                'mediawiki.page.startup',
                                'mediawiki.user',
                        ],
-                       // modules that enhance the page content in some way
+                       // modules that enhance the content in some way
                        'content' => [
                                'mediawiki.page.ready',
                        ],
@@ -200,6 +206,8 @@ abstract class Skin extends ContextSource {
                        'watch' => [],
                        // modules which relate to the current users preferences
                        'user' => [],
+                       // modules relating to RSS/Atom Feeds
+                       'syndicate' => [],
                ];
 
                // Support for high-density display images if enabled
@@ -219,6 +227,12 @@ abstract class Skin extends ContextSource {
                        $modules['styles']['content'][] = 'jquery.makeCollapsible.styles';
                }
 
+               // Deprecated since 1.26: Unconditional loading of mediawiki.ui.button
+               // on every page is deprecated. Express a dependency instead.
+               if ( strpos( $out->getHTML(), 'mw-ui-button' ) !== false ) {
+                       $modules['styles']['content'][] = 'mediawiki.ui.button';
+               }
+
                if ( $out->isTOCEnabled() ) {
                        $modules['content'][] = 'mediawiki.toc';
                }
@@ -241,6 +255,11 @@ abstract class Skin extends ContextSource {
                if ( $out->isArticle() && $user->getOption( 'editondblclick' ) ) {
                        $modules['user'][] = 'mediawiki.action.view.dblClickEdit';
                }
+
+               if ( $out->isSyndicated() ) {
+                       $modules['styles']['syndicate'][] = 'mediawiki.feedlink';
+               }
+
                return $modules;
        }
 
@@ -412,14 +431,14 @@ abstract class Skin extends ContextSource {
        }
 
        /**
-        * Add skin specific stylesheets
-        * Calling this method with an $out of anything but the same OutputPage
-        * inside ->getOutput() is deprecated. The $out arg is kept
-        * for compatibility purposes with skins.
-        * @param OutputPage $out
-        * @todo delete
+        * Hook point for adding style modules to OutputPage.
+        *
+        * @deprecated since 1.32 Use getDefaultModules() instead.
+        * @param OutputPage $out Legacy parameter, identical to $this->getOutput()
         */
-       abstract function setupSkinUserCss( OutputPage $out );
+       public function setupSkinUserCss( OutputPage $out ) {
+               // Stub.
+       }
 
        /**
         * TODO: document

diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php
index 9461bcf..1d5d534 100644 (file)
--- a/includes/skins/SkinTemplate.php
+++ b/includes/skins/SkinTemplate.php
@@ -56,29 +56,6 @@ class SkinTemplate extends Skin {
        public $username;
        public $userpageUrlDetails;
 
-       /**
-        * Add specific styles for this skin
-        *
-        * @param OutputPage $out
-        */
-       public function setupSkinUserCss( OutputPage $out ) {
-               $moduleStyles = [
-                       'mediawiki.legacy.shared',
-                       'mediawiki.legacy.commonPrint',
-               ];
-               if ( $out->isSyndicated() ) {
-                       $moduleStyles[] = 'mediawiki.feedlink';
-               }
-
-               // Deprecated since 1.26: Unconditional loading of mediawiki.ui.button
-               // on every page is deprecated. Express a dependency instead.
-               if ( strpos( $out->getHTML(), 'mw-ui-button' ) !== false ) {
-                       $moduleStyles[] = 'mediawiki.ui.button';
-               }
-
-               $out->addModuleStyles( $moduleStyles );
-       }
-
        /**
         * Create the template engine object; we feed it a bunch of data
         * and eventually it spits out some HTML. Should have interface

diff --git a/tests/phpunit/includes/skins/SkinTemplateTest.php b/tests/phpunit/includes/skins/SkinTemplateTest.php
index b746fc3..6ea5b40 100644 (file)
--- a/tests/phpunit/includes/skins/SkinTemplateTest.php
+++ b/tests/phpunit/includes/skins/SkinTemplateTest.php
@@ -49,13 +49,13 @@ class SkinTemplateTest extends MediaWikiTestCase {
                $mock->expects( $this->once() )
                        ->method( 'isSyndicated' )
                        ->will( $this->returnValue( $isSyndicated ) );
-               $mock->expects( $this->once() )
+               $mock->expects( $this->any() )
                        ->method( 'getHTML' )
                        ->will( $this->returnValue( $html ) );
                return $mock;
        }
 
-       public function provideSetupSkinUserCss() {
+       public function provideGetDefaultModules() {
                $defaultStyles = [
                        'mediawiki.legacy.shared',
                        'mediawiki.legacy.commonPrint',
@@ -64,37 +64,46 @@ class SkinTemplateTest extends MediaWikiTestCase {
                $feedStyle = 'mediawiki.feedlink';
                return [
                        [
-                               $this->getMockOutputPage( false, '' ),
+                               false,
+                               '',
                                $defaultStyles
                        ],
                        [
-                               $this->getMockOutputPage( true, '' ),
+                               true,
+                               '',
                                array_merge( $defaultStyles, [ $feedStyle ] )
                        ],
                        [
-                               $this->getMockOutputPage( false, 'FOO mw-ui-button BAR' ),
+                               false,
+                               'FOO mw-ui-button BAR',
                                array_merge( $defaultStyles, [ $buttonStyle ] )
                        ],
                        [
-                               $this->getMockOutputPage( true, 'FOO mw-ui-button BAR' ),
-                               array_merge( $defaultStyles, [ $feedStyle, $buttonStyle ] )
+                               true,
+                               'FOO mw-ui-button BAR',
+                               array_merge( $defaultStyles, [ $buttonStyle, $feedStyle ] )
                        ],
                ];
        }
 
        /**
-        * @param PHPUnit_Framework_MockObject_MockObject|OutputPage $outputPageMock
-        * @param string[] $expectedModuleStyles
-        *
-        * @covers SkinTemplate::setupSkinUserCss
-        * @dataProvider provideSetupSkinUserCss
+        * @covers Skin::getDefaultModules
+        * @dataProvider provideGetDefaultModules
         */
-       public function testSetupSkinUserCss( $outputPageMock, $expectedModuleStyles ) {
-               $outputPageMock->expects( $this->once() )
-                       ->method( 'addModuleStyles' )
-                       ->with( $expectedModuleStyles );
+       public function testgetDefaultModules( $isSyndicated, $html, $expectedModuleStyles ) {
+               $skin = new SkinTemplate();
 
-               $skinTemplate = new SkinTemplate();
-               $skinTemplate->setupSkinUserCss( $outputPageMock );
+               $context = new DerivativeContext( $skin->getContext() );
+               $context->setOutput( $this->getMockOutputPage( $isSyndicated, $html ) );
+               $skin->setContext( $context );
+
+               $modules = $skin->getDefaultModules();
+
+               $actualStylesModule = call_user_func_array( 'array_merge', $modules['styles'] );
+               $this->assertArraySubset(
+                       $expectedModuleStyles,
+                       $actualStylesModule,
+                       'style modules'
+               );
        }
 }