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