MediaWiki r61700 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r61699‎ | r61700 (on ViewVC)‎ | r61701 >
Date:23:57, 29 January 2010
Author:catrope
Status:resolved (Comments)
Tags:
Comment:
Patch jQuery to test for 'nullpx' as well as 'NaNpx'
Modified paths:

Diff [purge]

Index: trunk/phase3/skins/common/jquery.js
——@@ -1059,7 +1059,7 @@
10601060 return letter.toUpperCase();
10611061 });
10621062
1063 - if ( set && value != 'NaNpx' ) // Patched by Trevor, see http://is.gd/5NXiD
 1063+ if ( set && value != 'NaNpx' && value != 'nullpx' ) // Patched by Trevor, see http://is.gd/5NXiD
10641064 elem[ name ] = value;
10651065
10661066 return elem[ name ];
Index: trunk/phase3/skins/common/jquery.min.js
——@@ -121,7 +121,7 @@
122122 if(!jQuery.support.opacity&&name=="opacity"){if(set){elem.zoom=1;elem.filter=(elem.filter||"").replace(/alpha\([^)]*\)/,"")+
123123 (parseInt(value)+''=="NaN"?"":"alpha(opacity="+value*100+")");}
124124 return elem.filter&&elem.filter.indexOf("opacity=")>=0?(parseFloat(elem.filter.match(/opacity=([^)]*)/)[1])/100)+'':"";}
125 -name=name.replace(/-([a-z])/ig,function(all,letter){return letter.toUpperCase();});if(set&&value!='NaNpx')
 125+name=name.replace(/-([a-z])/ig,function(all,letter){return letter.toUpperCase();});if(set&&value!='NaNpx'&&value!='nullpx')
126126 elem[name]=value;return elem[name];},trim:function(text){return(text||"").replace(/^\s+|\s+$/g,"");},makeArray:function(array){var ret=[];if(array!=null){var i=array.length;if(i==null||typeof array==="string"||jQuery.isFunction(array)||array.setInterval)
127127 ret[0]=array;else
128128 while(i)

Follow-up revisions

Rev.Commit summaryAuthorDate
r61711Bump $wgStyleVersion for r61700catrope12:07, 30 January 2010
r61872wmf-deployment: Merge r61700 (jQuery patch) from trunkcatrope20:44, 2 February 2010

Comments

#Comment by Tim Starling (talk | contribs)   02:29, 15 February 2010

I suppose this is http://dev.jquery.com/ticket/5081 , closed invalid. Can someone please either find a solution which the jQuery devs are willing to commit, or document this patch and any other patches so that we have some hope of doing an update from upstream without a regression?

#Comment by Catrope (talk | contribs)   10:04, 15 February 2010

Yes, this fix is not ideal in that it's the caller's responsibility to check it's passing valid values. However, when you've been hunting IE bugs all day and just want something to work, catching this in the callee is much easier than tracking down the callers passing invalid values (I think null is our fault, and NaN is jQuery UI's fault).

#Comment by ^demon (talk | contribs)   01:48, 28 September 2010

I can't find references to NaNpx or nullpx anywhere in trunk anymore. Assuming this was resolved in some refactor?

#Comment by Catrope (talk | contribs)   11:55, 28 September 2010

We updated jQuery to version 1.4, that'll have done it.

#Comment by ^demon (talk | contribs)   08:56, 13 October 2010

Marking resolved per r74326 and upstream fix.

Status & tagging log

  • 08:56, 13 October 2010 ^demon (talk | contribs) changed the status of r61700 [removed: fixme added: resolved]
  • 02:29, 15 February 2010 Tim Starling (talk | contribs) changed the status of r61700 [removed: new added: fixme]
Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/61700"