r65051 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r65050‎ | r65051 | r65052 >
Date:10:01, 15 April 2010
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
Changes for 0.6 - Fixed bug caused by a change to Validator and added some escaping for messages
Modified paths:
  • /trunk/extensions/Maps/ParserFunctions/GeoFunctions/Maps_GeoFunctions.php (modified) (history)
  • /trunk/extensions/Maps/ParserFunctions/Maps_ParserFunctions.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Maps/ParserFunctions/GeoFunctions/Maps_GeoFunctions.php
@@ -190,7 +190,6 @@
191191 ),
192192 array( 'location', 'bearing', 'distance' )
193193 );
194 -
195194 $doCalculation = $parameters !== false;
196195
197196 if ( $doCalculation ) {
@@ -203,7 +202,9 @@
204203 }
205204
206205 if ( $location ) {
 206+ var_dump($location);
207207 $destination = self::findDestination( $location, $parameters['bearing'], $parameters['distance'] );
 208+ var_dump($destination);exit;
208209 $output = MapsCoordinateParser::formatCoordinates( $destination, $parameters['format'], $parameters['directional'] );
209210 } else {
210211 global $egValidatorFatalLevel;
@@ -279,6 +280,8 @@
280281 * @return array The desitination coordinates, as non-directional floats in an array with lat and lon keys.
281282 */
282283 public static function findDestination( array $startingCoordinates, $bearing, $distance ) {
 284+ $startingCoordinates['lat'] = (float)$startingCoordinates['lat'];
 285+ $startingCoordinates['lon'] = (float)$startingCoordinates['lon'];
283286 $angularDistance = $distance / Maps_EARTH_RADIUS;
284287 $lat = asin(
285288 sin( $startingCoordinates['lat'] ) * cos( $angularDistance ) +
Index: trunk/extensions/Maps/ParserFunctions/Maps_ParserFunctions.php
@@ -78,7 +78,7 @@
7979 * @return array
8080 */
8181 public static function getMapHtml( Parser &$parser, array $params, $parserFunction ) {
82 - global $wgLang, $egValidatorErrorLevel;
 82+ global $wgLang, $egValidatorErrorLevel, $egValidatorFatalLevel;
8383
8484 array_shift( $params ); // We already know the $parser.
8585
@@ -116,25 +116,22 @@
117117
118118 if ( $egValidatorErrorLevel >= Validator_ERRORS_WARN ) {
119119 if ( count( $coordFails ) > 0 ) {
120 - // TODO: escaping
121 - $output .= '<i>' . wfMsgExt( 'maps_unrecognized_coords_for', array( 'parsemag' ), $wgLang->listToText( $coordFails ), count( $coordFails ) ) . '</i>';
 120+ $output .= '<i>' . htmlspecialchars( wfMsgExt( 'maps_unrecognized_coords_for', array( 'parsemag' ), $wgLang->listToText( $coordFails ), count( $coordFails ) ) ) . '</i>';
122121 }
123122
124123 if ( count( $geoFails ) > 0 ) {
125 - // TODO: escaping
126 - $output .= '<i>' . wfMsgExt( 'maps_geocoding_failed_for', array( 'parsemag' ), $wgLang->listToText( $geoFails ), count( $geoFails ) ) . '</i>';
 124+ $output .= '<i>' . htmlspecialchars( wfMsgExt( 'maps_geocoding_failed_for', array( 'parsemag' ), $wgLang->listToText( $geoFails ), count( $geoFails ) ) ) . '</i>';
127125 }
128126 }
129127 }
130 - elseif ( $egValidatorErrorLevel >= Validator_ERRORS_MINIMAL ) {
 128+ elseif ( $egValidatorFatalLevel >= Validator_ERRORS_NONE ) {
131129 if ( $coords == '' && ( count( $geoFails ) > 0 || count( $coordFails ) > 0 ) ) {
132 - // TODO: escaping
133130 if ( count( $coordFails ) > 0 ) $output = '<i>' . wfMsgExt( 'maps_unrecognized_coords', array( 'parsemag' ), $wgLang->listToText( $coordFails ), count( $coordFails ) ) . '</i>';
134131 if ( count( $geoFails ) > 0 ) $output = '<i>' . wfMsgExt( 'maps_geocoding_failed', array( 'parsemag' ), $wgLang->listToText( $geoFails ), count( $geoFails ) ) . '</i>';
135 - $output .= '<i>' . wfMsg( 'maps_map_cannot_be_displayed' ) . '</i>';
 132+ $output .= '<i>' . htmlspecialchars( wfMsg( 'maps_map_cannot_be_displayed' ) ) . '</i>';
136133 }
137134 else {
138 - $output = '<i>' . wfMsg( 'maps_coordinates_missing' ) . '</i>';
 135+ $output = '<i>' . htmlspecialchars( wfMsg( 'maps_coordinates_missing' ) ) . '</i>';
139136 }
140137 }
141138

Follow-up revisions

RevisionCommit summaryAuthorDate
r65053Follow up to r65051jeroendedauw10:46, 15 April 2010
r65057Follow up to r65051 - tried to fix indentingjeroendedauw12:01, 15 April 2010

Comments

#Comment by Nikerabbit (talk | contribs)   10:22, 15 April 2010
+var_dump($destination);exit;

Is there some logic when there is $output = and when $output .=? There is also some weird indentation in Maps_ParserFunctions.php. Is there some specific reason you are using for loops instead of foreach loops while iterating over the params?

#Comment by Jeroen De Dauw (talk | contribs)   10:51, 15 April 2010

Oops, didn't mean to commit these debug lines :o

I'm using fors in MapsParserFunctions since in the loops array keys are getting modified, which would result into problems when using foreach if I'm not mistaken. I'm going to rewrite that code at some point, so maybe it'll even go then.

Can you point me to the weird indentation?

#Comment by Nikerabbit (talk | contribs)   11:43, 15 April 2010

Ah, that makes sense.

See for example filterInvalidCoords at http://svn.wikimedia.org/viewvc/mediawiki/trunk/extensions/Maps/ParserFunctions/Maps_ParserFunctions.php?view=markup&pathrev=65051 CODE REVIEW! STOP MESSING MY LINK KTHX! Most of the body seems to lack one level of indentation.

#Comment by Jeroen De Dauw (talk | contribs)   12:03, 15 April 2010

Well, I sort of fixed it, still a line off though. Apparently some tabs got replaced by 4 spaces there somehow, which does not show in Eclipse.

Status & tagging log