From 8cd2e1336354ea8459f019cd989c4efe2516fd5f Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Tue, 7 Jan 2020 11:18:51 -0800 Subject: [PATCH] Deprecate access of logos directly from config, introduce wgLogos Add getAvailableLogos static method and wgLogos config variable Longterm we'll phase out wgLogo and wgLogoHD for this more extendable config. wgLogoHD is marked as deprecated. wgLogo continues to function as before when wgLogos doesn't exist to cause minimum disruption. From now on all logos should be accessed via getAvailableLogos. Patches in Minerva and Vector follow. See I00899c16c0325f36b671baf17e88c2b5187b3526, I569e0d800e147eabc7852567acd140108613f074 and I013bd0904fe8c55efa49d14e84cf06ec1412896f. Bug: T232140 Change-Id: I66a971631c623cc94b58eb0e5e5bad804789bf1c --- includes/DefaultSettings.php | 26 ++++ includes/Setup.php | 6 + includes/api/ApiQuerySiteinfo.php | 5 +- .../ResourceLoaderSkinModule.php | 55 +++++++- includes/skins/Skin.php | 7 +- includes/skins/SkinTemplate.php | 7 +- tests/phpunit/includes/OutputPageTest.php | 20 +-- .../ResourceLoaderSkinModuleTest.php | 119 +++++++++++++++--- 8 files changed, 203 insertions(+), 42 deletions(-) diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index a71c8099996..8529755c64e 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -264,9 +264,31 @@ $wgFileCacheDirectory = false; /** * The URL path of the wiki logo. The logo size should be 135x135 pixels. * Defaults to "$wgResourceBasePath/resources/assets/wiki.png". + * Developers should retrieve this logo (and other variants) using + * the static function ResourceLoaderSkinModule::getAvailableLogos + * Ignored if wgLogos is set. */ $wgLogo = false; +/** + * The URL path to various wiki logos. + * The `1x` logo size should be 135x135 pixels. + * The `1.5x` 1.5x version of square logo + * The `2x` 2x version of square logo + * The `svg` version of square logo + * The `wordmark` key should point to an array with the following fields + * - `src` relative or absolute path to a landscape logo + * - `width` defining the width of the logo in pixels. + * - `height` defining the height of the logo in pixels. + * All values can be either an absolute or relative URI + * Configuration is optional provided wgLogo is used instead. + * Defaults to [ "1x" => $wgLogo ], + * or [ "1x" => "$wgResourceBasePath/resources/assets/wiki.png" ] if $wgLogo is not set. + * @since 1.35 + * @var array|false + */ +$wgLogos = false; + /** * Array with URL paths to HD versions of the wiki logo. The scaled logo size * should be under 135x155 pixels. @@ -291,7 +313,11 @@ $wgLogo = false; * ]; * @endcode * + * @var array|false * @since 1.25 + * @deprecated since 1.35. Developers should retrieve this logo (and other variants) using + * the static function ResourceLoaderSkinModule::getAvailableLogos. wgLogos should be used + * instead. */ $wgLogoHD = false; diff --git a/includes/Setup.php b/includes/Setup.php index d36ad79ae62..4687283e507 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -179,6 +179,12 @@ if ( $wgExtensionAssetsPath === false ) { $wgExtensionAssetsPath = "$wgResourceBasePath/extensions"; } +// For backwards compatibility, the value of wgLogos is copied to wgLogo. +// This is because some extensions/skins may be using $config->get('Logo') +// to access the value. +if ( $wgLogos !== false ) { + $wgLogo = $wgLogos['1x']; +} if ( $wgLogo === false ) { $wgLogo = "$wgResourceBasePath/resources/assets/wiki.png"; } diff --git a/includes/api/ApiQuerySiteinfo.php b/includes/api/ApiQuerySiteinfo.php index ef0833bc668..264f86e5a3e 100644 --- a/includes/api/ApiQuerySiteinfo.php +++ b/includes/api/ApiQuerySiteinfo.php @@ -134,9 +134,10 @@ class ApiQuerySiteinfo extends ApiQueryBase { $data['sitename'] = $config->get( 'Sitename' ); $data['mainpageisdomainroot'] = (bool)$config->get( 'MainPageIsDomainRoot' ); - // wgLogo can either be a relative or an absolute path + // A logo can either be a relative or an absolute path // make sure we always return an absolute path - $data['logo'] = wfExpandUrl( $config->get( 'Logo' ), PROTO_RELATIVE ); + $logo = ResourceLoaderSkinModule::getAvailableLogos( $config ); + $data['logo'] = wfExpandUrl( $logo['1x'], PROTO_RELATIVE ); $data['generator'] = "MediaWiki {$config->get( 'Version' )}"; diff --git a/includes/resourceloader/ResourceLoaderSkinModule.php b/includes/resourceloader/ResourceLoaderSkinModule.php index 8ca62b750f4..bb604d1face 100644 --- a/includes/resourceloader/ResourceLoaderSkinModule.php +++ b/includes/resourceloader/ResourceLoaderSkinModule.php @@ -300,6 +300,52 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule { } } + /** + * Return an array of all available logos that a skin may use. + * @since 1.35 + * @param Config $conf + * @return array with the following keys: + * - 1x: a square logo (required) + * - 2x: a square logo for HD displays (optional) + * - wordmark: a rectangle logo (wordmark) for print media and skins which desire + * horizontal logo (optional) + */ + public static function getAvailableLogos( $conf ) { + $logos = $conf->get( 'Logos' ); + if ( $logos === false ) { + // no logos were defined... this will either + // 1. Load from wgLogo and wgLogoHD + // 2. Trigger runtime exception if those are not defined. + $logos = []; + } + + // If logos['1x'] is not defined, see if we can use wgLogo + if ( !isset( $logos[ '1x' ] ) ) { + $logo = $conf->get( 'Logo' ); + if ( $logo ) { + $logos['1x'] = $logo; + } + } + + try { + $logoHD = $conf->get( 'LogoHD' ); + // make sure not false + if ( $logoHD ) { + wfDeprecated( '$wgLogoHD', '1.35', 'Rename configuration variable to $wgLogos' ); + $logos += $logoHD; + } + } catch ( ConfigException $e ) { + // no backwards compatibility changes needed. + } + + // check the configuration is valid + if ( !isset( $logos['1x'] ) ) { + throw new \RuntimeException( "The key `1x` is required for wgLogos or wgLogo must be defined." ); + } + // return the modified logos! + return $logos; + } + /** * @since 1.31 * @param Config $conf @@ -309,12 +355,12 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule { * in which case variants other than "1x" are omitted. */ protected function getLogoData( Config $conf ) { - $logo = $conf->get( 'Logo' ); - $logoHD = $conf->get( 'LogoHD' ); + $logoHD = self::getAvailableLogos( $conf ); + $logo = $logoHD['1x']; $logo1Url = OutputPage::transformResourcePath( $conf, $logo ); - if ( !$logoHD ) { + if ( count( $logoHD ) === 1 ) { return $logo1Url; } @@ -359,8 +405,7 @@ class ResourceLoaderSkinModule extends ResourceLoaderFileModule { public function getDefinitionSummary( ResourceLoaderContext $context ) { $summary = parent::getDefinitionSummary( $context ); $summary[] = [ - 'logo' => $this->getConfig()->get( 'Logo' ), - 'logoHD' => $this->getConfig()->get( 'LogoHD' ), + 'logos' => self::getAvailableLogos( $this->getConfig() ), ]; return $summary; } diff --git a/includes/skins/Skin.php b/includes/skins/Skin.php index ec13ed46d99..d5797ae2b29 100644 --- a/includes/skins/Skin.php +++ b/includes/skins/Skin.php @@ -495,11 +495,12 @@ abstract class Skin extends ContextSource { } /** - * URL to the logo + * URL to the default square logo (1x key) + * Please use ResourceLoaderSkinModule::getAvailableLogos * @return string */ function getLogo() { - return $this->getConfig()->get( 'Logo' ); + return ResourceLoaderSkinModule::getAvailableLogos( $this->getConfig() )[ '1x' ]; } /** @@ -507,7 +508,7 @@ abstract class Skin extends ContextSource { * * @deprecated since 1.32 Redundant. It now happens automatically based on whether * the skin loads a stylesheet based on ResourceLoaderSkinModule, which all - * skins that use wgLogo in CSS do, and other's would not. + * skins that use wgLogos in CSS do, and other's would not. * @since 1.29 * @return bool */ diff --git a/includes/skins/SkinTemplate.php b/includes/skins/SkinTemplate.php index 601133341ff..34da1ae6eac 100644 --- a/includes/skins/SkinTemplate.php +++ b/includes/skins/SkinTemplate.php @@ -257,7 +257,7 @@ class SkinTemplate extends Skin { */ protected function prepareQuickTemplate() { global $wgScript, $wgStylePath, $wgMimeType, - $wgSitename, $wgLogo, $wgMaxCredits, + $wgSitename, $wgMaxCredits, $wgShowCreditsIfMax, $wgArticlePath, $wgScriptPath, $wgServer; @@ -323,7 +323,8 @@ class SkinTemplate extends Skin { $tpl->set( 'articlepath', $wgArticlePath ); $tpl->set( 'scriptpath', $wgScriptPath ); $tpl->set( 'serverurl', $wgServer ); - $tpl->set( 'logopath', $wgLogo ); + $logos = ResourceLoaderSkinModule::getAvailableLogos( $this->getConfig() ); + $tpl->set( 'logopath', $logos['1x'] ); $tpl->set( 'sitename', $wgSitename ); $userLang = $this->getLanguage(); @@ -582,7 +583,7 @@ class SkinTemplate extends Skin { * Output a boolean indicating if buildPersonalUrls should output separate * login and create account links or output a combined link * By default we simply return a global config setting that affects most skins - * This is setup as a method so that like with $wgLogo and getLogo() a skin + * This is setup as a method so that like with $wgLogos and getLogo() a skin * can override this setting and always output one or the other if it has * a reason it can't output one of the two modes. * @return bool diff --git a/tests/phpunit/includes/OutputPageTest.php b/tests/phpunit/includes/OutputPageTest.php index 5a2ab8d0633..92030ca591e 100644 --- a/tests/phpunit/includes/OutputPageTest.php +++ b/tests/phpunit/includes/OutputPageTest.php @@ -2904,7 +2904,7 @@ class OutputPageTest extends MediaWikiTestCase { [ 'wgResourceBasePath' => '/w', 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogos' => [ '1.5x' => '/img/one-point-five.png', '2x' => '/img/two-x.png', ], @@ -2918,16 +2918,17 @@ class OutputPageTest extends MediaWikiTestCase { [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/img/default.png', + ], ], 'Link: ;rel=preload;as=image' ], [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogos' => [ + '1x' => '/img/default.png', '2x' => '/img/two-x.png', ], ], @@ -2938,8 +2939,8 @@ class OutputPageTest extends MediaWikiTestCase { [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogos' => [ + '1x' => '/img/default.png', 'svg' => '/img/vector.svg', ], ], @@ -2949,8 +2950,9 @@ class OutputPageTest extends MediaWikiTestCase { [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/w/test.jpg', - 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/w/test.jpg', + ], 'wgUploadPath' => '/w/images', 'IP' => dirname( __DIR__ ) . '/data/media', ], diff --git a/tests/phpunit/includes/resourceloader/ResourceLoaderSkinModuleTest.php b/tests/phpunit/includes/resourceloader/ResourceLoaderSkinModuleTest.php index 5e994c58b71..0f7f3bbacda 100644 --- a/tests/phpunit/includes/resourceloader/ResourceLoaderSkinModuleTest.php +++ b/tests/phpunit/includes/resourceloader/ResourceLoaderSkinModuleTest.php @@ -7,6 +7,50 @@ use Wikimedia\TestingAccessWrapper; */ class ResourceLoaderSkinModuleTest extends MediaWikiTestCase { + public static function provideGetAvailableLogos() { + return [ + [ + [ + 'Logos' => [], + 'Logo' => '/logo.png', + ], + [ + '1x' => '/logo.png', + ] + ], + [ + [ + 'Logos' => [ + 'svg' => '/logo.svg', + '2x' => 'logo-2x.png' + ], + 'Logo' => '/logo.png', + ], + [ + 'svg' => '/logo.svg', + '2x' => 'logo-2x.png', + '1x' => '/logo.png', + ] + ], + [ + [ + 'Logos' => [ + 'wordmark' => '/logo-wordmark.png', + '1x' => '/logo.png', + 'svg' => '/logo.svg', + '2x' => 'logo-2x.png' + ], + ], + [ + 'wordmark' => '/logo-wordmark.png', + '1x' => '/logo.png', + 'svg' => '/logo.svg', + '2x' => 'logo-2x.png', + ] + ] + ]; + } + public static function provideGetStyles() { // phpcs:disable Generic.Files.LineLength return [ @@ -93,6 +137,27 @@ CSS ); } + /** + * @dataProvider provideGetAvailableLogos + * @covers ResourceLoaderSkinModule::getAvailableLogos + */ + public function testGetAvailableLogos( $config, $expected ) { + $logos = ResourceLoaderSkinModule::getAvailableLogos( new HashConfig( $config ) ); + $this->assertSame( $logos, $expected ); + } + + /** + * @covers ResourceLoaderSkinModule::getAvailableLogos + */ + public function testGetAvailableLogosRuntimeException() { + $this->expectException( \RuntimeException::class ); + ResourceLoaderSkinModule::getAvailableLogos( new HashConfig( [ + 'Logo' => false, + 'Logos' => false, + 'LogoHD' => false, + ] ) ); + } + /** * @covers ResourceLoaderSkinModule::isKnownEmpty */ @@ -127,16 +192,17 @@ CSS 'simple' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/img/default.png', - 'LogoHD' => false, + 'Logos' => [ + '1x' => '/img/default.png', + ], ], 'expected' => '/img/default.png', ], 'default and 2x' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/img/default.png', - 'LogoHD' => [ + 'Logos' => [ + '1x' => '/img/default.png', '2x' => '/img/two-x.png', ], ], @@ -148,8 +214,8 @@ CSS 'default and all HiDPIs' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/img/default.png', - 'LogoHD' => [ + 'Logos' => [ + '1x' => '/img/default.png', '1.5x' => '/img/one-point-five.png', '2x' => '/img/two-x.png', ], @@ -163,8 +229,8 @@ CSS 'default and SVG' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/img/default.png', - 'LogoHD' => [ + 'Logos' => [ + '1x' => '/img/default.png', 'svg' => '/img/vector.svg', ], ], @@ -176,8 +242,8 @@ CSS 'everything' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/img/default.png', - 'LogoHD' => [ + 'Logos' => [ + '1x' => '/img/default.png', '1.5x' => '/img/one-point-five.png', '2x' => '/img/two-x.png', 'svg' => '/img/vector.svg', @@ -191,9 +257,10 @@ CSS 'versioned url' => [ 'config' => [ 'ResourceBasePath' => '/w', - 'Logo' => '/w/test.jpg', - 'LogoHD' => false, 'UploadPath' => '/w/images', + 'Logos' => [ + '1x' => '/w/test.jpg', + ], ], 'expected' => '/w/test.jpg?edcf2', 'baseDir' => dirname( dirname( __DIR__ ) ) . '/data/media', @@ -221,8 +288,10 @@ CSS [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogo' => false, + 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/img/default.png', '1.5x' => '/img/one-point-five.png', '2x' => '/img/two-x.png', ], @@ -236,16 +305,21 @@ CSS [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', + 'wgLogo' => false, 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/img/default.png', + ], ], 'Link: ;rel=preload;as=image' ], [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogo' => false, + 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/img/default.png', '2x' => '/img/two-x.png', ], ], @@ -256,8 +330,10 @@ CSS [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/img/default.png', - 'wgLogoHD' => [ + 'wgLogo' => false, + 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/img/default.png', 'svg' => '/img/vector.svg', ], ], @@ -267,8 +343,11 @@ CSS [ [ 'wgResourceBasePath' => '/w', - 'wgLogo' => '/w/test.jpg', + 'wgLogo' => false, 'wgLogoHD' => false, + 'wgLogos' => [ + '1x' => '/w/test.jpg', + ], 'wgUploadPath' => '/w/images', 'IP' => dirname( dirname( __DIR__ ) ) . '/data/media', ],