Pmiazga has uploaded a new change for review. https://gerrit.wikimedia.org/r/322230
Change subject: Hygiene: use constants ...................................................................... Hygiene: use constants For better maintenance use contants instead of strings Changes: - introduced constants for all cookies - introduced constants for stable/mode - unit tests now will also use constants Change-Id: I67743bb5b9512578d0a4efbdd9af9d1c0f586339 --- M includes/MobileContext.php M includes/MobileFrontend.hooks.php M tests/phpunit/MobileContextTest.php M tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php 4 files changed, 36 insertions(+), 27 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/MobileFrontend refs/changes/30/322230/1 diff --git a/includes/MobileContext.php b/includes/MobileContext.php index c9233e3..58a4f5e 100644 --- a/includes/MobileContext.php +++ b/includes/MobileContext.php @@ -11,7 +11,11 @@ * Provide various request-dependant methods to use in mobile context */ class MobileContext extends ContextSource { - + const MODE_BETA = 'beta'; + const MODE_STABLE = 'stable'; + const DISABLE_IMAGES_COOKIE_NAME = 'disableImages'; + const OPTIN_COOKIE_NAME = 'optin'; + const STOP_MOBILE_REDIRECT_COKIE_NAME = 'stopMobileRedirect'; const USEFORMAT_COOKIE_NAME = 'mf_useformat'; const USER_MODE_PREFERENCE_NAME = 'mfMode'; const LAZY_LOAD_IMAGES_COOKIE_NAME = 'mfLazyLoadImages'; @@ -261,8 +265,9 @@ public function imagesDisabled() { if ( is_null( $this->disableImages ) ) { $this->disableImages = ( - ( isset( $_COOKIE['disableImages'] ) && $_COOKIE['disableImages'] === '1' ) || - (bool) $this->getRequest()->getCookie( 'disableImages' ) + ( isset( $_COOKIE[ self::DISABLE_IMAGES_COOKIE_NAME ] ) + && $_COOKIE[ self::DISABLE_IMAGES_COOKIE_NAME ] === '1' ) || + (bool) $this->getRequest()->getCookie( self::DISABLE_IMAGES_COOKIE_NAME ) ); } @@ -343,7 +348,7 @@ * If the cookie is not set the value will be an empty string. */ private function loadMobileModeCookie() { - $this->mobileMode = $this->getRequest()->getCookie( 'optin', '' ); + $this->mobileMode = $this->getRequest()->getCookie( self::OPTIN_COOKIE_NAME, '' ); } /** @@ -358,7 +363,7 @@ } if ( is_null( $this->mobileMode ) ) { $mobileAction = $this->getMobileAction(); - if ( $mobileAction === 'beta' || $mobileAction === 'stable' ) { + if ( $mobileAction === self::MODE_BETA || $mobileAction === self::MODE_STABLE ) { $this->mobileMode = $mobileAction; } else { $user = $this->getUser(); @@ -384,11 +389,11 @@ * @param string $mode Mode to set */ public function setMobileMode( $mode ) { - if ( $mode !== 'beta' ) { + if ( $mode !== self::MODE_BETA ) { $mode = ''; } // Update statistics - if ( $mode === 'beta' ) { + if ( $mode === self::MODE_BETA ) { wfIncrStats( 'mobile.opt_in_cookie_set' ); } if ( !$mode ) { @@ -399,7 +404,7 @@ $user->setOption( self::USER_MODE_PREFERENCE_NAME, $mode ); $user->saveSettings(); - $this->getRequest()->response()->setCookie( 'optin', $mode, 0, [ + $this->getRequest()->response()->setCookie( self::OPTIN_COOKIE_NAME, $mode, 0, [ 'prefix' => '', 'domain' => $this->getBaseDomain() ] ); @@ -410,7 +415,7 @@ * @return boolean */ public function isBetaGroupMember() { - return $this->getMobileMode() === 'beta'; + return $this->getMobileMode() === self::MODE_BETA; } /** @@ -605,7 +610,8 @@ $expiry = $this->getUseFormatCookieExpiry(); } - $this->getRequest()->response()->setcookie( 'stopMobileRedirect', 'true', $expiry, + $this->getRequest()->response()->setcookie( + self::STOP_MOBILE_REDIRECT_COKIE_NAME, 'true', $expiry, [ 'domain' => $this->getStopMobileRedirectCookieDomain(), 'prefix' => '', @@ -631,7 +637,8 @@ * @return string */ public function getStopMobileRedirectCookie() { - $stopMobileRedirectCookie = $this->getRequest()->getCookie( 'stopMobileRedirect', '' ); + $stopMobileRedirectCookie = $this->getRequest() + ->getCookie( self::STOP_MOBILE_REDIRECT_COKIE_NAME, '' ); return $stopMobileRedirectCookie; } @@ -657,9 +664,9 @@ public function setDisableImagesCookie( $shouldDisableImages ) { $resp = $this->getRequest()->response(); if ( $shouldDisableImages ) { - $resp->setCookie( 'disableImages', 1, 0, [ 'prefix' => '' ] ); + $resp->setCookie( self::DISABLE_IMAGES_COOKIE_NAME, 1, 0, [ 'prefix' => '' ] ); } else { - $resp->clearCookie( 'disableImages', [ 'prefix' => '' ] ); + $resp->clearCookie( self::DISABLE_IMAGES_COOKIE_NAME, [ 'prefix' => '' ] ); } } diff --git a/includes/MobileFrontend.hooks.php b/includes/MobileFrontend.hooks.php index 1d752a4..7a9fc53 100644 --- a/includes/MobileFrontend.hooks.php +++ b/includes/MobileFrontend.hooks.php @@ -360,10 +360,10 @@ // Enables mobile cookies on wikis w/o mobile domain $cookies[] = MobileContext::USEFORMAT_COOKIE_NAME; // Don't redirect to mobile if user had explicitly opted out of it - $cookies[] = 'stopMobileRedirect'; + $cookies[] = MobileContext::STOP_MOBILE_REDIRECT_COKIE_NAME; if ( $context->shouldDisplayMobileView() || !$mobileUrlTemplate ) { - $cookies[] = 'optin'; // beta cookie + $cookies[] = MobileContext::OPTIN_COOKIE_NAME; // beta cookie } // Redirect people who want so from HTTP to HTTPS. Ideally, should be // only for HTTP but we don't vary on protocol. @@ -746,11 +746,12 @@ $request = $context->getRequest(); // Migrate prefixed disableImages cookie to unprefixed cookie. - if ( isset( $_COOKIE[$config->get( 'CookiePrefix' ) . 'disableImages'] ) ) { - if ( (bool)$request->getCookie( 'disableImages' ) ) { + $rawCookie = $config->get( 'CookiePrefix' ) . MobileContext::DISABLE_IMAGES_COOKIE_NAME; + if ( isset( $_COOKIE[ $rawCookie ] ) ) { + if ( (bool)$request->getCookie( MobileContext::DISABLE_IMAGES_COOKIE_NAME ) ) { $context->setDisableImagesCookie( true ); } - $request->response()->clearCookie( 'disableImages' ); + $request->response()->clearCookie( MobileContext::DISABLE_IMAGES_COOKIE_NAME ); } # Add deep link to a mobile app specified by $wgMFAppScheme diff --git a/tests/phpunit/MobileContextTest.php b/tests/phpunit/MobileContextTest.php index 5208208..dfc9224 100644 --- a/tests/phpunit/MobileContextTest.php +++ b/tests/phpunit/MobileContextTest.php @@ -470,11 +470,11 @@ public function optInProvider() { return [ [ [], false, true ], - [ [ 'optin' => 'beta' ], true, true ], - [ [ 'optin' => 'foobar' ], false, true ], + [ [ MobileContext::OPTIN_COOKIE_NAME => MobileContext::MODE_BETA ], true, true ], + [ [ MobileContext::OPTIN_COOKIE_NAME => 'foobar' ], false, true ], [ [], false, false ], - [ [ 'optin' => 'beta' ], false, false ], - [ [ 'optin' => 'foobar' ], false, false ], + [ [ MobileContext::OPTIN_COOKIE_NAME => MobileContext::MODE_BETA ], false, false ], + [ [ MobileContext::OPTIN_COOKIE_NAME => 'foobar' ], false, false ], ]; } @@ -571,7 +571,7 @@ public function testGetConfigVariable( $expected, $wgMinervaUseFooterV2, - $mobileMode = 'stable' + $mobileMode = MobileContext::MODE_STABLE ) { $this->setMwGlobals( [ 'wgMFEnableBeta' => true, @@ -594,8 +594,8 @@ ]; return [ - [ 'foo', $wgMinervaUseFooterV2, 'stable' ], - [ 'bar', $wgMinervaUseFooterV2, 'beta' ], + [ 'foo', $wgMinervaUseFooterV2, MobileContext::MODE_STABLE ], + [ 'bar', $wgMinervaUseFooterV2, MobileContext::MODE_BETA ], [ null, [ 'alpha' => 'baz' ] ], diff --git a/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php b/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php index 526b916..0e9f690 100644 --- a/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php +++ b/tests/phpunit/context/MobileContextShouldDisplayMobileViewIntegrationTest.php @@ -69,11 +69,12 @@ // N.B. that the format and the "stop mobile redirect" cookies // ("mf_useformat" and "stopMobileRedirect" respectively) aren't prefix // with MediaWiki's cookie prefix ($wgCookiePrefix). - $request->setCookie( 'mf_useformat', $formatCookie, '' ); + $request->setCookie( MobileContext::USEFORMAT_COOKIE_NAME, $formatCookie, '' ); } if ( $stopMobileRedirectCookie !== null ) { - $request->setCookie( 'stopMobileRedirect', $stopMobileRedirectCookie, '' ); + $request->setCookie( + MobileContext::STOP_MOBILE_REDIRECT_COKIE_NAME, $stopMobileRedirectCookie, '' ); } if ( $isMobileUA ) { -- To view, visit https://gerrit.wikimedia.org/r/322230 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I67743bb5b9512578d0a4efbdd9af9d1c0f586339 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/MobileFrontend Gerrit-Branch: master Gerrit-Owner: Pmiazga <pmia...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits