From f68b218076005404170e487ec50e575b17a61e97 Mon Sep 17 00:00:00 2001 From: Fomafix Date: Sat, 23 Mar 2024 21:43:33 +0000 Subject: [PATCH] LanguagesHelper: Move $out from contructor to doesTitleHasLanguagesOrVariants Services MUST NOT vary their behaviour on global state, especially not WebRequest, RequestContext (T218555). Change-Id: I4b73ef713166d8b254023c1eebbb45c963880a99 --- includes/LanguagesHelper.php | 18 +++++------ includes/Menu/PageActions/ToolbarBuilder.php | 30 ++++++++++--------- .../UserNamespaceOverflowBuilder.php | 19 ++++++++---- .../Permissions/MinervaPagePermissions.php | 6 +++- includes/ServiceWiring.php | 3 +- includes/Skins/SkinMinerva.php | 5 +++- tests/phpunit/LanguagesHelperTest.php | 8 +++-- 7 files changed, 52 insertions(+), 37 deletions(-) diff --git a/includes/LanguagesHelper.php b/includes/LanguagesHelper.php index afa8d8d99..564b3e177 100644 --- a/includes/LanguagesHelper.php +++ b/includes/LanguagesHelper.php @@ -31,30 +31,26 @@ class LanguagesHelper { private LanguageConverterFactory $languageConverterFactory; - /** - * @var bool - */ - private $hasLanguages; - /** * @param LanguageConverterFactory $languageConverterFactory - * @param OutputPage $out Output page to fetch language links */ public function __construct( - LanguageConverterFactory $languageConverterFactory, - OutputPage $out + LanguageConverterFactory $languageConverterFactory ) { $this->languageConverterFactory = $languageConverterFactory; - $this->hasLanguages = $out->getLanguageLinks() !== []; } /** * Whether the Title is also available in other languages or variants + * @param OutputPage $out Output page to fetch language links * @param Title $title * @return bool */ - public function doesTitleHasLanguagesOrVariants( Title $title ) { - if ( $this->hasLanguages ) { + public function doesTitleHasLanguagesOrVariants( + OutputPage $out, + Title $title + ) { + if ( $out->getLanguageLinks() !== [] ) { return true; } $langConv = $this->languageConverterFactory->getLanguageConverter( $title->getPageLanguage() ); diff --git a/includes/Menu/PageActions/ToolbarBuilder.php b/includes/Menu/PageActions/ToolbarBuilder.php index e018f5d8e..932f1d881 100644 --- a/includes/Menu/PageActions/ToolbarBuilder.php +++ b/includes/Menu/PageActions/ToolbarBuilder.php @@ -22,6 +22,7 @@ namespace MediaWiki\Minerva\Menu\PageActions; use ExtensionRegistry; use MediaWiki\Config\ServiceOptions; +use MediaWiki\Context\IContextSource; use MediaWiki\Minerva\LanguagesHelper; use MediaWiki\Minerva\Menu\Entries\IMenuEntry; use MediaWiki\Minerva\Menu\Entries\LanguageSelectorEntry; @@ -35,7 +36,6 @@ use MediaWiki\Title\Title; use MediaWiki\User\User; use MediaWiki\User\UserIdentity; use MediaWiki\Watchlist\WatchlistManager; -use MessageLocalizer; use SpecialMobileHistory; class ToolbarBuilder { @@ -48,10 +48,9 @@ class ToolbarBuilder { * @var Title Article title user is currently browsing */ private $title; - /** - * @var MessageLocalizer Message localizer to generate localized texts - */ - private $messageLocalizer; + + private IContextSource $context; + /** * @var IMinervaPagePermissions */ @@ -93,7 +92,7 @@ class ToolbarBuilder { * Build Group containing icons for toolbar * @param Title $title Article title user is currently browsing * @param User $user Currently logged in user - * @param MessageLocalizer $msgLocalizer Message localizer to generate localized texts + * @param IContextSource $context * @param IMinervaPagePermissions $permissions Minerva permissions system * @param SkinOptions $skinOptions * @param SkinUserPageHelper $relevantUserPageHelper User Page helper. The @@ -107,7 +106,7 @@ class ToolbarBuilder { public function __construct( Title $title, User $user, - MessageLocalizer $msgLocalizer, + IContextSource $context, IMinervaPagePermissions $permissions, SkinOptions $skinOptions, SkinUserPageHelper $relevantUserPageHelper, @@ -117,7 +116,7 @@ class ToolbarBuilder { ) { $this->title = $title; $this->user = $user; - $this->messageLocalizer = $msgLocalizer; + $this->context = $context; $this->permissions = $permissions; $this->skinOptions = $skinOptions; $this->relevantUserPageHelper = $relevantUserPageHelper; @@ -141,8 +140,11 @@ class ToolbarBuilder { IMinervaPagePermissions::SWITCH_LANGUAGE ) ) { $group->insertEntry( new LanguageSelectorEntry( $this->title, - $this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->title ), - $this->messageLocalizer, + $this->languagesHelper->doesTitleHasLanguagesOrVariants( + $this->context->getOutput(), + $this->title + ), + $this->context, true ) ); } @@ -153,7 +155,7 @@ class ToolbarBuilder { 'icon' => 'star', 'class' => '', 'href' => $this->getLoginUrl( [ 'returnto' => $this->title ] ), - 'text' => $this->messageLocalizer->msg( 'watch' ), + 'text' => $this->context->msg( 'watch' ), ]; if ( $permissions->isAllowed( IMinervaPagePermissions::WATCHABLE ) && $watchData ) { $group->insertEntry( $this->createWatchPageAction( $watchKey, $watchData ) ); @@ -192,7 +194,7 @@ class ToolbarBuilder { * @return IMenuEntry */ protected function createContributionsPageAction( UserIdentity $user ): IMenuEntry { - $label = $this->messageLocalizer->msg( 'mobile-frontend-user-page-contributions' ); + $label = $this->context->msg( 'mobile-frontend-user-page-contributions' ); $entry = new SingleMenuEntry( 'page-actions-contributions', @@ -227,7 +229,7 @@ class ToolbarBuilder { $icon = $editAction['icon'] ?? $iconFallback; $entry->setIcon( $icon . '-base20' ) ->trackClicks( $key ) - ->setTitle( $this->messageLocalizer->msg( 'tooltip-' . $id ) ) + ->setTitle( $this->context->msg( 'tooltip-' . $id ) ) ->setNodeID( $id ); return $entry; } @@ -255,7 +257,7 @@ class ToolbarBuilder { } return $entry->trackClicks( $watchKey ) ->setIcon( $icon ) - ->setTitle( $this->messageLocalizer->msg( $watchKey ) ) + ->setTitle( $this->context->msg( $watchKey ) ) ->setNodeID( 'ca-watch' ); } diff --git a/includes/Menu/PageActions/UserNamespaceOverflowBuilder.php b/includes/Menu/PageActions/UserNamespaceOverflowBuilder.php index 5fb4db70e..ff8a3fbfb 100644 --- a/includes/Menu/PageActions/UserNamespaceOverflowBuilder.php +++ b/includes/Menu/PageActions/UserNamespaceOverflowBuilder.php @@ -20,15 +20,20 @@ namespace MediaWiki\Minerva\Menu\PageActions; +use MediaWiki\Context\IContextSource; use MediaWiki\Minerva\LanguagesHelper; use MediaWiki\Minerva\Menu\Entries\LanguageSelectorEntry; use MediaWiki\Minerva\Menu\Group; use MediaWiki\Minerva\Permissions\IMinervaPagePermissions; use MediaWiki\Title\Title; -use MessageLocalizer; class UserNamespaceOverflowBuilder extends DefaultOverflowBuilder { + /** + * @var IContextSource + */ + private $context; + /** * @var LanguagesHelper */ @@ -37,18 +42,19 @@ class UserNamespaceOverflowBuilder extends DefaultOverflowBuilder { /** * Initialize the overflow menu visible on the User namespace * @param Title $title - * @param MessageLocalizer $msgLocalizer + * @param IContextSource $context * @param IMinervaPagePermissions $permissions * @param LanguagesHelper $languagesHelper */ public function __construct( Title $title, - MessageLocalizer $msgLocalizer, + IContextSource $context, IMinervaPagePermissions $permissions, LanguagesHelper $languagesHelper ) { + $this->context = $context; $this->languagesHelper = $languagesHelper; - parent::__construct( $title, $msgLocalizer, $permissions ); + parent::__construct( $title, $context, $permissions ); } /** @@ -60,7 +66,10 @@ class UserNamespaceOverflowBuilder extends DefaultOverflowBuilder { if ( $this->isAllowed( IMinervaPagePermissions::SWITCH_LANGUAGE ) ) { $group->prependEntry( new LanguageSelectorEntry( $this->getTitle(), - $this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->getTitle() ), + $this->languagesHelper->doesTitleHasLanguagesOrVariants( + $this->context->getOutput(), + $this->getTitle() + ), $this->getMessageLocalizer(), false, // no additional classes diff --git a/includes/Permissions/MinervaPagePermissions.php b/includes/Permissions/MinervaPagePermissions.php index 7dcd1fb2b..f1ea44a49 100644 --- a/includes/Permissions/MinervaPagePermissions.php +++ b/includes/Permissions/MinervaPagePermissions.php @@ -27,6 +27,7 @@ use MediaWiki\Content\IContentHandlerFactory; use MediaWiki\MainConfigNames; use MediaWiki\Minerva\LanguagesHelper; use MediaWiki\Minerva\SkinOptions; +use MediaWiki\Output\OutputPage; use MediaWiki\Permissions\Authority; use MediaWiki\Permissions\PermissionManager; use MediaWiki\Title\Title; @@ -51,6 +52,8 @@ final class MinervaPagePermissions implements IMinervaPagePermissions { */ private $performer; + private OutputPage $out; + /** * @var ContentHandler */ @@ -113,6 +116,7 @@ final class MinervaPagePermissions implements IMinervaPagePermissions { $this->title = $context->getTitle(); $this->config = $context->getConfig(); $this->performer = $context->getAuthority(); + $this->out = $context->getOutput(); // Title may be undefined in certain contexts (T179833) // TODO: Check if this is still true if we always pass a context instead of using global one if ( $this->title ) { @@ -196,7 +200,7 @@ final class MinervaPagePermissions implements IMinervaPagePermissions { if ( $this->config->get( MainConfigNames::HideInterlanguageLinks ) ) { return false; } - return $this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->title ) || + return $this->languagesHelper->doesTitleHasLanguagesOrVariants( $this->out, $this->title ) || $this->config->get( 'MinervaAlwaysShowLanguageButton' ); } diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index a2bd04526..c671e9d1e 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -156,8 +156,7 @@ return [ }, 'Minerva.LanguagesHelper' => static function ( MediaWikiServices $services ): LanguagesHelper { return new LanguagesHelper( - $services->getLanguageConverterFactory(), - RequestContext::getMain()->getOutput() + $services->getLanguageConverterFactory() ); }, 'Minerva.SkinOptions' => static function ( MediaWikiServices $services ): SkinOptions { diff --git a/includes/Skins/SkinMinerva.php b/includes/Skins/SkinMinerva.php index 36ab3b49e..ff3d110e0 100644 --- a/includes/Skins/SkinMinerva.php +++ b/includes/Skins/SkinMinerva.php @@ -979,7 +979,10 @@ class SkinMinerva extends SkinMustache { } } - if ( $languagesHelper->doesTitleHasLanguagesOrVariants( $title ) && $title->isMainPage() ) { + if ( + $languagesHelper->doesTitleHasLanguagesOrVariants( $this->getOutput(), $title ) && + $title->isMainPage() + ) { $buttons['language'] = $this->getLanguageButton(); } diff --git a/tests/phpunit/LanguagesHelperTest.php b/tests/phpunit/LanguagesHelperTest.php index 093437625..a33ce9597 100644 --- a/tests/phpunit/LanguagesHelperTest.php +++ b/tests/phpunit/LanguagesHelperTest.php @@ -67,11 +67,13 @@ class LanguagesHelperTest extends MediaWikiIntegrationTestCase { */ public function testDoesTitleHasLanguagesOrVariants( bool $hasVariants, array $langLinks, bool $expected ) { $helper = new LanguagesHelper( - $this->getLanguageConverterFactory( $hasVariants ), - $this->getOutput( $langLinks ) + $this->getLanguageConverterFactory( $hasVariants ) ); - $this->assertSame( $expected, $helper->doesTitleHasLanguagesOrVariants( $this->getTitle() ) ); + $this->assertSame( $expected, $helper->doesTitleHasLanguagesOrVariants( + $this->getOutput( $langLinks ), + $this->getTitle() + ) ); } public static function provideDoesTitleHasLanguagesOrVariants() {