User "Krinkle" changed the status of MediaWiki.r86047. Old Status: new New Status: fixme
User "Krinkle" also posted a comment on MediaWiki.r86047. Full URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/86047#c15958 Commit summary: History: Adding JS that converts buttons to links for better UX. Patch based on one by Matthew Flaschen. Fixes Bug 16165 Comment: Please don't create new global legacy variables, or anything new in legacy files for that matter. Neither fixCompare, compareLink or updateCompare is globally (<code>window.</code>) needed. They are not referenced outside of their current scope (ie. another file) Instead define them '''locally''' (<code>var</code>) in mediawiki.action.history.js <pre> var updateCompare = function(){ }; var compareLink = {}; var fixCompare = funciton(){ compareLink..... }; .. fixCompare(); </pre> or, if needed outside this scope: <pre> var compareLink = {}; mw.history = {}; mw.history.updateCompare = function(){ }; mw.history.fixCompare = funciton(){ compareLink..... }; .. mw.history.fixCompare(); </pre> The only globals that should be created/used in core are <code>mw</code> (<code>mediaWiki</code>) and <code>jQuery</code>. Everything else is either part of those objects or local, or aliased. mediawiki.action.history.js already has an alias from jQuery to $, so <code>$</code> can be used safely here. Also, there's no need for an additional anonymous function for the call, and the fullname global 'mediaWiki' (alias <code>mw</code> is globally available). <pre> + mediaWiki.loader.using('jquery.ui.button', function() { + window.fixCompare(); + }); </pre> Functions are objects and are always passed by reference, the following is more effecient <pre> + mw.loader.using('jquery.ui.button', window.fixCompare ); </pre> This is creating an additional http request for all history pages though, should be added as a dependancy on the server side instead. _______________________________________________ MediaWiki-CodeReview mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
