MediaWiki r86961 - Code Review

Jump to: navigation, search
Repository:MediaWiki
Revision:< r86960‎ | r86961 (on ViewVC)‎ | r86962 >
Date:18:10, 26 April 2011
Author:nimishg
Status:reverted (Comments)
Tags:
Comment:
cross-browser fun
Modified paths:

Diff [purge]

Index: trunk/extensions/ClickTracking/modules/ext.UserBuckets.js
——@@ -86,7 +86,7 @@
8787
8888 // do the actual code in the campaign based on the bucket
8989 if($.getBuckets() && $.getBuckets()[campaign.name] && $.getBuckets()[campaign.name][0] != "none"){
90 - if(typeof campaign[$.getBuckets()[campaign.name][0]] == "function"){
 90+ if(typeof(campaign[$.getBuckets()[campaign.name][0]]) == "function"){
9191 campaign[$.getBuckets()[campaign.name][0]](); //function to execute
9292 }
9393 if(campaign.allActive){
——@@ -100,7 +100,7 @@
101101
102102 //no need to do any of this if there are no active campaigns
103103 if( (typeof(MW) != "undefined") && MW.activeCampaigns){
104 - $( document ).ready( jQuery.setupActiveBuckets );
 104+ $j( document ).ready( jQuery.setupActiveBuckets );
105105 }
106106
107107

Follow-up revisions

Rev.Commit summaryAuthorDate
r86976* Replace MW (inexistant) with mw (global alias to the mediaWiki object)...krinkle19:35, 26 April 2011
r87186Follow-up r86961: Code conventions; line-wrap around 80-100 chars; using isFu...krinkle13:33, 1 May 2011

Past revisions this follows-up on

Rev.Commit summaryAuthorDate
r85049Simplified buckets, created a possible framework for user bucket campaignsnimishg21:40, 30 March 2011

Comments

#Comment by Krinkle (talk | contribs)   19:38, 26 April 2011

Follow-up to r85049.

#Comment by Krinkle (talk | contribs)   19:41, 26 April 2011

Can you please describe what cross-browser funkiness is going on here ?

  • I seems odd that one needs to use $j instead of $ (no other major scripts use $j, so if that would fix something, please highlight the issue as this is not an acceptable fix).
  • typeof is not a function but a keyword. It can be used as a function, but that shouldn't make a difference. In what browsers was this causing problems ?
#Comment by Nimish Gautam (talk | contribs)   17:20, 27 April 2011

When I was testing on IE6 and had $, it wasn't loading any of the JS. I tried $j and it worked fine, and this change didn't seem to cause any regressions. the typeof as a function was just stylistic.

#Comment by Krinkle (talk | contribs)   13:25, 1 May 2011

Where did you test ? What did you test ?

In the development version of MediaWiki (eg. what we're working on in SVN) $ and $j are exactly the same, if they're then not something is wrong in your script.

However the version currently live on Wikimedia (before 1.17) didn't have $, so it's very likely it didn't work there, however without testing on the SVN version you should't be making modifications based on what is live on Wikimedia (one day this won't be an issue, but right now it is)

Can you test again (if not already) on a clean install of svn-trunk-phase3 (ie. a simple local LAMP/XAMP webserver)

Status & tagging log

  • 13:34, 1 May 2011 Krinkle (talk | contribs) changed the status of r86961 [removed: new added: reverted]
Retrieved from "http://www.mediawiki.org/wiki/Special:Code/MediaWiki/86961"