r90191 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r90190‎ | r90191 | r90192 >
Date:00:55, 16 June 2011
Author:kaldari
Status:ok (Comments)
Tags:
Comment:
make it so that images always work regardless of whether they include the prefix or not
Modified paths:
  • /trunk/extensions/WikiLove/modules/ext.wikiLove/ext.wikiLove.core.js (modified) (history)
  • /trunk/extensions/WikiLove/modules/ext.wikiLove/ext.wikiLove.defaultOptions.js (modified) (history)

Diff [purge]

Index: trunk/extensions/WikiLove/modules/ext.wikiLove/ext.wikiLove.defaultOptions.js
@@ -1,7 +1,7 @@
22 ( function( $ ) {
33 $.wikiLove.optionsHook = function() { return {
44 defaultText: '{| style="background-color: $5; border: 1px solid $6;"\n\
5 -|rowspan="2" style="vertical-align: middle; padding: 5px;" | [[File:$3|$4]]\n\
 5+|rowspan="2" style="vertical-align: middle; padding: 5px;" | [[$3|$4]]\n\
66 |style="font-size: x-large; padding: 3px; height: 1.5em;" | \'\'\'$2\'\'\'\n\
77 |-\n\
88 |style="vertical-align: middle; padding: 3px;" | $1 ~~~~\n\
@@ -26,7 +26,7 @@
2727 header: 'A barnstar for you!', // header that appears at the top of the talk page post (optional)
2828 title: 'The Original Barnstar', // title that appears inside the award box (optional)
2929 image: 'Original Barnstar Hires.png', // image for the award
30 - email: 'Hello $7!\n\nI just awarded you a barnstar.' // message to use in eemail notification; $7 is replaced by the recipient's username
 30+ email: 'Hello $7!\n\nI just awarded you a barnstar.' // message to use in email notification; $7 is replaced by the recipient's username
3131 },
3232 'admins': {
3333 fields: [ 'notify' ],
Index: trunk/extensions/WikiLove/modules/ext.wikiLove/ext.wikiLove.core.js
@@ -261,8 +261,7 @@
262262 if ( $( '#mw-wikilove-image' ).val().length <= 0 ) {
263263 if( typeof currentTypeOrSubtype.gallery == 'object' ) {
264264 $.wikiLove.showError( 'wikilove-err-image' ); return false;
265 - }
266 - else {
 265+ } else {
267266 $( '#mw-wikilove-image' ).val( options.defaultImage );
268267 }
269268 }
@@ -319,7 +318,12 @@
320319
321320 msg = msg.replace( '$1', $( '#mw-wikilove-message' ).val() ); // replace the raw message
322321 msg = msg.replace( '$2', $( '#mw-wikilove-title' ).val() ); // replace the title
323 - msg = msg.replace( '$3', $( '#mw-wikilove-image' ).val() ); // replace the image
 322+ var imageName = $( '#mw-wikilove-image' ).val();
 323+ // if the image name doesn't start with a prefix, add one
 324+ if ( imageName.indexOf( 'File:' ) !== 0 && imageName.indexOf( 'Image:' ) !== 0 ) {
 325+ imageName = 'File:' + imageName;
 326+ }
 327+ msg = msg.replace( '$3', imageName ); // replace the image
324328 msg = msg.replace( '$4', currentTypeOrSubtype.imageSize || options.defaultImageSize ); // replace the image size
325329 msg = msg.replace( '$5', currentTypeOrSubtype.backgroundColor || options.defaultBackgroundColor ); // replace the background color
326330 msg = msg.replace( '$6', currentTypeOrSubtype.borderColor || options.defaultBorderColor ); // replace the border color
@@ -439,11 +443,16 @@
440444 var titles = '';
441445 var imageList = currentTypeOrSubtype.gallery.imageList.slice( 0 );
442446 for( var i=0; i<currentTypeOrSubtype.gallery.number; i++ ) {
443 - // get a randomimage
 447+ // get a random image from imageList and add it to the list of titles to be retrieved
444448 var id = Math.floor( Math.random() * imageList.length );
445 - titles = titles + 'File:' + imageList[id] + '|';
 449+ // make sure the image name has a prefix
 450+ if ( imageList[id].indexOf( 'File:' ) === 0 || imageList[id].indexOf( 'Image:' ) === 0 ) {
 451+ titles = titles + imageList[id] + '|';
 452+ } else {
 453+ titles = titles + 'File:' + imageList[id] + '|';
 454+ }
446455
447 - // remove the random page from the keys array
 456+ // remove the randomly selected image from imageList so that it can't be added twice
448457 imageList.splice(id, 1);
449458 }
450459

Follow-up revisions

RevisionCommit summaryAuthorDate
r90220include localized prefix per comments at r90191kaldari17:58, 16 June 2011

Comments

#Comment by MZMcBride (talk | contribs)   00:58, 16 June 2011

> if ( imageName.indexOf( 'File:' ) !== 0 && imageName.indexOf( 'Image:' ) !== 0 ) {

This doesn't look localization friendly.

#Comment by Kaldari (talk | contribs)   05:51, 16 June 2011

True (although it is a slight improvement on the previous behavior which didn't account for people including prefixes at all). I just need to figure out how to detect for localized file prefixes on the client-side.

#Comment by MZMcBride (talk | contribs)   06:00, 16 June 2011

Yeah, it's probably fine to put a FIXME in the code and/or file a bug to track the issue. Inconsistency between the behavior in English and in other languages can make other parts of the code (like localized input instructions) different, though, so it does need to be noted, in my opinion.

#Comment by Catrope (talk | contribs)   10:30, 16 June 2011

Like MZ says, this should pull the localized file ns names and check against those. I think Krinkle is working on a problem that needs something similar, so you could hit him up about how to get that data.

Fine otherwise, marking OK.

#Comment by Kaldari (talk | contribs)   17:59, 16 June 2011

Fixed in r90220.

Status & tagging log