From 9849db9c79fcc7e6cdca8ccc311aea4265ff337e Mon Sep 17 00:00:00 2001 From: Timo Tijhof Date: Wed, 25 Apr 2018 00:22:00 +0100 Subject: [PATCH] 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 --- RELEASE-NOTES-1.32 | 3 ++ includes/installer/WebInstallerOutput.php | 4 +- includes/skins/Skin.php | 49 +++++++++++++------ includes/skins/SkinTemplate.php | 23 --------- .../includes/skins/SkinTemplateTest.php | 45 ++++++++++------- 5 files changed, 66 insertions(+), 58 deletions(-) diff --git a/RELEASE-NOTES-1.32 b/RELEASE-NOTES-1.32 index a437eb9029..72f877fc06 100644 --- 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 6a55d69690..cb0092d2a6 100644 --- 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 2247cc2ca6..340bc2f5bd 100644 --- 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 9461bcff2a..1d5d534ace 100644 --- 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 b746fc3615..6ea5b40b0b 100644 --- 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' + ); } } -- 2.20.1