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

Change subject: Further decouple banners from MobileFrontend. Dependent on 
change 67546.
......................................................................


Further decouple banners from MobileFrontend. Dependent on change 67546.

* Puts banners into template rendering hook instead of article render hook.
* Removed superfluous display:none now that only one banner is needed.
* Converted 'x' close symbol to an HTML entity for i18n.
* Some code reformat fixes from IDE.
* Display clickthrough URL in case user accidentally accessed Zero.

Change-Id: I1c05d1f97e2d6794b5e16eb20062df7153adbc05
---
M includes/PageRenderingHooks.php
1 file changed, 162 insertions(+), 66 deletions(-)

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



diff --git a/includes/PageRenderingHooks.php b/includes/PageRenderingHooks.php
index d7e5346..74fc587 100644
--- a/includes/PageRenderingHooks.php
+++ b/includes/PageRenderingHooks.php
@@ -25,6 +25,9 @@
  * @ingroup Extensions
  */
 class PageRenderingHooks {
+
+       // TODO: Clean up HTML concatenation. Review for any non-escaped user 
input.
+
        private static $formatMobileUrl = '//%s.m.wikipedia.org/wiki/Main_Page';
        private static $formatZeroUrl = 
'//%s.zero.wikipedia.org/wiki/Main_Page';
        private static $originParent = '.wikipedia.org/';
@@ -46,13 +49,39 @@
                return true;
        }
 
-
        public static function onMinervaPreRender( \BaseTemplate $template ) {
-               global $wgRequest;
+               $req = $template->getSkin()->getRequest();
+               $out = $template->getSkin()->getOutput();
+
+               // TODO: alias $template->data where appropriate
+               $bannersSupported = array_key_exists( 'banners', 
$template->data );
 
                $config = self::getConfig();
                if ( $config === null ) {
+                       if ( self::isZeroSubdomain() && $bannersSupported ) {
+                               $unsupported = self::renderUnknownCarrier( 
$req, $out);
+                               $template->data['banners'] = array( 
$unsupported );
+                               $template->set( 'banners', 
$template->data['banners'] );
+                       }
                        return true;
+               }
+
+               $redirectWarningQPS = 'renderZeroRatedRedirect=true&returnto=';
+
+               $warning = self::renderWarning( $config, $req, $out );
+
+               // TODO: If clearer way to put this, make it clearer
+               // trump all existing banners when in a banner-rendering context
+               if ( $bannersSupported ) {
+                       if ( isset( $warning ) ) {
+                               $template->data['banners'] = array( $warning );
+                               $template->set( 'banners', 
$template->data['banners'] );
+                               self::emptyLangLinks( $template );
+                       } else {
+                               $template->data['banners'] = array( 
self::renderBanner( $config ) );
+                               $template->set( 'banners', 
$template->data['banners'] );
+                               self::rewriteLangLinks( $template, $config, 
$redirectWarningQPS, $req );
+                       }
                }
 
                $skin = new SkinTemplate();
@@ -63,35 +92,13 @@
                $privacyText = $skin->footerLink( 
'mobile-frontend-privacy-link-text', 'privacypage' );
                $termsText = wfMessage( 'mobile-frontend-terms-use-text' 
)->parse();
 
-               $licenseText = self::addWarning( $wgRequest, $pattern, 
$licenseText, $redirectWarning ) ;
-               $privacyText = self::addWarning( $wgRequest, $pattern, 
$privacyText, $redirectWarning );
-               $termsText = self::addWarning( $wgRequest, $pattern, 
$termsText, $redirectWarning );
+               $licenseText = self::addWarning( $req, $pattern, $licenseText, 
$redirectWarning );
+               $privacyText = self::addWarning( $req, $pattern, $privacyText, 
$redirectWarning );
+               $termsText = self::addWarning( $req, $pattern, $termsText, 
$redirectWarning );
 
                $template->set( 'mobile-license', $licenseText );
                $template->set( 'privacy', $privacyText );
                $template->set( 'terms-use', $termsText );
-
-               $redirectWarningQPS = 'renderZeroRatedRedirect=true&returnto=';
-
-               if ( isset( $template->data['language_urls'] )
-                       && 0 < count( $template->data['language_urls'] )
-               ) {
-                       $languageUrls = $template->data['language_urls'];
-                       $freeLangs = $config['whitelistedLangs'];
-                       foreach ( $languageUrls as &$lang ) {
-                               if ( 
preg_match('#^//([-a-zA-Z_]+)\.(zero|m)\.#', $lang['href'], $match ) ) {
-                                       $link = $lang['href'];
-                                       $isFree = ( 0 === count( $freeLangs ) 
|| in_array( $match[1], $freeLangs ) );
-                                       if ( !$isFree ) {
-                                               if ( self::isZeroSubdomain() ) {
-                                                       $link = str_replace( 
self::$zerodotParent, self::$mdotParent, $link );
-                                               }
-                                               $lang['href'] = 
$wgRequest->appendQuery( $redirectWarningQPS . urlencode( $link ) );
-                                       }
-                               }
-                       }
-                       $template->set( 'language_urls', $languageUrls );
-               }
 
                return true;
        }
@@ -125,7 +132,7 @@
                        }
                        if ( self::isZeroSubdomain() ) {
                                $referer = $wgRequest->getHeader( 'REFERER' );
-                               if ( strpos( $referer , 'zero.wikipedia.org' ) 
!== false ) {
+                               if ( strpos( $referer, 'zero.wikipedia.org' ) 
!== false ) {
                                        $warn .= ' && zero referer';
                                }
                        }
@@ -143,7 +150,6 @@
                if ( !$showBanner && self::isZeroSubdomain() ) {
                        $out->clearHTML();
                        $out->setPageTitle( null );
-                       $out->addHTML( self::renderUnknownCarrier() );
                        wfProfileOut( __METHOD__ );
                        return true;
                }
@@ -175,23 +181,9 @@
                }
 
                if ( $showBanner && $showWarning && $acceptBilling !== 'yes' ) {
-                       $domainReplacement = self::isZeroSubdomain() ? 
self::$zerodotParent : self::$mdotParent;
-                       $output = self::renderQuestion(
-                               str_replace(
-                                       self::$originParent,
-                                       $domainReplacement,
-                                       wfExpandUrl( $wgRequest->appendQuery( 
'acceptbilling=yes' ), PROTO_CURRENT )
-                               )
-                       );
                        $out->clearHTML();
                        $out->setPageTitle( null );
                } elseif ( $wgRequest->getFuzzyBool( 'renderZeroRatedRedirect' 
) ) {
-                       $output = self::renderQuestion(
-                               $wgRequest->appendQuery(
-                                       
'acceptbilling=yes&renderZeroRatedBanner=true&returnto='
-                                       . urlencode( $wgRequest->getVal( 
'returnto' ) ) ),
-                               true // isWarning
-                       );
                        $out->clearHTML();
                        $out->setPageTitle( null );
                } elseif ( $showBanner && $config !== null ) {
@@ -201,21 +193,18 @@
                        $parsedHtml = self::parseLinksForZeroQueryString( 
$config, $html, $isFilePage );
                        $out->clearHTML();
                        $out->addHTML( $parsedHtml );
-                       $output = self::renderBanner( $config );
-               } else {
-                       $output = '';
                }
 
                if ( $out->getTitle()->isSpecial( 'ZeroRatedMobileAccess' ) ) {
                        $out->clearHTML();
                        $out->setPageTitle( null );
-                       $output .= self::renderSpecialPage( $config );
-               }
-
-               if ( $output ) {
-                       $output = Html::rawElement( 'div',
-                               array( 'id' => 'zero-landing-page' ), $output );
-                       $out->addHTML( $output );
+                       $output = self::renderSpecialPage( $config );
+                       if ( $output ) { // necessary in bizarre case of 
misconfiguraton
+                               // TODO: Remove unnecessary 'id' attributes, if 
applicable
+                               $output = Html::rawElement( 'div',
+                                       array( 'id' => 'zero-landing-page' ), 
$output );
+                               $out->addHTML( $output );
+                       }
                }
 
                wfProfileOut( __METHOD__ );
@@ -224,24 +213,32 @@
 
        /**
         * Render the "red" banner for the unknown IP when accessing zero.*
+        * @param \WebRequest $req
+        * @param OutputPage $out
         * @return string
         */
-       private static function renderUnknownCarrier() {
-               global $wgRequest;
-               $ip = $wgRequest->getIP();
+       private static function renderUnknownCarrier( \WebRequest $req, 
OutputPage $out) {
+               $title = $out->getTitle();
+               $url = $title->getCanonicalURL();
+               $link = Html::element( 'a',
+                       array( 'href' => $url ),
+                       urldecode( $url )
+               );
+               $ip = $req->getIP();
                // @FIXME: Unescaped UI text output as HTML in next 3 lines.
+               // TODO: Remove unnecessary 'id' attributes
                $bannerText = wfMessage( 'zero-rated-mobile-access-sorry' 
)->text();
                $bannerText .= '<br />' . wfMessage( 
'zero-rated-mobile-access-sorry-ip', $ip )->text();
                $bannerText .= '<br />' . wfMessage(
-                       'zero-rated-mobile-access-sorry-goto',
-                       '<a href="http://m.wikipedia.org/";>m.wikipedia.org</a>' 
)->text();
+                               'zero-rated-mobile-access-sorry-goto',
+                               $link )->text();
                $bannerText = Html::rawElement( 'div',
                        array( 'class' => 'mw-mf-message',
                                'id' => 'zero-rated-banner-text' ),
                        $bannerText );
                $banner = Html::rawElement( 'div',
                        array( 'class' => 'mw-mf-banner 
mw-mf-banner-undismissable',
-                               'style' => 'display:none;', 'id' => 
'zero-rated-banner-red' ),
+                               'id' => 'zero-rated-banner-red' ),
                        $bannerText );
                return Html::rawElement( 'div',
                        array( 'id' => 'zero-landing-page' ),
@@ -260,7 +257,7 @@
                        global $wgRequest;
                        $billingURL = $wgRequest->appendQuery(
                                
'renderZeroRatedBanner=true&renderwarning=yes&returnto='
-                                       . urlencode( $config['bannerUrl'] )
+                               . urlencode( $config['bannerUrl'] )
                        );
                        $carrierLink = Html::rawElement( 'a', array( 'href' => 
$billingURL ), $bannerText );
                        if ( !$config['bannerWarning'] ) {
@@ -287,6 +284,7 @@
                        '<a href="',
                        "<a style=\"color:$foreground;\" href=\"",
                        $carrierLink );
+
                $bannerText = Html::rawElement( 'span',
                        array(
                                'class' => 'mw-mf-message',
@@ -295,16 +293,60 @@
                        ),
                        $carrierLink );
                $dismissTitle = wfMessage( 
'zero-rated-mobile-access-dismiss-notification' )->escaped();
-               $displayNone = $lang === null ? 'display:none;' : '';
+               $displayNone = $carrierLink === '' ? 'display:none;' : '';
                $banner = <<<END
 <div id="zero-rated-banner" class="mw-mf-banner" 
style="background:$background;color:$foreground;$displayNone">
 <button class="notify-close" style="background:$background;" 
title="$dismissTitle">
-<span class="notify-close-x" 
style="background:$background;border-color:$foreground;color:$foreground;">×</span>
+<span class="notify-close-x" 
style="background:$background;border-color:$foreground;color:$foreground;">&#120;</span>
 </button>
 $bannerText
 </div>
 END;
                return $banner;
+       }
+
+       /**
+        * Provide an interstitial warning for links that may cause charges.
+        *
+        * @param $config
+        * @param $request \WebRequest
+        * @param $out OutputPage
+        * @return null|string Warning banner if applicable, else null
+        */
+       private static function renderWarning( $config, $request, $out ) {
+
+               $isFilePage = $out->getTitle()->inNamespace( NS_FILE );
+               $acceptBilling = $request->getVal( 'acceptbilling' );
+
+               $showWarning = false;
+               if ( $isFilePage ) {
+                       $showWarning = !$config['showImages'];
+               } else {
+                       $showWarningFlag = $request->getVal( 'renderwarning' );
+                       if ( $showWarningFlag === 'yes' ) {
+                               $showWarning = true;
+                       }
+               }
+
+               if ( $showWarning && $acceptBilling !== 'yes' ) {
+                       $domainReplacement = self::isZeroSubdomain() ? 
self::$zerodotParent : self::$mdotParent;
+                       $question = self::renderQuestion(
+                               str_replace(
+                                       self::$originParent,
+                                       $domainReplacement,
+                                       wfExpandUrl( $request->appendQuery( 
'acceptbilling=yes' ), PROTO_CURRENT )
+                               )
+                       );
+               } elseif ( $request->getFuzzyBool( 'renderZeroRatedRedirect' ) 
) {
+                       $question = self::renderQuestion(
+                               $request->appendQuery(
+                                       
'acceptbilling=yes&renderZeroRatedBanner=true&returnto='
+                                       . urlencode( $request->getVal( 
'returnto' ) ) ),
+                               true // isWarning
+                       );
+               }
+
+               return isset( $question ) ? $question : null;
        }
 
 
@@ -322,6 +364,7 @@
                $acceptBillingNo = Html::rawElement( 'a',
                        array( 'href' => $wgRequest->appendQuery( 
'acceptbilling=no&returnto=' . urlencode( $referrer ) ) ),
                        wfMessage( 
'zero-rated-mobile-access-banner-text-data-charges-no' )->escaped() );
+               // TODO: Remove unnecessary 'id' attributes
                $bannerText = Html::rawElement( 'div',
                        array( 'class' => 'mw-mf-message',
                                'id' => 'zero-rated-banner-text' ),
@@ -333,9 +376,61 @@
                        $cssClass .= ' mw-mf-banner-warning';
                }
                $banner = Html::rawElement( 'div',
-                       array( 'class' => $cssClass,
-                               'style' => 'display:none;', 'id' => 
'zero-rated-banner-red' ), $bannerText );
+                       array( 'class' => $cssClass, 'id' => 
'zero-rated-banner-red' ),
+                       $bannerText
+               );
                return $banner;
+       }
+
+       /**
+        * If a particular language could cause a charge, send user to an 
interstitial.
+        *
+        * @param $template \BaseTemplate
+        * @param $config array containing carrier's configuration
+        * @param $qps string Query path separator
+        * @param $request \WebRequest
+        * @return bool
+        */
+       private static function rewriteLangLinks( $template, $config, $qps, 
$request ) {
+
+               if ( isset( $template->data['language_urls'] )
+                       && 0 < count( $template->data['language_urls'] )
+               ) {
+                       $languageUrls = $template->data['language_urls'];
+                       $freeLangs = $config['whitelistedLangs'];
+                       foreach ( $languageUrls as &$lang ) {
+                               if ( preg_match( 
'#^//([-a-zA-Z_]+)\.(zero|m)\.#', $lang['href'], $match ) ) {
+                                       $link = $lang['href'];
+                                       $isFree = ( 0 === count( $freeLangs ) 
|| in_array( $match[1], $freeLangs ) );
+                                       if ( !$isFree ) {
+                                               if ( self::isZeroSubdomain() ) {
+                                                       $link = str_replace( 
self::$zerodotParent, self::$mdotParent, $link );
+                                               }
+                                               $lang['href'] = 
$request->appendQuery( $qps . urlencode( $link ) );
+                                       }
+                               }
+                       }
+                       $template->set( 'language_urls', $languageUrls );
+               }
+
+               return true;
+       }
+
+       /**
+        * Empty out the Read in Another Language list
+        *
+        * @param \BaseTemplate $template
+        * @return bool
+        */
+       private static function emptyLangLinks( $template ) {
+               if ( isset( $template->data['language_urls'] )
+                       && 0 < count( $template->data['language_urls'] )
+               ) {
+                       $template->data['language_urls'] = array();
+                       $template->set( 'language_urls', 
$template->data['language_urls'] );
+               }
+
+               return true;
        }
 
 
@@ -371,6 +466,7 @@
                }
                $output .= Html::element( 'hr' );
                $output .= wfMessage( 
'zero-rated-mobile-access-home-page-selection-text' )->escaped();
+               // TODO: Remove unnecessary 'id' attributes
                $output .= Html::openElement( 'select',
                        array( 'id' => 'languageselection',
                                'onchange' => 'javascript:window.location = 
this.options[this.selectedIndex].value;',
@@ -391,7 +487,7 @@
                                        // offer upgraded ux
                                        $languageUrl = $wgRequest->appendQuery(
                                                
'renderZeroRatedBanner=true&renderwarning=yes&returnto='
-                                                       . urlencode( 
$languageUrl ) );
+                                               . urlencode( $languageUrl ) );
                                }
                        } else {
                                // send low bandwidth user to whitelisted zero 
subdomain
@@ -743,7 +839,7 @@
        private static function logDebug( $dbg ) {
                static $printedHeaders = false;
                global $wgRequest;
-               $dbg .= "\t" . $wgRequest->getIP() .  "\t" . 
$wgRequest->getFullRequestURL();
+               $dbg .= "\t" . $wgRequest->getIP() . "\t" . 
$wgRequest->getFullRequestURL();
                if ( !$printedHeaders ) {
                        $headers = $wgRequest->getAllHeaders();
                        ksort( $headers );

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

Gerrit-MessageType: merged
Gerrit-Change-Id: I1c05d1f97e2d6794b5e16eb20062df7153adbc05
Gerrit-PatchSet: 7
Gerrit-Project: mediawiki/extensions/ZeroRatedMobileAccess
Gerrit-Branch: master
Gerrit-Owner: Dr0ptp4kt <[email protected]>
Gerrit-Reviewer: Dr0ptp4kt <[email protected]>
Gerrit-Reviewer: Jdlrobson <[email protected]>
Gerrit-Reviewer: MaxSem <[email protected]>
Gerrit-Reviewer: Yurik <[email protected]>
Gerrit-Reviewer: awjrichards <[email protected]>
Gerrit-Reviewer: jenkins-bot

_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to