jenkins-bot has submitted this change and it was merged.

Change subject: Cleanup - in preparation for ESI
......................................................................


Cleanup - in preparation for ESI

* Optimizing to use Title instead of Output object
* Added getConfigId()

Change-Id: Ifd588d87eaeb1e243a2b8b28bc1ad26c2ce16d66
---
M includes/PageRenderingHooks.php
M includes/ZeroSpecialPage.php
2 files changed, 50 insertions(+), 40 deletions(-)

Approvals:
  Dr0ptp4kt: Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/PageRenderingHooks.php b/includes/PageRenderingHooks.php
index ade0feb..2bf1c15 100644
--- a/includes/PageRenderingHooks.php
+++ b/includes/PageRenderingHooks.php
@@ -44,7 +44,7 @@
         */
        public static function onGetMobileUrl( &$subdomainTokenReplacement ) {
                // TODO: logic check
-               if ( self::isZeroSubdomain() && self::disableImages() ) {
+               if ( self::isZeroSubdomain() ) {
                        $subdomainTokenReplacement = 'zero.';
                }
                return true;
@@ -62,7 +62,7 @@
                }
 
                $config = self::getConfig();
-               if ( $config === null ) {
+               if ( $config === null || $context->getTitle()->isSpecial( 
'ZeroRatedMobileAccess' ) ) {
                        return true;
                }
 
@@ -73,6 +73,7 @@
                $doc = $formatter->getDoc();
                $xpath = new DOMXpath( $doc );
 
+               // ZERO site - replace all images with links to images
                if ( !$isFilePage && self::disableImages() ) {
                        $tagToReplaceNodes = $doc->getElementsByTagName( 'img' 
);
                        $tagToReplaceNodesCollection = array();
@@ -91,7 +92,6 @@
                                        $tagToReplaceNodesCollection[] = array( 
'tagToReplaceNode' => $tagToReplaceNode, 'spanNode' => $spanNode );
                                }
                        }
-
                        foreach ( $tagToReplaceNodesCollection as $element ) {
                                
$element['tagToReplaceNode']->parentNode->replaceChild( $element['spanNode'], 
$element['tagToReplaceNode'] );
                        }
@@ -135,7 +135,7 @@
         */
        public static function onMinervaPreRender( MinervaTemplate $template ) {
                $req = $template->getSkin()->getRequest();
-               $out = $template->getSkin()->getOutput();
+               $title = $template->getSkin()->getTitle();
 
                // TODO: alias $template->data where appropriate
                $bannersSupported = array_key_exists( 'banners', 
$template->data );
@@ -143,7 +143,7 @@
                $config = self::getConfig();
                if ( $config === null ) {
                        if ( self::isZeroSubdomain() && $bannersSupported ) {
-                               $unsupported = self::renderUnknownCarrier( 
$req, $out, false );
+                               $unsupported = self::renderUnknownCarrier( 
$req, $title, false );
                                $template->set( 'banners', array( $unsupported 
) );
                        }
                        return true;
@@ -151,7 +151,7 @@
 
                $redirectWarningQPS = 'renderZeroRatedRedirect=true&returnto=';
 
-               $warning = self::renderWarning( $config, $req, $out, false );
+               $warning = self::renderWarning( $config, $req, $title, false );
 
                // TODO: If clearer way to put this, make it clearer
                // trump all existing banners when in a banner-rendering context
@@ -185,18 +185,18 @@
                &$notice
        ) {
                global $wgRequest;
-               global $wgOut;
 
                $config = self::getConfig();
+               $title = $sk->getTitle();
 
                if ( $config === null ) {
                        if ( self::isZeroSubdomain() ) {
-                               $notice = self::renderUnknownCarrier( 
$wgRequest, $wgOut, true );
+                               $notice = self::renderUnknownCarrier( 
$wgRequest, $title, true );
                        }
                        return true;
                }
 
-               $warning = self::renderWarning( $config, $wgRequest, $wgOut, 
true );
+               $warning = self::renderWarning( $config, $wgRequest, $title, 
true );
 
                if ( isset( $warning ) ) {
                        $notice = $warning;
@@ -227,14 +227,8 @@
 
                wfProfileIn( __METHOD__ );
 
-               $out->addVaryHeader( 'X-CS' );
-               $out->addVaryHeader( 'X-Subdomain' );
-               $out->addVaryHeader( 'X-Images' );
-
-               $config = self::getConfig();
-
                // @FIXME: Debug checks - should be remove once all issues are 
gone
-               if ( $config === null ) {
+               if ( self::getConfigId() === null ) {
                        $warn = '';
                        if ( $wgRequest->getCheck( 'renderZeroRatedBanner' ) ||
                                $wgRequest->getCheck( 'acceptbilling' ) ||
@@ -254,10 +248,15 @@
                        }
                }
 
+               $out->addVaryHeader( 'X-CS' );
+               $out->addVaryHeader( 'X-Subdomain' );
+               $out->addVaryHeader( 'X-Images' );
+
                // @FIXME: Should not be added for non-zero sites
                $out->addModuleStyles( 'mobile.zero.styles' );
                $out->addModules( 'mobile.zero.scripts' );
 
+               $config = self::getConfig();
                $isFilePage = $out->getTitle()->inNamespace( NS_FILE );
                $showBanner = $config !== null || $wgRequest->getFuzzyBool( 
'renderZeroRatedBanner' );
                if ( !$showBanner && self::isZeroSubdomain() ) {
@@ -310,12 +309,11 @@
        /**
         * Render the "red" banner for the unknown IP when accessing zero.*
         * @param WebRequest $req
-        * @param OutputPage $out
+        * @param Title $title
         * @param bool $wap
         * @return string
         */
-       private static function renderUnknownCarrier( WebRequest $req, 
OutputPage $out, $wap = false ) {
-               $title = $out->getTitle();
+       private static function renderUnknownCarrier( WebRequest $req, Title 
$title, $wap = false ) {
                $url = $title->getCanonicalURL();
                $link = Html::element( 'a',
                        array( 'href' => $url ),
@@ -417,14 +415,13 @@
         *
         * @param $config
         * @param $request
-        * @param $out
-        * @param bool $wap
+        * @param Title $title
         * @param bool $wap
         * @return null|string Warning banner if applicable, else null
         */
-       private static function renderWarning( $config, WebRequest $request, 
OutputPage $out, $wap = false ) {
+       private static function renderWarning( $config, WebRequest $request, 
Title $title, $wap = false ) {
 
-               $isFilePage = $out->getTitle()->inNamespace( NS_FILE );
+               $isFilePage = $title->inNamespace( NS_FILE );
                $acceptBilling = $request->getVal( 'acceptbilling' );
 
                $showWarning = false;
@@ -612,6 +609,26 @@
        }
 
        /**
+        * Return Carrier ID (X-CS) value or null if no value was set
+        * @return null|string
+        */
+       public static function getConfigId() {
+               static $id = false;
+               if ( $id === false ) {
+                       global $wgRequest;
+                       // Allow URL override of the X-CS parameter for testing 
purposes
+                       $id = $wgRequest->getVal( 'X-CS' );
+                       if ( $id === null ) {
+                               $id = $wgRequest->getHeader( 'X-CS' );
+                       }
+                       if ( $id === '(null)' || !$id ) {
+                               $id = null;
+                       }
+               }
+               return $id;
+       }
+
+       /**
         * Returns cached carrier configuration based on X-CS header or query 
parameter
         * @return array|null Carrier configuration or null if it doesn't exist 
or is disabled
         */
@@ -624,17 +641,9 @@
                }
 
                wfProfileIn( __METHOD__ );
-               global $wgRequest;
                $config = null;
                // Allow URL override of the X-CS parameter for testing purposes
-               $id = $wgRequest->getVal( 'X-CS' );
-               $isDebug = $id !== null;
-               if ( !$isDebug ) {
-                       $id = $wgRequest->getHeader( 'X-CS' );
-               }
-               if ( $id === '(null)' || !$id ) {
-                       $id = null;
-               }
+               $id = self::getConfigId();
                if ( $id !== null ) {
                        $store = new CarrierConfigStore( $id );
                        $text = $store->get();
@@ -642,7 +651,7 @@
                                $conf = new CarrierConfig( $text );
                                if ( !$conf->isError() ) {
                                        $config = $conf->getData();
-                                       if ( !self::isZeroEnabled( $config, 
$isDebug ) ) {
+                                       if ( !self::isZeroEnabled( $config ) ) {
                                                $config = null;
                                        }
                                }
@@ -656,12 +665,13 @@
         * As part of self::getConfig(), determine if zero should be enabled
         * for this site based on configuration
         * @param array $config
-        * @param bool $isDebug If true, overrides 'enabled' config value
         * @return bool
         */
-       private static function isZeroEnabled( $config, $isDebug ) {
-               if ( !$isDebug && !$config['enabled'] ) {
-                       // If the configuration is disabled, pretend like it 
doesn't exist
+       private static function isZeroEnabled( $config ) {
+               global $wgRequest;
+               if ( !$config['enabled'] && !$wgRequest->getCheck( 'X-CS' )) {
+                       // If the configuration is disabled, pretend like it 
doesn't exist,
+                       // unless the configuration was specified with the X-CS 
request parameter
                        return false;
                }
                // Check if the wiki is in the allowed list
diff --git a/includes/ZeroSpecialPage.php b/includes/ZeroSpecialPage.php
index 675b927..4b6d0d0 100644
--- a/includes/ZeroSpecialPage.php
+++ b/includes/ZeroSpecialPage.php
@@ -25,9 +25,10 @@
        /**
         * Show the special page
         *
-        * @param $par Mixed: parameter passed to the special page or null
+        * @param null|string $par parameter passed to the special page or null
         */
        public function execute( $par ) {
+               $out = $this->getOutput();
                $this->setHeaders();
                $config = PageRenderingHooks::getConfig();
 
@@ -36,11 +37,10 @@
                }
 
                if ( !$config['showZeroPage'] ) {
-                       $this->getOutput()->redirect( self::getMainPage(), 
'301' );
+                       $out->redirect( self::getMainPage(), '301' );
                        return;
                }
 
-               $out = $this->getOutput();
                $out->setPageTitle( null );
                $output = $this->renderSpecialPage( $config );
                $out->addHTML( $output );

-- 
To view, visit https://gerrit.wikimedia.org/r/84730
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifd588d87eaeb1e243a2b8b28bc1ad26c2ce16d66
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/ZeroRatedMobileAccess
Gerrit-Branch: master
Gerrit-Owner: Yurik <yu...@wikimedia.org>
Gerrit-Reviewer: Dr0ptp4kt <ab...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to