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;">x</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