"Santhosh.thottingal" posted a comment on MediaWiki.r106706.
URL: http://www.mediawiki.org/wiki/Special:Code/MediaWiki/106706#c28182
Commit summary for MediaWiki.r106706:
[WebFonts] Code quality & clean up
* Various good practices, white space conventions (Thanks JSLint)
* Such as:
-- JavaScript does not have block scope! Removed var statements from loops, and
put them outside the loop for clarity
-- Optimize a few loops (swapped a 'for in' loop for a 'for i < length' loop,
since it was an array, and improved the existing one by caching the length
property since otherwise the length property would be accessed every iteration
- also prevents infinite loops in some situations)
-- Combining var statements. Ideally every function only needs 1 var statement
(since there is only function scope and everything is hoisted internally
anyway), but no time to go through and restructure them all right now, did most
of them.
-- Improve function documentation ('@param config The webfont configuration.'
-> '@param fonts {Array} List of fonts to be provided as a menu option.')
-- Removed unused variables (such as 'languages' in
loadFontsForFontFamilyStyle, added in r100677)
-- Using strict comparison to false values (match(..) === null), both for speed
gain and improve readability as loose comparison is ambiguous in JavaScript
(val == null also returns true for undefined instead of null)
-- Using direct property access and strict comparison instead of an ' in '
lookup
-- Rename a variable called 'config' to 'fonts' (array) to avoid confusion with
another variable that is a plain object also called 'config'
-- Make 'mw' a local variable by passing it through the IIFE closure from the
global scope (for faster lookups in the context object)
-- Using the "||" operator in a variable assignment as "var = foo || bar"
instead of "var = foo ? foo : bar". In JavaScript comparison operators do not
return boolean, instead they return the last compared value. (which in an
if-statement is then type-coerced to a boolean)
-- Using mw.util.addCSS instead of creating a new <style> element locally.
-- Trailing whitespace
-- Redundant spaces (e.g. double space where single space was intended)
Santhosh.thottingal's comment:
Hi, I tested this in IE7 and IE8. Both were crashing with mw.utll.addCSS.
During the code review, you had suggested to use mw.util.addCSS, and I had
replied there saying that it is crashing IE.
In the stackoverlfow thread, please see the code snippet in answer 1. Direct
link http://stackoverflow.com/a/7952904/337907
_______________________________________________
MediaWiki-CodeReview mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-codereview
