r98951 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r98950‎ | r98951 | r98952 >
Date:23:39, 4 October 2011
Author:krinkle
Status:ok (Comments)
Tags:todo 
Comment:
[RL2] Pass error callback to getGadgetCategories
* The error callback was fired on prototypewiki (before r98950 at least). Eventhough that bug is fixed, it should still account of the error case. Otherwise mw.gadget.api will attempt to call undefined(). Error handling is mandatory apparently.
Modified paths:
  • /branches/RL2/extensions/Gadgets/README (modified) (history)
  • /branches/RL2/extensions/Gadgets/modules/ext.gadgets.gadgetmanager.js (modified) (history)

Diff [purge]

Index: branches/RL2/extensions/Gadgets/modules/ext.gadgets.gadgetmanager.js
——@@ -269,6 +269,14 @@
270270 }
271271 // getGadgetData not done yet, leave cats for it's callback to use
272272 cats = ret;
 273+ // Error callback. Fallback to empty array
 274+ }, function( errorCode ) {
 275+ if ( gadget ) {
 276+ // getGadgetData already done
 277+ return ga.ui.showFancyForm( gadget, [], mode );
 278+ }
 279+ // getGadgetData not done yet, leave cats for it's callback to use
 280+ cats = [];
273281 });
274282 },
275283
Index: branches/RL2/extensions/Gadgets/README
——@@ -37,5 +37,5 @@
3838 * Gadget modules extend ResourceLoaderWikiModule and thus qualify as having
3939 origin ORIGIN_USER_SITEWIDE. This means that gadgets are never loaded on (special) pages
4040 that call OutputPage::disallowUserJs(). For example Special:Preferences, Special:UserLogin
41 - Special:ResetPass and Special:Gadgets so users can always disable any broken gadgets
42 - they may have enabled, and malicious gadgets will be unable to steal passwords.
 41+ and Special:ResetPass so users can always disable any broken gadgets they may have enabled,
 42+ and malicious gadgets will be unable to steal passwords.

Follow-up revisions

RevisionCommit summaryAuthorDate
r100001[RL2] Disallow user JS on Special:Gadgets...krinkle22:41, 16 October 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r98950[RL2] getGadgetCategories: Allow array to be empty...krinkle23:37, 4 October 2011

Comments

#Comment by Catrope (talk | contribs)   12:48, 5 October 2011

Why does this edit README to remove Special:Gadgets from the list of special pages that gadgets are disabled on? Even if they're not, they should be, right? Also the "so users can always disable" phrase no longer makes sense.

Code is OK, marking fixme over README change.

#Comment by Krinkle (talk | contribs)   18:07, 5 October 2011

I added it in a previous revision because I thought it was the case. Here I removed it again because it's not the case.

Not sure whether or not to load it on SpecialGadgets. If concern is security (changing gadget properties), then that's not really the case as that script could do the same from any other page to the API.

If concern is ability to acces the ajax editor.. Any gadget can be disabled from the preferences, so a gadget can't make SpecialGadgets inaccessible even the gadget is enabled for everyone by default.

If you think we should disallow user scripts on SpecialGadgets, I can understand that too. I wouldn't remove it if it was there, but I didn't add it either. Just corrected the README file to reflect status quo.

Status & tagging log