Jdlrobson has uploaded a new change for review. (
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.
Change-Id: I6aa1c66ec131a8db75d6e6128d4e3af78f351af0
---
M includes/MobileContext.php
M includes/MobileFrontend.body.php
M includes/MobileFrontend.hooks.php
M includes/ServiceWiring.php
M tests/phpunit/MobileContextTest.php
A tests/phpunit/features/FeaturesManagerTest.php
6 files changed, 86 insertions(+), 135 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend
refs/changes/50/403550/1
diff --git a/includes/MobileContext.php b/includes/MobileContext.php
index 1da8860..49d7cb7 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.
*
@@ -1157,6 +1073,11 @@
* `wgMFDisplayWikibaseDescriptions` configuration variable for detail
*/
public function shouldShowWikibaseDescriptions( $feature ) {
+ $featureManager = \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( 'MobileFrontend.FeaturesManager' );
+ $isEnabled = $featureManager->isAvailableInContext(
+ 'MFEnableWikidataDescriptions', $context
+ );
$config = $this->getMFConfig();
$displayWikibaseDescriptions = $config->get(
'MFDisplayWikibaseDescriptions' );
@@ -1166,7 +1087,9 @@
);
}
- if (
+ if ( !$isEnabled ) {
+ return false;
+ } elseif (
$this->getConfigVariable(
'MFEnableWikidataDescriptions' ) ||
( $config->get( 'MFUseWikibase' ) &&
$displayWikibaseDescriptions[ $feature ] )
) {
diff --git a/includes/MobileFrontend.body.php b/includes/MobileFrontend.body.php
index 45f4196..fa5da35 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,10 @@
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..5b14067 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()
) );
@@ -1217,22 +1221,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..ad87737 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -22,6 +22,10 @@
$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' ) ) );
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/features/FeaturesManagerTest.php
b/tests/phpunit/features/FeaturesManagerTest.php
new file mode 100644
index 0000000..9dfadc4
--- /dev/null
+++ b/tests/phpunit/features/FeaturesManagerTest.php
@@ -0,0 +1,52 @@
+<?php
+
+use MobileFrontend\Features;
+
+/**
+ * @group MobileFrontend
+ */
+class FeaturesManagerTest extends MediaWikiTestCase {
+ /**
+ * @dataProvider provideTransform
+ * @covers FeaturesManager
+ *
+ * @param string $html
+ * @param string $expected
+ */
+ public function testIsFeatureAvailableInContext(
+ $expected,
+ $madeUpConfigVariable,
+ $mobileMode = MobileContext::MODE_STABLE
+ ) {
+ $manager = new FeaturesManager();
+ $manager->registerFeature(
+ new Feature( 'MFMadeUpConfigVariable', 'test-group',
$madeUpConfigVariable ) );
+
+
+ $context = MobileContext::singleton();
+ $context->setMobileMode( $mobileMode );
+
+ $this->assertEquals(
+ $expected,
+ $manager->isFeatureAvailableInContext(
'MFMadeUpConfigVariable', $context )
+ );
+ }
+
+ public function provideIsFeatureAvailableInContext() {
+ $madeUpConfigVariable = [
+ 'beta' => 'bar',
+ 'base' => 'foo',
+ ];
+
+ return [
+ [ 'foo', $madeUpConfigVariable,
MobileContext::MODE_STABLE ],
+ [ 'bar', $madeUpConfigVariable,
MobileContext::MODE_BETA ],
+
+ [ false, [ 'alpha' => 'baz' ] ],
+
+ // When the config variable isn't an array, unlike the
predecessor MobileContext::getConfigVariable
+ // then it will be ignored as a feature and assumed to
be disabled.
+ [ false, true ],
+ ];
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/403550
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6aa1c66ec131a8db75d6e6128d4e3af78f351af0
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/MobileFrontend
Gerrit-Branch: specialpages
Gerrit-Owner: Jdlrobson <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
