r97026 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r97025‎ | r97026 | r97027 >
Date:00:16, 14 September 2011
Author:preilly
Status:resolved (Comments)
Tags:
Comment:
fix for wml view pagination links
Modified paths:
  • /trunk/extensions/MobileFrontend/MobileFrontend.php (modified) (history)

Diff [purge]

Index: trunk/extensions/MobileFrontend/MobileFrontend.php
——@@ -208,7 +208,7 @@
209209 self::$enableImagesURL = $wgRequest->escapeAppendQuery( 'enableImages=1' );
210210 self::$disableMobileSiteURL = $wgRequest->escapeAppendQuery( 'mobileaction=disable_mobile_site' );
211211 self::$viewNormalSiteURL = $wgRequest->escapeAppendQuery( 'mobileaction=view_normal_site' );
212 - self::$currentURL = htmlspecialchars( $wgRequest->getFullRequestURL() );
 212+ self::$currentURL = $wgRequest->getFullRequestURL();
213213 self::$leaveFeedbackURL = $wgRequest->escapeAppendQuery( 'mobileaction=leave_feedback' );
214214
215215 $skin = $wgUser->getSkin();
——@@ -707,6 +707,12 @@
708708 return $s;
709709 }
710710
 711+ private function removeQuerystringVar( $url, $key ) {
 712+ $url = preg_replace( '/(.*)(\?|&)' . $key . '=[^&]+?(&)(.*)/i', '$1$2$4', $url . '&' );
 713+ $url = substr( $url, 0, -1 );
 714+ return ( $url );
 715+ }
 716+
711717 private function createWMLCard( $s ) {
712718 wfProfileIn( __METHOD__ );
713719 $segments = explode( $this->WMLSectionSeperator, $s );
——@@ -722,18 +728,22 @@
723729
724730 $useFormatParam = ( isset( self::$useFormat ) ) ? '&' . 'useformat=' . self::$useFormat : '';
725731
726 - $basePage = htmlspecialchars( $_SERVER['PHP_SELF'] );
 732+ $basePage = self::$currentURL;
727733
 734+ $basePage = $this->removeQuerystringVar( $this->removeQuerystringVar( $basePage, 'useformat' ), 'seg' );
 735+
 736+ $delimiter = ( strpos( $basePage, '?' ) === false ) ? '?' : '&';
 737+
728738 if ( $idx < $segmentsCount ) {
729 - $card .= "<p><a href=\"{$basePage}?seg={$idx}{$useFormatParam}\">" . self::$messages['mobile-frontend-wml-continue'] . "</a></p>";
 739+ $card .= "<p><a href=\"{$basePage}{$delimiter}seg={$idx}{$useFormatParam}\">" . self::$messages['mobile-frontend-wml-continue'] . "</a></p>";
730740 }
731741
732742 if ( $idx > 1 ) {
733743 $back_idx = $requestedSegment - 1;
734 - $card .= "<p><a href=\"{$basePage}?seg={$back_idx}{$useFormatParam}\">" . self::$messages['mobile-frontend-wml-back'] . "</a></p>";
 744+ $card .= "<p><a href=\"{$basePage}{$delimiter}seg={$back_idx}{$useFormatParam}\">" . self::$messages['mobile-frontend-wml-back'] . "</a></p>";
735745 }
736746
737 - $card .= '</div></card>';
 747+ $card .= '</card>';
738748 wfProfileOut( __METHOD__ );
739749 return $card;
740750 }
——@@ -921,7 +931,7 @@
922932 && empty( self::$search ) && !self::$isMainPage ) {
923933 $contentHtml = $this->headingTransform( $contentHtml );
924934 } elseif ( $this->contentFormat == 'WML' ) {
925 - header( 'Content-Type: text/vnd.wap.wml' );
 935+ //header( 'Content-Type: text/vnd.wap.wml' );
926936
927937 // TODO: Content transformations required
928938 // WML Validator:

Follow-up revisions

RevisionCommit summaryAuthorDate
r97027mft r97026preilly00:16, 14 September 2011
r97076fix for r97026 based on c22503preilly17:38, 14 September 2011

Comments

#Comment by 😂 (talk | contribs)   00:57, 14 September 2011

This seems incredibly prone to breakage and wish it had been reviewed before it was merged to 1.17wmf1. Suggest using WebRequest::getQueryValues(), remove the array keys you don't want, then build a fresh url from the new query.

#Comment by Catrope (talk | contribs)   10:12, 14 September 2011

Addendum: you can build a URL from a query array with wfArrayToCGI() or wfAppendQuery().

#Comment by Nikerabbit (talk | contribs)   06:52, 14 September 2011

Or use wfParseUrl.

#Comment by Preilly (talk | contribs)   17:40, 14 September 2011

This should be fixed in r97076. But, note that Title::getLocalUrl doesn't work at this point in the class since PHP 5.1.x, all objects have their destructors called before the output buffer callback function executes. So, globalized objects will not be available as expected in the function. This is stated to be intended behavior, as per the following: [1]

Status & tagging log