jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/403550 )
Change subject: Hygiene: Remove MobileContext::getConfigVariable
......................................................................
Hygiene: Remove MobileContext::getConfigVariable
Going forward we will make use of the FeatureManager for these kinds
of checks.
Given the complexity of the shouldShowWikibaseDescriptions method
I've not (yet) removed it, but have wired it up to the feature
management system.
We should also register features that have been promoted to stable
as if they are ever demoted to beta they will not appear in
Special:MobileOptions
Change-Id: I6aa1c66ec131a8db75d6e6128d4e3af78f351af0
---
M i18n/en.json
M i18n/qqq.json
M includes/MobileContext.php
M includes/MobileFrontend.body.php
M includes/MobileFrontend.hooks.php
M includes/ServiceWiring.php
M tests/phpunit/MobileContextTest.php
M tests/phpunit/context/MobileContextWikibaseDescriptionsTest.php
A tests/phpunit/features/FeaturesManagerTest.php
9 files changed, 118 insertions(+), 152 deletions(-)
Approvals:
Pmiazga: Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index 6f28ee9..ce5af11 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -299,5 +299,13 @@
"mobile-frontend-mobile-option-MFEnableWikidataDescriptions": "Title
descriptions",
"mobile-frontend-mobile-option-MFEnableWikidataDescriptions-description":
"Learn about the subject of the article with a short description below the
title",
"mobile-frontend-mobile-option-MFLazyLoadReferences-description": "Load
article references only when needed",
- "mobile-frontend-send-feedback": "Send feedback"
+ "mobile-frontend-send-feedback": "Send feedback",
+ "mobile-frontend-mobile-option-MFLazyLoadImages-description": "Load
article images only when needed.",
+ "mobile-frontend-mobile-option-MFLazyLoadImages": "Lazy load images",
+ "mobile-frontend-mobile-option-MFShowFirstParagraphBeforeInfobox":
"Lead paragraph",
+
"mobile-frontend-mobile-option-MFShowFirstParagraphBeforeInfobox-description":
"Text will always show above infoboxes, improving performance and readability.",
+ "mobile-frontend-mobile-option-MFExpandAllSectionsUserOption": "Expand
all sections by default",
+
"mobile-frontend-mobile-option-MFExpandAllSectionsUserOption-description":
"Disable section collapsing on mobile web.",
+ "mobile-frontend-mobile-option-MFEnableFontChanger": "Article font
size",
+ "mobile-frontend-mobile-option-MFEnableFontChanger-description":
"Control the font size of your reading experience."
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 7042381..47addc6 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -300,5 +300,13 @@
"mobile-frontend-mobile-option-MFEnableWikidataDescriptions": "An
identifier for the wikidata descriptions subproject/feature",
"mobile-frontend-mobile-option-MFEnableWikidataDescriptions-description": "A
description of the wikidata description subproject/feature",
"mobile-frontend-mobile-option-MFLazyLoadReferences-description": "A
description of the lazy load references subproject/feature",
- "mobile-frontend-send-feedback": "Link label for sending feedback to
the mobile web beta"
+ "mobile-frontend-send-feedback": "Link label for sending feedback to
the mobile web beta",
+ "mobile-frontend-mobile-option-MFLazyLoadImages-description": "Lazy
load images feature description",
+ "mobile-frontend-mobile-option-MFLazyLoadImages": "Lazy load images
feature name",
+ "mobile-frontend-mobile-option-MFShowFirstParagraphBeforeInfobox":
"Lead paragraph feature name",
+
"mobile-frontend-mobile-option-MFShowFirstParagraphBeforeInfobox-description":
"Lead paragraph feature description",
+ "mobile-frontend-mobile-option-MFExpandAllSectionsUserOption": "Expand
all sections feature name",
+
"mobile-frontend-mobile-option-MFExpandAllSectionsUserOption-description":
"Expand sections feature description",
+ "mobile-frontend-mobile-option-MFEnableFontChanger": "Article font size
feature name",
+ "mobile-frontend-mobile-option-MFEnableFontChanger-description": "Font
size feature description"
}
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 1da8860..84a6b30 100644
--- a/includes/MobileContext.php
+++ b/includes/MobileContext.php
@@ -141,90 +141,6 @@
}
/**
- * Gets the value of a config variable whose value depends on whether
the
- * user is a member of the beta group.
- *
- * @warning If the value of the config variable doesn't behave this
way, then
- * `null` is returned.
- *
- * @example
- * ```
- * $wgFoo = [
- * 'beta' => 'bar',
- * 'base' => 'baz',
- * ];
- * $wgQux = 'quux';
- * $wgCorge = [
- * 'grault' => 'garply',
- * ];
- *
- * $context = MobileContext::singleton();
- * $context->getConfigVariable( 'Foo' ); // => 'baz'
- *
- * $context->setMobileMode( 'beta' );
- * $context->getConfigVariable( 'Foo' ); // => 'bar'
- *
- * // If the config variable isn't a dictionary, then its value will be
- * // returned returned regardless of whether the user is a member of
the beta
- * // group.
- * $context->getConfigVariable( 'Qux' ); // => 'quux'
- *
- * // If the config variable is a dictionary but doesn't have "beta" or
"base"
- * // entries, then `null` will be returned.
- * $context->getConfigVariable( 'Corge' ); // => null
- * ```
- *
- * @param string $variableName Config variable to be returned
- * @return mixed|null
- * @throws ConfigException If the config variable doesn't exist
- *
- * @TODO Should this be renamed, e.g. `getFlag`, or extracted?
- */
- public function getConfigVariable( $variableName ) {
- $configVariable = $this->getMFConfig()->get( $variableName ) ?:
[];
-
- if ( !is_array( $configVariable ) ) {
- return $configVariable;
- }
-
- if ( $this->isBetaGroupMember() && array_key_exists( 'beta',
$configVariable ) ) {
- return $configVariable['beta'];
- } elseif ( array_key_exists( 'base', $configVariable ) ) {
- return $configVariable['base'];
- }
- return null;
- }
-
- /**
- * Checks whether references should be lazy loaded for the current user
- * @return bool
- */
- public function isLazyLoadReferencesEnabled() {
- return $this->getConfigVariable( 'MFLazyLoadReferences' );
- }
-
- /**
- * Checks whether images should be lazy loaded for the current user
- * @return bool
- */
- public function isLazyLoadImagesEnabled() {
- return $this->getConfigVariable( 'MFLazyLoadImages' );
- }
-
- /**
- * Checks whether the first paragraph from the lead section should be
- * shown before all infoboxes that come earlier.
- * @return bool
- */
- public function shouldShowFirstParagraphBeforeInfobox() {
- if ( $this->showFirstParagraphBeforeInfobox === null ) {
- $this->showFirstParagraphBeforeInfobox =
$this->getConfigVariable(
- 'MFShowFirstParagraphBeforeInfobox' );
- }
- return $this->showFirstParagraphBeforeInfobox;
- }
-
- /**
* Detects whether the UA is sending the request from a device and, if
so,
* whether to display the mobile view to that device.
*
@@ -1150,6 +1066,8 @@
/**
* Gets whether Wikibase descriptions should be shown in search
results, including nearby search,
* and watchlists; or as taglines on article pages.
+ * Doesn't take into account whether the wikidata descriptions
+ * feature has been enabled.
*
* @param string $feature which description to show?
* @return bool
@@ -1159,20 +1077,12 @@
public function shouldShowWikibaseDescriptions( $feature ) {
$config = $this->getMFConfig();
$displayWikibaseDescriptions = $config->get(
'MFDisplayWikibaseDescriptions' );
-
if ( !isset( $displayWikibaseDescriptions[ $feature ] ) ) {
throw new DomainException(
"\"{$feature}\" isn't a feature that shows
Wikidata descriptions."
);
}
- if (
- $this->getConfigVariable(
'MFEnableWikidataDescriptions' ) ||
- ( $config->get( 'MFUseWikibase' ) &&
$displayWikibaseDescriptions[ $feature ] )
- ) {
- return true;
- } else {
- return false;
- }
+ return $displayWikibaseDescriptions[ $feature ];
}
}
diff --git a/includes/MobileFrontend.body.php b/includes/MobileFrontend.body.php
index 45f4196..fadda3d 100644
--- a/includes/MobileFrontend.body.php
+++ b/includes/MobileFrontend.body.php
@@ -30,6 +30,8 @@
* @return string
*/
public static function domParse( OutputPage $out, $text = null ) {
+ $featureManager = \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( 'MobileFrontend.FeaturesManager' );
$context = MobileContext::singleton();
$config = $context->getMFConfig();
$factory = new ContentProviderFactory();
@@ -61,10 +63,11 @@
Hooks::run( 'MobileFrontendBeforeDOM', [ $context, $formatter ]
);
- $removeImages = $context->isLazyLoadImagesEnabled();
- $removeReferences = $context->isLazyLoadReferencesEnabled();
- $showFirstParagraphBeforeInfobox =
$context->shouldShowFirstParagraphBeforeInfobox()
- && $ns === NS_MAIN;
+ $removeImages = $featureManager->isFeatureAvailableInContext(
'MFLazyLoadImages', $context );
+ $removeReferences =
+ $featureManager->isFeatureAvailableInContext(
'MFLazyLoadReferences', $context );
+ $showFirstParagraphBeforeInfobox = $ns === NS_MAIN &&
+ $featureManager->isFeatureAvailableInContext(
'MFShowFirstParagraphBeforeInfobox', $context );
if ( $context->getContentTransformations() ) {
// Remove images if they're disabled from special
pages, but don't transform otherwise
diff --git a/includes/MobileFrontend.hooks.php
b/includes/MobileFrontend.hooks.php
index fdc08c4..7fa8846 100644
--- a/includes/MobileFrontend.hooks.php
+++ b/includes/MobileFrontend.hooks.php
@@ -184,10 +184,14 @@
*/
public static function onSkinAfterBottomScripts( $sk, &$html ) {
$context = MobileContext::singleton();
+ $featureManager = \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( 'MobileFrontend.FeaturesManager' );
// TODO: We may want to enable the following script on Desktop
Minerva...
// ... when Minerva is widely used.
- if ( $context->shouldDisplayMobileView() &&
$context->isLazyLoadImagesEnabled() ) {
+ if ( $context->shouldDisplayMobileView() &&
+ $featureManager->isFeatureAvailableInContext(
'MFLazyLoadImages', $context )
+ ) {
$html .= Html::inlineScript( ResourceLoader::filter(
'minify-js',
MobileFrontendSkinHooks::gradeCImageSupport()
) );
@@ -475,9 +479,16 @@
$config = $context->getMFConfig();
$features = array_keys( $config->get(
'MFDisplayWikibaseDescriptions' ) );
$result = [ 'wgMFDisplayWikibaseDescriptions' => [] ];
+ $featureManager = \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( 'MobileFrontend.FeaturesManager' );
+
+ $descriptionsEnabled =
$featureManager->isFeatureAvailableInContext(
+ 'MFEnableWikidataDescriptions',
+ $context
+ );
foreach ( $features as $feature ) {
- $result['wgMFDisplayWikibaseDescriptions'][$feature] =
+ $result['wgMFDisplayWikibaseDescriptions'][$feature] =
$descriptionsEnabled &&
$context->shouldShowWikibaseDescriptions(
$feature );
}
@@ -1217,22 +1228,29 @@
* @return bool true in all cases
*/
public static function onMakeGlobalVariablesScript( array &$vars,
OutputPage $out ) {
+ $featureManager = \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( 'MobileFrontend.FeaturesManager' );
+
// If the device is a mobile, Remove the category entry.
$context = MobileContext::singleton();
if ( $context->shouldDisplayMobileView() ) {
unset( $vars['wgCategories'] );
$vars['wgMFMode'] = $context->isBetaGroupMember() ?
'beta' : 'stable';
- $vars['wgMFLazyLoadImages'] =
$context->isLazyLoadImagesEnabled();
- $vars['wgMFLazyLoadReferences'] =
$context->isLazyLoadReferencesEnabled();
+ $vars['wgMFLazyLoadImages'] =
+ $featureManager->isFeatureAvailableInContext(
'MFLazyLoadImages', $context );
+ $vars['wgMFLazyLoadReferences'] =
+ $featureManager->isFeatureAvailableInContext(
'MFLazyLoadReferences', $context );
}
$title = $out->getTitle();
$vars['wgPreferredVariant'] =
$title->getPageLanguage()->getPreferredVariant();
// Accesses getBetaGroupMember so does not belong in
onResourceLoaderGetConfigVars
$vars['wgMFExpandAllSectionsUserOption'] =
- $context->getConfigVariable(
'MFExpandAllSectionsUserOption' );
+ $featureManager->isFeatureAvailableInContext(
'MFExpandAllSectionsUserOption', $context );
- $vars['wgMFEnableFontChanger'] = $context->getConfigVariable(
'MFEnableFontChanger' );
+ $vars['wgMFEnableFontChanger'] =
+ $featureManager->isFeatureAvailableInContext(
'MFEnableFontChanger', $context );
+
$vars += self::getWikibaseStaticConfigVars( $context );
return true;
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index 700f804..6782744 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -22,6 +22,14 @@
$config->get( 'MFEnableWikidataDescriptions' ) ) );
$manager->registerFeature( new Feature( 'MFLazyLoadReferences',
'mobile-frontend',
$config->get( 'MFLazyLoadReferences' ) ) );
+ $manager->registerFeature( new Feature( 'MFLazyLoadImages',
'mobile-frontend',
+ $config->get( 'MFLazyLoadImages' ) ) );
+ $manager->registerFeature( new Feature(
'MFShowFirstParagraphBeforeInfobox', 'mobile-frontend',
+ $config->get( 'MFShowFirstParagraphBeforeInfobox' ) ) );
+ $manager->registerFeature( new Feature(
'MFExpandAllSectionsUserOption', 'mobile-frontend',
+ $config->get( 'MFExpandAllSectionsUserOption' ) ) );
+ $manager->registerFeature( new Feature( 'MFEnableFontChanger',
'mobile-frontend',
+ $config->get( 'MFEnableFontChanger' ) ) );
return $manager;
},
diff --git a/tests/phpunit/MobileContextTest.php
b/tests/phpunit/MobileContextTest.php
index 9bbc422..e45f143 100644
--- a/tests/phpunit/MobileContextTest.php
+++ b/tests/phpunit/MobileContextTest.php
@@ -570,47 +570,6 @@
}
/**
- * @dataProvider provideGetConfigVariable
- * @covers MobileContext::getConfigVariable
- */
- public function testGetConfigVariable(
- $expected,
- $madeUpConfigVariable,
- $mobileMode = MobileContext::MODE_STABLE
- ) {
- $this->setMwGlobals( [
- 'wgMFEnableBeta' => true,
- 'wgMFMadeUpConfigVariable' => $madeUpConfigVariable
- ] );
-
- $context = MobileContext::singleton();
- $context->setMobileMode( $mobileMode );
-
- $this->assertEquals(
- $expected,
- $context->getConfigVariable( 'MFMadeUpConfigVariable' )
- );
- }
-
- public static function provideGetConfigVariable() {
- $madeUpConfigVariable = [
- 'beta' => 'bar',
- 'base' => 'foo',
- ];
-
- return [
- [ 'foo', $madeUpConfigVariable,
MobileContext::MODE_STABLE ],
- [ 'bar', $madeUpConfigVariable,
MobileContext::MODE_BETA ],
-
- [ null, [ 'alpha' => 'baz' ] ],
-
- // When the config variable isn't an array, then its
value is returned
- // regardless of whether the user is a member of the
beta group.
- [ true, true ],
- ];
- }
-
- /**
* @dataProvider provideShouldStripResponsiveImages
* @covers MobileContext::shouldStripResponsiveImages
*/
diff --git a/tests/phpunit/context/MobileContextWikibaseDescriptionsTest.php
b/tests/phpunit/context/MobileContextWikibaseDescriptionsTest.php
index 0f53f1d..e6d514c 100644
--- a/tests/phpunit/context/MobileContextWikibaseDescriptionsTest.php
+++ b/tests/phpunit/context/MobileContextWikibaseDescriptionsTest.php
@@ -15,7 +15,6 @@
// Set relevant configuration variables to their default values.
$this->setMwGlobals( [
- 'wgMFUseWikibase' => false,
'wgMFDisplayWikibaseDescriptions' => [
'search' => true,
'tagline' => false,
@@ -29,17 +28,13 @@
* @covers MobileContext::shouldShowWikibaseDescriptions
*/
public function testShowingDescriptionsIsDisabledByDefault() {
- $this->assertFalse(
$this->context->shouldShowWikibaseDescriptions( 'search' ) );
+ $this->assertTrue(
$this->context->shouldShowWikibaseDescriptions( 'search' ) );
}
/**
* @covers MobileContext::shouldShowWikibaseDescriptions
*/
public function testShowingDescriptionsCanBeEnabled() {
- $this->setMwGlobals( [
- 'wgMFUseWikibase' => true,
- ] );
-
$this->assertTrue(
$this->context->shouldShowWikibaseDescriptions(
'search' ),
'Showing descriptions is flagged by new variables.'
diff --git a/tests/phpunit/features/FeaturesManagerTest.php
b/tests/phpunit/features/FeaturesManagerTest.php
new file mode 100644
index 0000000..b21c55e
--- /dev/null
+++ b/tests/phpunit/features/FeaturesManagerTest.php
@@ -0,0 +1,57 @@
+<?php
+
+use MobileFrontend\Features\FeaturesManager;
+use MobileFrontend\Features\Feature;
+
+/**
+ * @group MobileFrontend
+ * @coversDefaultClass MobileFrontend\Features\FeaturesManager
+ */
+class FeaturesManagerTest extends MediaWikiTestCase {
+
+ /**
+ * @dataProvider provideIsFeatureAvailableInContext
+ * @covers ::isFeatureAvailableInContext
+ *
+ * @param string $html
+ * @param string $expected
+ * @param string $isBetaGroupMember isBetaGroup memeber
+ */
+ public function testIsFeatureAvailableInContext(
+ $expected,
+ $madeUpConfigVariable,
+ $isBetaGroupMember
+ ) {
+ $manager = new FeaturesManager();
+ $manager->registerFeature(
+ new Feature( 'MFMadeUpConfigVariable', 'test-group',
$madeUpConfigVariable ) );
+
+ $contextMock = $this->getMockBuilder( MobileContext::class )
+ ->disableOriginalConstructor()
+ ->setMethods( [ 'isBetaGroupMember' ] )
+ ->getMock();
+
+ $contextMock->expects( $this->once() )
+ ->method( 'isBetaGroupMember' )
+ ->willReturn( $isBetaGroupMember );
+
+ $this->assertEquals(
+ $expected,
+ $manager->isFeatureAvailableInContext(
'MFMadeUpConfigVariable', $contextMock )
+ );
+ }
+
+ public function provideIsFeatureAvailableInContext() {
+ $madeUpConfigVariable = [
+ 'beta' => true,
+ 'base' => false,
+ ];
+
+ return [
+ [ false, $madeUpConfigVariable, false ],
+ [ true, $madeUpConfigVariable, true ],
+
+ [ false, [ 'alpha' => 'baz' ], true ],
+ ];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/403550
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6aa1c66ec131a8db75d6e6128d4e3af78f351af0
Gerrit-PatchSet: 10
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: specialpages
Gerrit-Owner: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Jdlrobson <jrob...@wikimedia.org>
Gerrit-Reviewer: Pmiazga <pmia...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
