jenkins-bot has submitted this change and it was merged. Change subject: Explicitly define module position ......................................................................
Explicitly define module position Style modules currently added through addModuleStyles default to being in the head ("top" position). This is an unhealthy default, since only critical styles that are needed at pageload should be in the head. In order to be able to switch the default to "bottom", existing module positions have to be defined explicitly. Bug: T97410 Change-Id: I31e3e68487d91d0bee2be5b0b4d1c63344830c67 (cherry picked from commit 3e05e3810ed50486cbbf305720a911152ceeef56) --- M GlobalCssJs.hooks.php M ResourceLoaderGlobalModule.php A ResourceLoaderGlobalSiteCssModule.php A ResourceLoaderGlobalSiteJsModule.php M ResourceLoaderGlobalSiteModule.php A ResourceLoaderGlobalUserCssModule.php A ResourceLoaderGlobalUserJsModule.php M ResourceLoaderGlobalUserModule.php M extension.json M tests/ResourceLoaderGlobalModuleTest.php M tests/ResourceLoaderGlobalSiteModuleTest.php M tests/ResourceLoaderGlobalUserModuleTest.php 12 files changed, 285 insertions(+), 59 deletions(-) Approvals: Legoktm: Looks good to me, approved jenkins-bot: Verified diff --git a/GlobalCssJs.hooks.php b/GlobalCssJs.hooks.php index 4bf994e..67feef1 100644 --- a/GlobalCssJs.hooks.php +++ b/GlobalCssJs.hooks.php @@ -25,7 +25,7 @@ * @return bool */ static function onBeforePageDisplay( OutputPage $out ) { - $out->addModuleStyles( array( 'ext.globalCssJs.user', 'ext.globalCssJs.site' ) ); + $out->addModuleStyles( array( 'ext.globalCssJs.user.styles', 'ext.globalCssJs.site.styles' ) ); $out->addModuleScripts( array( 'ext.globalCssJs.user', 'ext.globalCssJs.site' ) ); return true; @@ -57,15 +57,27 @@ return true; } - $user = array( - 'class' => 'ResourceLoaderGlobalUserModule', + $userJs = array( + 'class' => 'ResourceLoaderGlobalUserJsModule', ) + $config; - $resourceLoader->register( 'ext.globalCssJs.user', $user ); + $resourceLoader->register( 'ext.globalCssJs.user', $userJs ); - $site = array( - 'class' => 'ResourceLoaderGlobalSiteModule', + $userCss = array( + 'position' => 'top', + 'class' => 'ResourceLoaderGlobalUserCssModule', ) + $config; - $resourceLoader->register( 'ext.globalCssJs.site', $site ); + $resourceLoader->register( 'ext.globalCssJs.user.styles', $userCss ); + + $siteJs = array( + 'class' => 'ResourceLoaderGlobalSiteJsModule', + ) + $config; + $resourceLoader->register( 'ext.globalCssJs.site', $siteJs ); + + $siteCss = array( + 'position' => 'top', + 'class' => 'ResourceLoaderGlobalSiteCssModule', + ) + $config; + $resourceLoader->register( 'ext.globalCssJs.site.styles', $siteCss ); return true; } diff --git a/ResourceLoaderGlobalModule.php b/ResourceLoaderGlobalModule.php index daedaca..7117f88 100644 --- a/ResourceLoaderGlobalModule.php +++ b/ResourceLoaderGlobalModule.php @@ -40,9 +40,21 @@ */ protected $source; + /** @var string Position on the page to load this module at */ + protected $position = 'bottom'; + public function __construct( $options ) { - $this->wiki = $options['wiki']; - $this->source = $options['source']; + foreach ( $options as $member => $option ) { + switch ( $member ) { + case 'position': + $this->isPositionDefined = true; + // fall through + case 'wiki': + case 'source': + $this->{$member} = (string)$option; + break; + } + } } /** @@ -63,4 +75,7 @@ } } + public function getPosition() { + return $this->position; + } } diff --git a/ResourceLoaderGlobalSiteCssModule.php b/ResourceLoaderGlobalSiteCssModule.php new file mode 100644 index 0000000..7ecf5f6 --- /dev/null +++ b/ResourceLoaderGlobalSiteCssModule.php @@ -0,0 +1,40 @@ +<?php +/** + * ResourceLoader module for global site customizations. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @author Kunal Mehta + */ + +/** + * Module for sitewide global CSS customizations + */ +class ResourceLoaderGlobalSiteCssModule extends ResourceLoaderGlobalSiteModule { + protected function doGetPages( ResourceLoaderContext $context ) { + $pages = array(); + + $config = $context->getResourceLoader()->getConfig(); + + if ( $config->get( 'UseSiteCss' ) ) { + $pages["MediaWiki:Global.css"] = array( 'type' => 'style' ); + $pages['MediaWiki:Global-' . $context->getSkin() . '.css'] = array( 'type' => 'style' ); + } + + return $pages; + } +} diff --git a/ResourceLoaderGlobalSiteJsModule.php b/ResourceLoaderGlobalSiteJsModule.php new file mode 100644 index 0000000..d079fb2 --- /dev/null +++ b/ResourceLoaderGlobalSiteJsModule.php @@ -0,0 +1,40 @@ +<?php +/** + * ResourceLoader module for global site customizations. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @author Kunal Mehta + */ + +/** + * Module for sitewide global JS customizations + */ +class ResourceLoaderGlobalSiteJsModule extends ResourceLoaderGlobalSiteModule { + protected function doGetPages( ResourceLoaderContext $context ) { + $pages = array(); + + $config = $context->getResourceLoader()->getConfig(); + + if ( $config->get( 'UseSiteJs' ) ) { + $pages["MediaWiki:Global.js"] = array( 'type' => 'script' ); + $pages['MediaWiki:Global-' . $context->getSkin() . '.js'] = array( 'type' => 'script' ); + } + + return $pages; + } +} diff --git a/ResourceLoaderGlobalSiteModule.php b/ResourceLoaderGlobalSiteModule.php index 9c1c3dd..061ae72 100644 --- a/ResourceLoaderGlobalSiteModule.php +++ b/ResourceLoaderGlobalSiteModule.php @@ -24,7 +24,7 @@ /** * Module for sitewide global customizations */ -class ResourceLoaderGlobalSiteModule extends ResourceLoaderGlobalModule { +abstract class ResourceLoaderGlobalSiteModule extends ResourceLoaderGlobalModule { protected $origin = self::ORIGIN_USER_SITEWIDE; @@ -37,23 +37,11 @@ return array(); } - $pages = array(); - - $config = $context->getResourceLoader()->getConfig(); - - if ( $config->get( 'UseSiteJs' ) ) { - $pages["MediaWiki:Global.js"] = array( 'type' => 'script' ); - $pages['MediaWiki:Global-' . $context->getSkin() . '.js'] = array( 'type' => 'script' ); - } - - if ( $config->get( 'UseSiteCss' ) ) { - $pages["MediaWiki:Global.css"] = array( 'type' => 'style' ); - $pages['MediaWiki:Global-' . $context->getSkin() . '.css'] = array( 'type' => 'style' ); - } - - return $pages; + return $this->doGetPages( $context ); } + abstract protected function doGetPages( ResourceLoaderContext $context ); + /** * @return string */ diff --git a/ResourceLoaderGlobalUserCssModule.php b/ResourceLoaderGlobalUserCssModule.php new file mode 100644 index 0000000..e393730 --- /dev/null +++ b/ResourceLoaderGlobalUserCssModule.php @@ -0,0 +1,40 @@ +<?php +/** + * ResourceLoader module for global user customizations. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @author Szymon Świerkosz + * @author Kunal Mehta + */ + +/** + * Module for user CSS customizations - runs on all wikis + */ +class ResourceLoaderGlobalUserCssModule extends ResourceLoaderGlobalUserModule { + protected function doGetPages( ResourceLoaderContext $context, $userpage ) { + $pages = array(); + + $config = $context->getResourceLoader()->getConfig(); + + if ( $config->get( 'AllowUserCss' ) ) { + $pages["User:$userpage/global.css"] = array( 'type' => 'style' ); + } + + return $pages; + } +} diff --git a/ResourceLoaderGlobalUserJsModule.php b/ResourceLoaderGlobalUserJsModule.php new file mode 100644 index 0000000..932a014 --- /dev/null +++ b/ResourceLoaderGlobalUserJsModule.php @@ -0,0 +1,40 @@ +<?php +/** + * ResourceLoader module for global user customizations. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along + * with this program; if not, write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * http://www.gnu.org/copyleft/gpl.html + * + * @file + * @author Szymon Świerkosz + * @author Kunal Mehta + */ + +/** + * Module for user JS customizations - runs on all wikis + */ +class ResourceLoaderGlobalUserJsModule extends ResourceLoaderGlobalUserModule { + protected function doGetPages( ResourceLoaderContext $context, $userpage ) { + $pages = array(); + + $config = $context->getResourceLoader()->getConfig(); + + if ( $config->get( 'AllowUserJs' ) ) { + $pages["User:$userpage/global.js"] = array( 'type' => 'script' ); + } + + return $pages; + } +} diff --git a/ResourceLoaderGlobalUserModule.php b/ResourceLoaderGlobalUserModule.php index 6d21aab..6d30c55 100644 --- a/ResourceLoaderGlobalUserModule.php +++ b/ResourceLoaderGlobalUserModule.php @@ -25,7 +25,7 @@ /** * Module for user customizations - runs on all wikis */ -class ResourceLoaderGlobalUserModule extends ResourceLoaderGlobalModule { +abstract class ResourceLoaderGlobalUserModule extends ResourceLoaderGlobalModule { protected $origin = self::ORIGIN_USER_INDIVIDUAL; @@ -34,7 +34,6 @@ * @return array */ protected function getPages( ResourceLoaderContext $context ) { - $config = $context->getResourceLoader()->getConfig(); $username = $context->getUser(); if ( $username === null ) { @@ -53,19 +52,12 @@ } $userpage = $user->getUserPage()->getDBkey(); - $pages = array(); - if ( $config->get( 'AllowUserJs' ) ) { - $pages["User:$userpage/global.js"] = array( 'type' => 'script' ); - } - - if ( $config->get( 'AllowUserCss' ) ) { - $pages["User:$userpage/global.css"] = array( 'type' => 'style' ); - } - - return $pages; + return $this->doGetPages( $context, $userpage ); } + abstract protected function doGetPages( ResourceLoaderContext $context, $userpage ); + /** * @return string */ diff --git a/extension.json b/extension.json index 63c6895..3ac983d 100644 --- a/extension.json +++ b/extension.json @@ -46,7 +46,11 @@ "AutoloadClasses": { "ResourceLoaderGlobalModule": "ResourceLoaderGlobalModule.php", "ResourceLoaderGlobalSiteModule": "ResourceLoaderGlobalSiteModule.php", + "ResourceLoaderGlobalSiteCssModule": "ResourceLoaderGlobalSiteCssModule.php", + "ResourceLoaderGlobalSiteJsModule": "ResourceLoaderGlobalSiteJsModule.php", "ResourceLoaderGlobalUserModule": "ResourceLoaderGlobalUserModule.php", + "ResourceLoaderGlobalUserCssModule": "ResourceLoaderGlobalUserCssModule.php", + "ResourceLoaderGlobalUserJsModule": "ResourceLoaderGlobalUserJsModule.php", "GlobalCssJsHooks": "GlobalCssJs.hooks.php", "ResourceLoaderGlobalModuleTestCase": "tests/ResourceLoaderGlobalModuleTestCase.php" }, diff --git a/tests/ResourceLoaderGlobalModuleTest.php b/tests/ResourceLoaderGlobalModuleTest.php index 27cafad..850ff51 100644 --- a/tests/ResourceLoaderGlobalModuleTest.php +++ b/tests/ResourceLoaderGlobalModuleTest.php @@ -35,4 +35,19 @@ ), ); } + + /** + * Verify that all style modules are setting an explicit position + * + * @covers ResourceLoaderGlobalModule::isPositionDefault + */ + public function testIsPositionDefault() { + $this->setMwGlobals( + 'wgGlobalCssJsConfig', + array( 'wiki' => wfWikiID(), 'source' => 'fakesource' ) + ); + $rl = new ResourceLoader( ConfigFactory::getDefaultInstance()->makeConfig( 'main' ) ); + $this->assertTrue( !$rl->getModule( 'ext.globalCssJs.user.styles' )->isPositionDefault() ); + $this->assertTrue( !$rl->getModule( 'ext.globalCssJs.site.styles' )->isPositionDefault() ); + } } diff --git a/tests/ResourceLoaderGlobalSiteModuleTest.php b/tests/ResourceLoaderGlobalSiteModuleTest.php index e65f1ef..c713e1b 100644 --- a/tests/ResourceLoaderGlobalSiteModuleTest.php +++ b/tests/ResourceLoaderGlobalSiteModuleTest.php @@ -7,50 +7,86 @@ // format: array( array( config => value ), $expectedPages, $skin, $description ) return array( array( + 'ResourceLoaderGlobalSiteCssModule', array(), array( - 'MediaWiki:Global.js', 'MediaWiki:Global-skinname.js', 'MediaWiki:Global.css', 'MediaWiki:Global-skinname.css' ), 'skinname', - 'With default settings, 4 global pages are loaded' + 'With default settings, 2 CSS global pages are loaded' ), array( + 'ResourceLoaderGlobalSiteJsModule', + array(), + array( + 'MediaWiki:Global.js', 'MediaWiki:Global-skinname.js', + ), + 'skinname', + 'With default settings, 2 JS global pages are loaded' + ), + array( + 'ResourceLoaderGlobalSiteCssModule', array( 'wgUseGlobalSiteCssJs' => false), array(), 'skinname', - 'No pages are loaded with $wgUseGlobalSiteCssJs = false' + 'No CSS pages are loaded with $wgUseGlobalSiteCssJs = false' ), array( + 'ResourceLoaderGlobalSiteJsModule', + array( 'wgUseGlobalSiteCssJs' => false), + array(), + 'skinname', + 'No JS pages are loaded with $wgUseGlobalSiteCssJs = false' + ), + array( + 'ResourceLoaderGlobalSiteJsModule', array( 'wgUseSiteCss' => false ), array( 'MediaWiki:Global.js', 'MediaWiki:Global-skinname.js', ), 'skinname', - 'Only JS pages are loaded if $wgUseSiteCss = false' + 'JS pages are loaded if $wgUseSiteCss = false' ), array( + 'ResourceLoaderGlobalSiteCssModule', array( 'wgUseSiteJs' => false ), array( 'MediaWiki:Global.css', 'MediaWiki:Global-skinname.css', ), 'skinname', - 'Only CSS pages are loaded if $wgUseSiteJs = false' + 'CSS pages are loaded if $wgUseSiteJs = false' ), array( + 'ResourceLoaderGlobalSiteCssModule', array( 'wgUseSiteJs' => false, 'wgUseSiteCss' => false ), array(), 'skinname', - 'No pages loaded if $wgUseSiteJs and $wgUseSiteCss are false' + 'No CSS pages loaded if $wgUseSiteJs and $wgUseSiteCss are false' ), array( + 'ResourceLoaderGlobalSiteJsModule', + array( 'wgUseSiteJs' => false, 'wgUseSiteCss' => false ), + array(), + 'skinname', + 'No JS pages loaded if $wgUseSiteJs and $wgUseSiteCss are false' + ), + array( + 'ResourceLoaderGlobalSiteCssModule', array(), array( - 'MediaWiki:Global.js', 'MediaWiki:Global-monobook.js', 'MediaWiki:Global.css', 'MediaWiki:Global-monobook.css' ), 'monobook', - 'Global-monobook.js/css pages are loaded if monobook is set as the skin' + 'Global-monobook.css pages are loaded if monobook is set as the skin' + ), + array( + 'ResourceLoaderGlobalSiteJsModule', + array(), + array( + 'MediaWiki:Global.js', 'MediaWiki:Global-monobook.js' + ), + 'monobook', + 'Global-monobook.js pages are loaded if monobook is set as the skin' ), ); } @@ -58,18 +94,19 @@ /** * @covers ResourceLoaderGlobalSiteModule::getPages * @dataProvider provideGetPages + * @param $class * @param $configOverrides * @param $expectedPages * @param $skin * @param $desc */ - public function testGetPages( $configOverrides, $expectedPages, $skin, $desc ) { + public function testGetPages( $class, $configOverrides, $expectedPages, $skin, $desc ) { // First set default config options $this->setMwGlobals( array_merge( $this->getDefaultGlobalSettings( $skin ), $configOverrides ) ); - $module = new ResourceLoaderGlobalSiteModule( $this->getFakeOptions() ); + $module = new $class( $this->getFakeOptions() ); $context = $this->getContext( array( 'skin' => $skin ) ); $getPages = new ReflectionMethod( $module , 'getPages' ); $getPages->setAccessible( true ); diff --git a/tests/ResourceLoaderGlobalUserModuleTest.php b/tests/ResourceLoaderGlobalUserModuleTest.php index eb22960..df52681 100644 --- a/tests/ResourceLoaderGlobalUserModuleTest.php +++ b/tests/ResourceLoaderGlobalUserModuleTest.php @@ -17,81 +17,84 @@ // format: array( array( config => value ), $expectedPages, $description ) return array( array( + 'ResourceLoaderGlobalUserCssModule', array(), array(), 'TestUser', 'With default settings, no pages are loaded' ), array( + 'ResourceLoaderGlobalUserJsModule', array( 'wgAllowUserJs' => true ), array( 'User:TestUser/global.js', ), 'TestUser', - 'Only JS page is loaded if $wgAllowUserJs = true' + 'JS page is loaded if $wgAllowUserJs = true' ), array( + 'ResourceLoaderGlobalUserCssModule', array( 'wgAllowUserCss' => true ), array( 'User:TestUser/global.css', ), 'TestUser', - 'Only CSS page is loaded if $wgAllowUserCss = true' + 'CSS page is loaded if $wgAllowUserCss = true' ), array( - array( 'wgAllowUserCss' => true, 'wgAllowUserJs' => true ), - array( 'User:TestUser/global.js', 'User:TestUser/global.css' ), - 'TestUser', - '2 global pages loaded if both $wgAllowUserCss and $wgAllowUserJs = true' - ), - array( + 'ResourceLoaderGlobalUserCssModule', array( 'wgLanguageCode' => 'zh', 'wgAllowUserCss' => true, 'wgAllowUserJs' => true ), - array( 'User:TestUser/global.js', 'User:TestUser/global.css' ), + array( 'User:TestUser/global.css' ), 'TestUser', 'User: namespace used in page titles even if $wgLanguageCode != "en"' ), array( + 'ResourceLoaderGlobalUserJsModule', array( 'wgGlobalCssJsConfig' => array( 'wiki' => false ) ), array(), 'TestUser', "If \$wgGlobalCssJsConfig['wiki'] = false, no pages are loaded", ), array( + 'ResourceLoaderGlobalUserCssModule', array( 'wgAllowUserCss' => true, 'wgAllowUserJs' => true ), array(), null, 'No pages loaded if $username = null', ), array( + 'ResourceLoaderGlobalUserJsModule', array( 'wgAllowUserCss' => true, 'wgAllowUserJs' => true ), array(), '[Invalid@Username]', 'No pages loaded if username is invalid', ), array( + 'ResourceLoaderGlobalUserCssModule', array( 'wgAllowUserCss' => true, 'wgAllowUserJs' => true ), array(), 'UserThatHopefullyDoesntExist12', 'No pages loaded if user doesnt exist', - ), + ) ); } /** * @covers ResourceLoaderGlobalUserModule::getPages * @dataProvider provideGetPages + * @param $class * @param $configOverrides * @param $expectedPages * @param $user * @param $desc */ - public function testGetPages( $configOverrides, $expectedPages, $user, $desc ) { + public function testGetPages( $class, $configOverrides, $expectedPages, $user, $desc ) { // First set default config options $this->setMwGlobals( array_merge( $this->getDefaultGlobalSettings(), $configOverrides ) ); - $module = new ResourceLoaderGlobalUserModule( $this->getFakeOptions() ); + $module = new $class( $this->getFakeOptions() ); $context = $this->getContext( array( 'user' => $user ) ); $getPages = new ReflectionMethod( $module , 'getPages' ); $getPages->setAccessible( true ); -- To view, visit https://gerrit.wikimedia.org/r/214399 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: I31e3e68487d91d0bee2be5b0b4d1c63344830c67 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/GlobalCssJs Gerrit-Branch: wmf/1.26wmf8 Gerrit-Owner: Legoktm <legoktm.wikipe...@gmail.com> Gerrit-Reviewer: Gilles <gdu...@wikimedia.org> Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits