From 6022b032ea7fce65869e27ffecefc948371616e6 Mon Sep 17 00:00:00 2001 From: jdlrobson Date: Mon, 18 May 2020 11:55:52 -0700 Subject: [PATCH] Avoid using `get` indirection in VectorTemplate Begin our journey away from BaseTemplate by moving VectorTemplate code to SkinVector. In future all methods will live here but to lower risk, I've only targetted the get method. Bug: T251212 Change-Id: I58c2ff5edaacc2d5e45492c121cf0f87d08b623f --- includes/SkinVector.php | 30 ++++++++++++++++++ includes/VectorTemplate.php | 31 +++++-------------- .../integration/VectorTemplateTest.php | 27 ++++++++++------ 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/includes/SkinVector.php b/includes/SkinVector.php index d9eb3cf5..5ea4d791 100644 --- a/includes/SkinVector.php +++ b/includes/SkinVector.php @@ -130,4 +130,34 @@ class SkinVector extends SkinTemplate { return !$isLatestSkinFeatureEnabled; } + + /** + * @internal only for use inside VectorTemplate + * @return array of data for a Mustache template + */ + public function getTemplateData() { + $out = $this->getOutput(); + $title = $out->getTitle(); + + return [ + 'html-sitenotice' => $this->getSiteNotice(), + 'html-userlangattributes' => $this->prepareUserLanguageAttributes(), + 'html-subtitle' => $this->prepareSubtitle(), + // Always returns string, cast to null if empty. + 'html-undelete' => $this->prepareUndeleteLink() ?: null, + // Result of OutputPage::addHTML calls + 'html-bodycontent' => $this->wrapHTML( $title, $out->mBodytext ), + 'html-dataAfterContent' => $this->afterContentHook(), + // From MWDebug::getHTMLDebugLog (when $wgShowDebug is enabled) + 'html-debuglog' => $this->generateDebugHTML(), + ]; + } + + /** + * @internal only for use inside VectorTemplate + * @return array + */ + public function getMenuProps() { + return $this->buildContentNavigationUrls(); + } } diff --git a/includes/VectorTemplate.php b/includes/VectorTemplate.php index 88693b6d..b067dd3d 100644 --- a/includes/VectorTemplate.php +++ b/includes/VectorTemplate.php @@ -111,11 +111,13 @@ class VectorTemplate extends BaseTemplate { } /** + * @deprecated Please use Skin::getTemplateData instead * @return array Returns an array of data shared between Vector and legacy * Vector. */ private function getSkinData() : array { - $contentNavigation = $this->get( 'content_navigation', [] ); + // @phan-suppress-next-line PhanUndeclaredMethod + $contentNavigation = $this->getSkin()->getMenuProps(); $skin = $this->getSkin(); $out = $skin->getOutput(); $title = $out->getTitle(); @@ -142,9 +144,9 @@ class VectorTemplate extends BaseTemplate { // // Conditionally used values must use null to indicate absence (not false or ''). $mainPageHref = Skin::makeMainPageUrl(); - $commonSkinData = [ + // @phan-suppress-next-line PhanUndeclaredMethod + $commonSkinData = $skin->getTemplateData() + [ 'html-headelement' => $out->headElement( $skin ), - 'html-sitenotice' => $this->get( 'sitenotice', null ), 'html-indicators' => $this->getIndicators(), 'page-langcode' => $title->getPageViewLanguage()->getHtmlCode(), 'page-isarticle' => (bool)$out->isArticle(), @@ -152,17 +154,7 @@ class VectorTemplate extends BaseTemplate { // Remember that the string '0' is a valid title. // From OutputPage::getPageTitle, via ::setPageTitle(). 'html-title' => $out->getPageTitle(), - - 'html-prebodyhtml' => $this->get( 'prebodyhtml', '' ), 'msg-tagline' => $this->msg( 'tagline' )->text(), - // TODO: Use Skin::prepareUserLanguageAttributes() when available. - 'html-userlangattributes' => $this->get( 'userlangattributes', '' ), - // From OutputPage::getSubtitle() - 'html-subtitle' => $this->get( 'subtitle', '' ), - - // TODO: Use Skin::prepareUndeleteLink() when available - // Always returns string, cast to null if empty. - 'html-undelete' => $this->get( 'undelete', null ) ?: null, // From Skin::getNewtalks(). Always returns string, cast to null if empty. 'html-newtalk' => $skin->getNewtalks() ?: null, @@ -170,18 +162,11 @@ class VectorTemplate extends BaseTemplate { 'msg-vector-jumptonavigation' => $this->msg( 'vector-jumptonavigation' )->text(), 'msg-vector-jumptosearch' => $this->msg( 'vector-jumptosearch' )->text(), - // Result of OutputPage::addHTML calls - 'html-bodycontent' => $this->get( 'bodycontent' ), - 'html-printfooter' => $skin->printSource(), 'html-catlinks' => $skin->getCategories(), - 'html-dataAfterContent' => $this->get( 'dataAfterContent', '' ), - // From MWDebug::getHTMLDebugLog (when $wgShowDebug is enabled) - 'html-debuglog' => $this->get( 'debughtml', '' ), // From BaseTemplate::getTrail (handles bottom JavaScript) 'html-printtail' => $this->getTrail() . '', 'data-footer' => [ - 'html-userlangattributes' => $this->get( 'userlangattributes', '' ), 'html-hook-vector-before-footer' => $htmlHookVectorBeforeFooter, 'array-footer-rows' => $this->getTemplateFooterRows(), ], @@ -279,7 +264,7 @@ class VectorTemplate extends BaseTemplate { */ private function buildSidebar() : array { $skin = $this->getSkin(); - $portals = $this->get( 'sidebar', [] ); + $portals = $skin->buildSidebar(); $props = []; // Force the rendering of the following portals if ( !isset( $portals['TOOLBOX'] ) ) { @@ -418,7 +403,6 @@ class VectorTemplate extends BaseTemplate { 'label-id' => "p-{$label}-label", // If no message exists fallback to plain text (T252727) 'label' => $msgObj->exists() ? $msgObj->text() : $label, - 'html-userlangattributes' => $this->get( 'userlangattributes', '' ), 'list-classes' => $listClasses[$type] ?? 'vector-menu-content-list', 'html-items' => '', 'is-dropdown' => self::MENU_TYPE_DROPDOWN === $type, @@ -459,7 +443,8 @@ class VectorTemplate extends BaseTemplate { * @return array */ private function getMenuProps() : array { - $contentNavigation = $this->get( 'content_navigation', [] ); + // @phan-suppress-next-line PhanUndeclaredMethod + $contentNavigation = $this->getSkin()->getMenuProps(); $personalTools = $this->getPersonalTools(); $skin = $this->getSkin(); diff --git a/tests/phpunit/integration/VectorTemplateTest.php b/tests/phpunit/integration/VectorTemplateTest.php index 9ccbc442..4736a565 100644 --- a/tests/phpunit/integration/VectorTemplateTest.php +++ b/tests/phpunit/integration/VectorTemplateTest.php @@ -3,7 +3,9 @@ namespace MediaWiki\Skins\Vector\Tests\Integration; use GlobalVarConfig; use MediaWikiIntegrationTestCase; +use RequestContext; use TemplateParser; +use Title; use VectorTemplate; use Wikimedia\TestingAccessWrapper; @@ -95,17 +97,25 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase { * @covers ::getMenuProps */ public function testGetMenuProps() { - $langAttrs = 'LANG_ATTRIBUTES'; + $title = Title::newFromText( 'SkinTemplateVector' ); + $context = RequestContext::getMain(); + $context->setTitle( $title ); + $context->setLanguage( 'fr' ); $vectorTemplate = $this->provideVectorTemplateObject(); // used internally by getPersonalTools $vectorTemplate->set( 'personal_urls', [] ); - $vectorTemplate->set( 'content_navigation', [ - 'actions' => [], - 'namespaces' => [], - 'variants' => [], - 'views' => [], + $this->setMwGlobals( 'wgHooks', [ + 'SkinTemplateNavigation' => [ + function ( &$skinTemplate, &$content_navigation ) { + $content_navigation = [ + 'actions' => [], + 'namespaces' => [], + 'variants' => [], + 'views' => [], + ]; + } + ] ] ); - $vectorTemplate->set( 'userlangattributes', $langAttrs ); $openVectorTemplate = TestingAccessWrapper::newFromObject( $vectorTemplate ); $props = $openVectorTemplate->getMenuProps(); @@ -115,8 +125,7 @@ class VectorTemplateTest extends MediaWikiIntegrationTestCase { $this->assertSame( $views, [ 'id' => 'p-views', 'label-id' => 'p-views-label', - 'label' => 'Views', - 'html-userlangattributes' => $langAttrs, + 'label' => $context->msg( 'views' )->text(), 'list-classes' => 'vector-menu-content-list', 'html-items' => '', 'is-dropdown' => false,