r99759 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r99758‎ | r99759 | r99760 >
Date:08:01, 14 October 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Documentation

Did some conditional refactoring

Remove some unused variables
Modified paths:
  • /trunk/extensions/MobileFrontend/DeviceDetection.php (modified) (history)
  • /trunk/extensions/MobileFrontend/MobileFrontend.php (modified) (history)
  • /trunk/extensions/MobileFrontend/tests/MobileFrontendTest.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/tests/MobileFrontendTest.php
——@@ -1,10 +1,11 @@
22 <?php
33
44 class ExtMobileFrontendTest extends MediaWikiTestCase {
5 -
65 /**
76 * PHP 5.3.2 introduces the ReflectionMethod::setAccessible() method to allow the invocation of
87 * protected and private methods directly through the Reflection API
 8+ *
 9+ * @param $name string
910 */
1011 protected static function getMethod( $name ) {
1112 $class = new ReflectionClass( 'ExtMobileFrontend' );
——@@ -15,32 +16,34 @@
1617
1718 protected function setUp() {
1819 parent::setUp();
19 - $this->wgExtMobileFrontend = new ExtMobileFrontend();
 20+ global $wgExtMobileFrontend;
 21+ $wgExtMobileFrontend = new ExtMobileFrontend();
2022 }
2123
2224 protected function tearDown() {
23 - unset( $this->wgExtMobileFrontend );
 25+ global $wgExtMobileFrontend;
 26+ unset( $wgExtMobileFrontend );
2427 parent::tearDown();
2528 }
2629
2730 public function testGetBaseDomain() {
 31+ global $wgExtMobileFrontend;
2832 $getBaseDomain = self::getMethod( 'getBaseDomain' );
29 - $wgExtMobileFrontend = new ExtMobileFrontend();
3033 $_SERVER['HTTP_HOST'] = 'en.wikipedia.org';
3134 $this->assertEquals( '.wikipedia.org', $getBaseDomain->invokeArgs( $wgExtMobileFrontend, array() ) );
3235 }
3336
3437 public function testGetRelativeURL() {
 38+ global $wgExtMobileFrontend;
3539 $getRelativeURL = self::getMethod( 'getRelativeURL' );
36 - $wgExtMobileFrontend = new ExtMobileFrontend();
3740 $url = 'http://en.wikipedia.org/wiki/Positional_astronomy';
3841 $this->assertEquals( '/wiki/Positional_astronomy', $getRelativeURL->invokeArgs( $wgExtMobileFrontend, array( $url ) ) );
3942 }
4043
4144 public function testDisableCaching() {
42 - global $wgRequest;
 45+ global $wgRequest, $wgExtMobileFrontend;
4346 $disableCaching = self::getMethod( 'disableCaching' );
44 - $wgExtMobileFrontend = new ExtMobileFrontend();
 47+
4548 $_SERVER['HTTP_VIA'] = '.wikimedia.org:3128';
4649 $disableCaching->invokeArgs( $wgExtMobileFrontend, array() );
4750 $this->assertEquals( 'no-cache, must-revalidate', $wgRequest->response()->getheader( 'Cache-Control' ) );
——@@ -49,9 +52,8 @@
5053 }
5154
5255 public function testSendXDeviceVaryHeader() {
53 - global $wgRequest;
 56+ global $wgRequest, $wgExtMobileFrontend;
5457 $sendXDeviceVaryHeader = self::getMethod( 'sendXDeviceVaryHeader' );
55 - $wgExtMobileFrontend = new ExtMobileFrontend();
5658 $_SERVER['HTTP_X_DEVICE'] = 'device';
5759 $sendXDeviceVaryHeader->invokeArgs( $wgExtMobileFrontend, array() );
5860 $this->assertEquals( $_SERVER['HTTP_X_DEVICE'], $wgRequest->response()->getheader( 'X-Device' ) );
Index: trunk/extensions/MobileFrontend/DeviceDetection.php
——@@ -249,11 +249,18 @@
250250 return $formats;
251251 }
252252
 253+ /**
 254+ * @param $formatName
 255+ * @return array
 256+ */
253257 public function format( $formatName ) {
254258 $format = $this->availableFormats();
255259 return ( isset( $format[$formatName] ) ) ? $format[$formatName] : array();
256260 }
257261
 262+ /**
 263+ * @return string
 264+ */
258265 public function testFormatName() {
259266 $testResults = '';
260267
Index: trunk/extensions/MobileFrontend/MobileFrontend.php
——@@ -72,12 +72,15 @@
7373 * Make the classes stripped from page content configurable. Each item will
7474 * be stripped from the page. See $itemsToRemove for more info
7575 */
76 -$wgMFRemovableClasses = array(
77 -);
 76+$wgMFRemovableClasses = array();
7877
7978 # Unit tests
8079 $wgHooks['UnitTestsList'][] = 'efExtMobileFrontendUnitTests';
8180
 81+/**
 82+ * @param $files array
 83+ * @return bool
 84+ */
8285 function efExtMobileFrontendUnitTests( &$files ) {
8386 $files[] = dirname( __FILE__ ) . '/tests/MobileFrontendTest.php';
8487 return true;
——@@ -89,8 +92,7 @@
9093 /**
9194 * @var DOMDocument
9295 */
93 - private $doc;
94 - private $mainPage;
 96+ private $doc, $mainPage;
9597
9698 public static $messages = array();
9799
——@@ -200,6 +202,13 @@
201203 '.nomobile',
202204 );
203205
 206+ /**
 207+ * @param $request WebRequest
 208+ * @param $title Title
 209+ * @param $output OutputPage
 210+ * @return bool
 211+ * @throws HttpError
 212+ */
204213 public function testCanonicalRedirect( $request, $title, $output ) {
205214 global $wgUsePathInfo, $wgMobileDomain;
206215 $xDevice = isset( $_SERVER['HTTP_X_DEVICE'] ) ? $_SERVER['HTTP_X_DEVICE'] : '';
——@@ -251,6 +260,11 @@
252261 }
253262 }
254263
 264+ /**
 265+ * @param $obj Article
 266+ * @param $tpl
 267+ * @return bool
 268+ */
255269 public function addMobileFooter( &$obj, &$tpl ) {
256270 global $wgRequest;
257271 wfProfileIn( __METHOD__ );
——@@ -269,7 +283,12 @@
270284 wfProfileOut( __METHOD__ );
271285 return true;
272286 }
273 -
 287+
 288+ /**
 289+ * @param $url string
 290+ * @param $field string
 291+ * @return string
 292+ */
274293 private function removeQueryStringParameter( $url, $field ) {
275294 $url = preg_replace( '/(.*)(\?|&)' . $field . '=[^&]+?(&)(.*)/i', '$1$2$4', $url . '&' );
276295 $url = substr( $url, 0, -1 );
——@@ -344,6 +363,7 @@
345364
346365 $key = wfMemcKey( 'mobile', 'ua', $uAmd5 );
347366
 367+ $props = null;
348368 try {
349369 $props = $wgMemc->get( $key );
350370 if ( ! $props ) {
——@@ -465,28 +485,22 @@
466486 exit();
467487 }
468488
469 - if ( $mobileAction == 'disable_mobile_site' ) {
470 - if ( $this->contentFormat == 'XHTML' ) {
471 - echo $this->renderDisableMobileSiteXHTML();
472 - wfProfileOut( __METHOD__ );
473 - exit();
474 - }
 489+ if ( $mobileAction == 'disable_mobile_site' && $this->contentFormat == 'XHTML' ) {
 490+ echo $this->renderDisableMobileSiteXHTML();
 491+ wfProfileOut( __METHOD__ );
 492+ exit();
475493 }
476494
477 - if ( $mobileAction == 'opt_in_mobile_site' ) {
478 - if ( $this->contentFormat == 'XHTML' ) {
479 - echo $this->renderOptInMobileSiteXHTML();
480 - wfProfileOut( __METHOD__ );
481 - exit();
482 - }
 495+ if ( $mobileAction == 'opt_in_mobile_site' && $this->contentFormat == 'XHTML' ) {
 496+ echo $this->renderOptInMobileSiteXHTML();
 497+ wfProfileOut( __METHOD__ );
 498+ exit();
483499 }
484500
485 - if ( $mobileAction == 'opt_out_mobile_site' ) {
486 - if ( $this->contentFormat == 'XHTML' ) {
487 - echo $this->renderOptOutMobileSiteXHTML();
488 - wfProfileOut( __METHOD__ );
489 - exit();
490 - }
 501+ if ( $mobileAction == 'opt_out_mobile_site' && $this->contentFormat == 'XHTML' ) {
 502+ echo $this->renderOptOutMobileSiteXHTML();
 503+ wfProfileOut( __METHOD__ );
 504+ exit();
491505 }
492506
493507 if ( $mobileAction == 'opt_in_cookie' ) {
——@@ -502,7 +516,7 @@
503517
504518 // WURFL documentation: http://wurfl.sourceforge.net/help_doc.php
505519 // Determine the kind of markup
506 - if ( is_array( $props ) && $props['preferred_markup'] ) {
 520+ if ( is_array( $props ) && isset( $props['preferred_markup'] ) && $props['preferred_markup'] ) {
507521 // wfDebug( __METHOD__ . ": preferred markup for this device: " . $props['preferred_markup'] );
508522 // xhtml/html: html_web_3_2, html_web_4_0
509523 // xthml basic/xhtmlmp (wap 2.0): html_wi_w3_xhtmlbasic html_wi_oma_xhtmlmp_1_0
——@@ -538,6 +552,9 @@
539553 return true;
540554 }
541555
 556+ /**
 557+ * @param $value string
 558+ */
542559 private function setOptInOutCookie( $value ) {
543560 global $wgCookieDomain, $wgRequest;
544561 wfProfileIn( __METHOD__ );
——@@ -548,6 +565,9 @@
549566 wfProfileOut( __METHOD__ );
550567 }
551568
 569+ /**
 570+ * @return string
 571+ */
552572 private function getBaseDomain() {
553573 wfProfileIn( __METHOD__ );
554574 // Validates value as IP address
——@@ -557,12 +577,15 @@
558578 // Although some browsers will accept cookies without the initial ., » RFC 2109 requires it to be included.
559579 wfProfileOut( __METHOD__ );
560580 return '.' . $domainParts[1] . '.' . $domainParts[0];
561 - } else {
562 - wfProfileOut( __METHOD__ );
563 - return $_SERVER['HTTP_HOST'];
564581 }
 582+ wfProfileOut( __METHOD__ );
 583+ return $_SERVER['HTTP_HOST'];
565584 }
566585
 586+ /**
 587+ * @param $url string
 588+ * @return string
 589+ */
567590 private function getRelativeURL( $url ) {
568591 wfProfileIn( __METHOD__ );
569592 $parsedUrl = parse_url( $url );
——@@ -572,10 +595,9 @@
573596 $baseUrl = $parsedUrl['scheme'] . '://' . $parsedUrl['host'];
574597 $baseUrl = str_replace( $baseUrl, '', $url );
575598 return $baseUrl;
576 - } else {
577 - wfProfileOut( __METHOD__ );
578 - return $url;
579599 }
 600+ wfProfileOut( __METHOD__ );
 601+ return $url;
580602 }
581603
582604 private function disableCaching() {
——@@ -600,6 +622,9 @@
601623 wfProfileOut( __METHOD__ );
602624 }
603625
 626+ /**
 627+ * @return string
 628+ */
604629 private function renderLeaveFeedbackXHTML() {
605630 global $wgRequest, $wgUser;
606631 wfProfileIn( __METHOD__ );
——@@ -628,6 +653,9 @@
629654 return '';
630655 }
631656
 657+ /**
 658+ * @return string
 659+ */
632660 private function renderOptInMobileSiteXHTML() {
633661 wfProfileIn( __METHOD__ );
634662 if ( $this->contentFormat == 'XHTML' ) {
——@@ -649,6 +677,9 @@
650678 return '';
651679 }
652680
 681+ /**
 682+ * @return string
 683+ */
653684 private function renderOptOutMobileSiteXHTML() {
654685 wfProfileIn( __METHOD__ );
655686 if ( $this->contentFormat == 'XHTML' ) {
——@@ -670,6 +701,9 @@
671702 return '';
672703 }
673704
 705+ /**
 706+ * @return string
 707+ */
674708 private function renderDisableMobileSiteXHTML() {
675709 wfProfileIn( __METHOD__ );
676710 if ( $this->contentFormat == 'XHTML' ) {
——@@ -692,6 +726,10 @@
693727 return '';
694728 }
695729
 730+ /**
 731+ * @param $matches array
 732+ * @return string
 733+ */
696734 private function headingTransformCallbackWML( $matches ) {
697735 wfProfileIn( __METHOD__ );
698736 static $headings = 0;
——@@ -705,6 +743,10 @@
706744 return $base;
707745 }
708746
 747+ /**
 748+ * @param $matches array
 749+ * @return string
 750+ */
709751 private function headingTransformCallbackXHTML( $matches ) {
710752 wfProfileIn( __METHOD__ );
711753 if ( isset( $matches[0] ) ) {
——@@ -720,13 +762,13 @@
721763 ++$headings;
722764 // Back to top link
723765 $base = Html::openElement( 'div',
724 - array( 'id' => 'anchor_' . intval( $headings - 1 ),
725 - 'class' => 'section_anchors', )
 766+ array( 'id' => 'anchor_' . intval( $headings - 1 ),
 767+ 'class' => 'section_anchors', )
726768 ) .
727769 Html::rawElement( 'a',
728770 array( 'href' => '#section_' . intval( $headings - 1 ),
729771 'class' => 'back_to_top' ),
730 - '&#8593;' . $backToTop ) .
 772+ '&#8593;' . $backToTop ) .
731773 Html::closeElement( 'div' );
732774 // generate the HTML we are going to inject
733775 $buttons = Html::element( 'button',
——@@ -789,8 +831,11 @@
790832 return $s;
791833 }
792834
 835+ /**
 836+ * @param $s string
 837+ * @return string
 838+ */
793839 private function createWMLCard( $s ) {
794 - global $wgRequest;
795840 wfProfileIn( __METHOD__ );
796841 $segments = explode( $this->WMLSectionSeperator, $s );
797842 $card = '';
——@@ -833,6 +878,9 @@
834879 return $card;
835880 }
836881
 882+ /**
 883+ * @return array
 884+ */
837885 private function parseItemsToRemove() {
838886 global $wgMFRemovableClasses;
839887 wfProfileIn( __METHOD__ );
——@@ -851,6 +899,9 @@
852900 return $itemToRemoveRecords;
853901 }
854902
 903+ /**
 904+ * @param $html string
 905+ */
855906 public function DOMParseMainPage( $html ) {
856907 wfProfileIn( __METHOD__ );
857908 $html = mb_convert_encoding( $html, 'HTML-ENTITIES', "UTF-8" );
——@@ -907,8 +958,12 @@
908959 return $contentHtml;
909960 }
910961
 962+ /**
 963+ * @param $html string
 964+ * @return string
 965+ */
911966 public function DOMParse( $html ) {
912 - global $wgSitename, $wgScript;
 967+ global $wgScript;
913968 wfProfileIn( __METHOD__ );
914969 $html = mb_convert_encoding( $html, 'HTML-ENTITIES', "UTF-8" );
915970 libxml_use_internal_errors( true );
——@@ -958,7 +1013,7 @@
9591014 foreach ( $itemToRemoveRecords['ID'] as $itemToRemove ) {
9601015 $itemToRemoveNode = $this->doc->getElementById( $itemToRemove );
9611016 if ( $itemToRemoveNode ) {
962 - $removedItemToRemove = $itemToRemoveNode->parentNode->removeChild( $itemToRemoveNode );
 1017+ $itemToRemoveNode->parentNode->removeChild( $itemToRemoveNode );
9631018 }
9641019 }
9651020
——@@ -968,7 +1023,7 @@
9691024 $elements = $xpath->query( '//*[@class="' . $classToRemove . '"]' );
9701025
9711026 foreach ( $elements as $element ) {
972 - $removedElement = $element->parentNode->removeChild( $element );
 1027+ $element->parentNode->removeChild( $element );
9731028 }
9741029 }
9751030
——@@ -1008,9 +1063,6 @@
10091064 $contentHtml = $this->DOMParseMainPage( $contentHtml );
10101065 }
10111066
1012 - $title = htmlspecialchars( self::$title->getText() );
1013 - $htmlTitle = htmlspecialchars( self::$htmlTitle );
1014 -
10151067 if ( strlen( $contentHtml ) > 4000 && $this->contentFormat == 'XHTML'
10161068 && self::$device['supports_javascript'] === true
10171069 && empty( self::$search ) && !self::$isMainPage ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r99787mft r99759 r99786preilly18:41, 14 October 2011
r99788mft r99759 r99786preilly18:42, 14 October 2011

Comments

#Comment by G.Hagedorn (talk | contribs)   08:56, 16 October 2011

applied to both 1.17 and 1.18wmf1. Tag for 1.18?

Status & tagging log