r106706 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r106705‎ | r106706 | r106707 >
Date:22:02, 19 December 2011
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
[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)
Modified paths:
  • /trunk/extensions/WebFonts/resources/ext.webfonts.fontlist.js (modified) (history)
  • /trunk/extensions/WebFonts/resources/ext.webfonts.js (modified) (history)

Diff [purge]

Index: trunk/extensions/WebFonts/resources/ext.webfonts.js
@@ -1,4 +1,4 @@
2 -( function( $ ) {
 2+( function( $, mw, undefined ) {
33
44 function fontID( font ) {
55 if ( typeof font !== 'string' ) {
@@ -19,28 +19,30 @@
2020 return;
2121 }
2222
23 - if ( !( font in mw.webfonts.config.fonts ) ) {
 23+ if ( mw.webfonts.config.fonts[font] === undefined ) {
2424 mw.log( 'Requested unknown font', font );
2525 return;
2626 }
27 - var config = mw.webfonts.config.fonts[font];
 27+ var config = mw.webfonts.config.fonts[font],
 28+ $body = $( 'body' );
2829
2930 // Load the style sheet for the font
3031 mw.webfonts.addFont( font );
31 -
 32+
3233 // Save the current font and its size. Used for reset.
3334 if ( !mw.webfonts.oldconfig ) {
34 - var $body = $( 'body' );
 35+
3536 mw.webfonts.oldconfig = {
3637 fontFamily: $body.css( 'font-family' ),
3738 fontSize: $body.css( 'font-size' )
3839 };
3940 }
4041
41 - // Set the font, fallback fonts.Need to change the fonts of Input Select and Textarea explicitly.
 42+ // Set the font, fallback fonts.
 43+ // Need to change the fonts of Input Select and Textarea explicitly.
4244 $( 'body, input, select, textarea' ).css( 'font-family', '"' + font + '", Helvetica, Arial, sans-serif' );
4345
44 - if ( 'normalization' in config ) {
 46+ if ( config.normalization !== undefined ) {
4547 $( document ).ready( function() {
4648 mw.webfonts.normalize( config.normalization );
4749 } );
@@ -79,7 +81,7 @@
8082 var searchPattern = new RegExp( search, 'g' );
8183 return $( '*' ).each( function() {
8284 var node = this.firstChild,
83 - val, newVal;
 85+ val, newVal;
8486 if ( node ) {
8587 do {
8688 if ( node.nodeType === 3 ) {
@@ -101,50 +103,45 @@
102104 * @param fontFamily The font-family name
103105 */
104106 loadCSS: function( fontFamily ) {
105 - var fontconfig = mw.webfonts.config.fonts[fontFamily];
106 - var base = mw.config.get( 'wgExtensionAssetsPath' ) + '/WebFonts/fonts/';
107 - var fontFormats = [];
108 - var version = "0.0";
109 - if ( 'version' in fontconfig ) {
110 - version = fontconfig.version;
111 - }
112 - var versionSuffix = "?version=" + version + '&20111213';
113 - var styleString =
114 - "<style type='text/css'>\n@font-face {\n"
115 - + "\tfont-family: '"+fontFamily+"';\n";
 107+ var fontconfig = mw.webfonts.config.fonts[fontFamily],
 108+ base = mw.config.get( 'wgExtensionAssetsPath' ) + '/WebFonts/fonts/',
 109+ fontFormats = [],
 110+ version = fontconfig.version || "0.0",
 111+ versionSuffix = "?version=" + version + '&20111213',
 112+ styleString = "@font-face { font-family: '"+fontFamily+"';\n",
 113+ ua = navigator.userAgent;
116114
117 - if ( 'eot' in fontconfig ) {
118 - styleString += "\tsrc: url('" + base+ fontconfig.eot + versionSuffix+ "');\n";
 115+ if ( fontconfig.eot !== undefined ) {
 116+ styleString += "\tsrc: url('" + base + fontconfig.eot + versionSuffix + "');\n";
119117 }
120118
121119 styleString += "\tsrc: ";
 120+
122121 // If the font is present locally, use it.
123 - var ua = navigator.userAgent;
124 - if( ua.match( /Android 2\.3/ ) == null ) {
125 - // Android 2.3.x does not respect local() syntax.
 122+ if( ua.match( /Android 2\.3/ ) === null ) {
 123+ // Android 2.3.x does not respect local() syntax.
126124 // http://code.google.com/p/android/issues/detail?id=10609
127 - styleString += "local('"+fontFamily+"'),";
 125+ styleString += "local('" + fontFamily + "'),";
128126 }
129 -
130 - if ( 'woff' in fontconfig ) {
 127+
 128+ if ( fontconfig.woff !== undefined ) {
131129 fontFormats.push( "\t\turl('" + base + fontconfig.woff + versionSuffix + "') format('woff')" );
132130 }
133131
134 - if ( 'svg' in fontconfig ) {
 132+ if ( fontconfig.svg !== undefined ) {
135133 fontFormats.push( "\t\turl('" + base + fontconfig.svg + versionSuffix + "#" + fontFamily + "') format('svg')" );
136134 }
137135
138 - if ( 'ttf' in fontconfig ) {
 136+ if ( fontconfig.ttf !== undefined ) {
139137 fontFormats.push( "\t\turl('" + base + fontconfig.ttf + versionSuffix + "') format('truetype')" );
140138 }
141 -
 139+
142140 styleString += fontFormats.join() + ";\n";
143 - styleString += "\tfont-weight: normal;\n}\n</style>\n";
 141+ styleString += "\tfont-weight: normal;}";
144142
145 - //inject the css to the head of the page.
146 - $( styleString ).appendTo( 'head' );
 143+ mw.util.addCSS( styleString );
147144 },
148 -
 145+
149146 /**
150147 * Add a font to the page.
151148 * This method ensures that css are not duplicated and
@@ -155,13 +152,13 @@
156153 // Avoid duplicate loading
157154 if ( $.inArray( fontFamilyName, mw.webfonts.fonts ) === -1 ) {
158155 // Check whether the requested font is available.
159 - if ( fontFamilyName in mw.webfonts.config.fonts ) {
 156+ if ( mw.webfonts.config.fonts[fontFamilyName] !== undefined ) {
160157 mw.webfonts.loadCSS( fontFamilyName );
161158 mw.webfonts.fonts.push( fontFamilyName );
162159 }
163160 }
164161 },
165 -
 162+
166163 /**
167164 * Setup the font selection menu.
168165 * It also apply the font from cookie, if any.
@@ -178,52 +175,53 @@
179176 return;
180177 }
181178 }
182 -
183 - var config = [];
184 - var languages = mw.webfonts.config.languages;
185 - var requested = [mw.config.get( 'wgUserVariant' ), mw.config.get( 'wgContentLanguage' ), mw.config.get( 'wgUserLanguage' )];
186179
187 - for ( var i = 0; i < requested.length; i++ ) {
188 - if ( requested[i] in languages ) {
189 - var fonts = languages[requested[i]];
190 - for ( var j = 0; j < fonts.length; j++ ) {
191 - if ( $.inArray( fonts[j], config ) === -1 ) {
192 - config.push( fonts[j] );
 180+ var fonts = [],
 181+ languages = mw.webfonts.config.languages,
 182+ requested = [mw.config.get( 'wgUserVariant' ), mw.config.get( 'wgContentLanguage' ), mw.config.get( 'wgUserLanguage' )],
 183+ i, j;
 184+
 185+ for ( i = 0; i < requested.length; i++ ) {
 186+ if ( languages[requested[i]] !== undefined ) {
 187+ fonts = languages[requested[i]];
 188+ for ( j = 0; j < fonts.length; j++ ) {
 189+ if ( $.inArray( fonts[j], fonts ) === -1 ) {
 190+ fonts.push( fonts[j] );
193191 }
194192 }
195193 }
196194 }
197195
198196 // Build font dropdown
199 - mw.webfonts.buildMenu( config );
 197+ mw.webfonts.buildMenu( fonts );
200198 // See if there is a font in cookie if not first font is default font.
201 - var cookieFont = $.cookie( 'webfonts-font' );
202 - var selectedFont = null;
 199+ var cookieFont = $.cookie( 'webfonts-font' ),
 200+ selectedFont = null;
203201 // check whether this font is for the current userlang/contentlang
204 - if ( $.inArray( cookieFont, config ) !== -1 || cookieFont === 'none' ) {
 202+ if ( $.inArray( cookieFont, fonts ) !== -1 || cookieFont === 'none' ) {
205203 selectedFont = cookieFont;
206204 }
207 - else{
208 - // We cannot use cookie font since it is not one of the fonts suitable
 205+ else {
 206+ // We cannot use cookie font since it is not one of the fonts suitable
209207 // for current language.
210 - selectedFont = config[0];
 208+ selectedFont = fonts[0];
211209 }
212210 if ( selectedFont ) {
213211 mw.webfonts.set( selectedFont );
214212 // Mark it as checked
215213 $( '#'+fontID( selectedFont ) ).prop( 'checked', true );
216214 }
217 -
 215+
218216 mw.webfonts.loadFontsForFontFamilyStyle();
219217 mw.webfonts.loadFontsForLangAttr();
220218
221219 if ( $( '.webfonts-lang-attr' ).length && !$( '#webfonts-fontsmenu' ).length ) {
222 - // We need to show the reset option even if there is no font to show
 220+ // We need to show the reset option even if there is no font to show
223221 // for the language, if there is lang attr based font embedding.
224 - mw.webfonts.buildMenu( config );
 222+ mw.webfonts.buildMenu( fonts );
225223 }
226224 },
227 -
 225+
228226 /**
229227 * Scan the page for tags with lang attr and load the default font
230228 * for that language if available.
@@ -232,32 +230,31 @@
233231 var languages = mw.webfonts.config.languages;
234232 var requested = [mw.config.get( 'wgUserVariant' ), mw.config.get( 'wgContentLanguage' ), mw.config.get( 'wgUserLanguage' )];
235233 var fontFamily = false;
236 - // If there are tags with lang attribute,
237 - $( 'body' ).find( '*[lang]' ).each( function( index ) {
 234+ // If there are tags with lang attribute,
 235+ $( 'body' ).find( '*[lang]' ).each( function( i, el) {
238236 // If the lang attribute value is same as one of
239237 // contentLang,useLang, variant, no need to do this.
240 - if( $.inArray( this.lang , requested ) === -1 ) {
 238+ if( $.inArray( el.lang , requested ) === -1 ) {
241239 // check the availability of font, add a font-family style if it does not have any
242 - if( languages[this.lang] && ( !this.style.fontFamily || this.style.fontFamily === 'none' ) ) {
243 - fontFamily = languages[this.lang][0];
 240+ if( languages[el.lang] && ( !el.style.fontFamily || el.style.fontFamily === 'none' ) ) {
 241+ fontFamily = languages[el.lang][0];
244242 mw.webfonts.addFont( fontFamily );
245 - $(this).css( 'font-family', fontFamily ).addClass( 'webfonts-lang-attr' );
 243+ $(el).css( 'font-family', fontFamily ).addClass( 'webfonts-lang-attr' );
246244 }
247245 }
248246 });
249247 },
250 -
 248+
251249 /**
252250 * Scan the page for tags with font-family style declarations
253251 * If that font is available, embed it.
254252 */
255253 loadFontsForFontFamilyStyle: function() {
256 - var languages = mw.webfonts.config.languages;
257254 // If there are tags with font-family style definition, get a list of fonts to be loaded
258 - $( 'body' ).find( '*[style]' ).each( function( index ) {
259 - if( this.style.fontFamily ) {
260 - var fontFamilyItems = this.style.fontFamily.split( ',' );
261 - $.each( fontFamilyItems, function( index, fontFamily ) {
 255+ $( 'body' ).find( '*[style]' ).each( function( i, el ) {
 256+ if( el.style.fontFamily ) {
 257+ var fontFamilyItems = el.style.fontFamily.split( ',' );
 258+ $.each( fontFamilyItems, function( i, fontFamily ) {
262259 // Remove the ' characters if any.
263260 fontFamily = fontFamily.replace( /'/g, '' );
264261 mw.webfonts.addFont( fontFamily );
@@ -266,79 +263,81 @@
267264 });
268265
269266 },
270 -
 267+
271268 /**
272269 * Prepare the div containing menu items.
273 - * @param config The webfont configuration.
 270+ * @param fonts {Array} List of fonts to be provided as a menu option.
274271 */
275 - buildMenuItems: function ( config ){
276 - var haveSchemes = false;
277 - // Build font dropdown
278 - var $fontsMenu = $( '<ul>' ).attr( 'id', 'webfonts-fontsmenu' );
279 - $fontsMenu.delegate( 'input:radio', 'click', function( ) {
280 - mw.webfonts.set( $(this).val() );
281 - } );
282 - for ( var scheme in config ) {
283 - if ( !Object.prototype.hasOwnProperty.call( config, scheme ) ) {
284 - continue;
285 - }
 272+ buildMenuItems: function ( fonts ){
 273+ var haveSchemes = false,
 274+ $fontsMenu = $( '<ul>' )
 275+ .attr( 'id', 'webfonts-fontsmenu' )
 276+ .delegate( 'input:radio', 'click', function() {
 277+ mw.webfonts.set( $(this).val() );
 278+ } ),
 279+ len = fonts.length,
 280+ i, font, $link, $label, $item;
286281
287 - var $fontLink = $( '<input type="radio" name="font" />' )
288 - .attr( 'id', fontID( config[scheme] ) )
289 - .val( config[scheme] );
 282+ for ( i = 0; i < len; i++ ) {
 283+ font = fonts[i];
290284
291 - var $fontLabel = $( '<label>' )
292 - .attr( 'for',fontID(config[scheme] ) )
293 - .append( $fontLink )
294 - .append( config[scheme] );
 285+ $link = $( '<input type="radio" name="font" />' )
 286+ .attr( 'id', fontID( font ) )
 287+ .val( font );
295288
296 - var $fontMenuItem = $( '<li>' )
297 - .val( config[scheme] )
298 - .append( $fontLabel );
 289+ $label = $( '<label>' )
 290+ .attr( 'for',fontID( font ) )
 291+ .append( $link )
 292+ .append( font );
299293
 294+ $item = $( '<li>' )
 295+ .val( font )
 296+ .append( $label );
 297+
300298 haveSchemes = true;
301299
302 - $fontsMenu.append( $fontMenuItem );
 300+ $fontsMenu.append( $item );
303301
304302 }
305303
306 - if ( !haveSchemes && !$( '.webfonts-lang-attr' ).length ) {
 304+ $link = $label = $item = undefined;
 305+
 306+ if ( !haveSchemes && !$( '.webfonts-lang-attr' ).length ) {
307307 // No schemes available, and no tags with lang attr
308308 // with fonts loaded. Don't show the menu.
309309 return null;
310310 }
311311
312 - var $resetLink = $( '<input type="radio" name="font" />' )
 312+ $link = $( '<input type="radio" name="font" />' )
313313 .attr( 'value', 'webfont-none' )
314314 .attr( 'id', 'webfont-none' )
315 - .click( function( e ) {
 315+ .click( function() {
316316 mw.webfonts.set( 'none' );
317317 });
318318
319 - var $resetLabel = $( '<label>' )
 319+ $label = $( '<label>' )
320320 .attr( 'for', 'webfont-none' )
321 - .append( $resetLink )
 321+ .append( $link )
322322 .append( mw.message( 'webfonts-reset' ).escaped() );
323323
324 - var $resetLinkItem = $( '<li>' )
 324+ $item = $( '<li>' )
325325 .val( 'none' )
326 - .append( $resetLabel );
 326+ .append( $label );
327327
328 - $fontsMenu.append( $resetLinkItem );
 328+ $fontsMenu.append( $item );
329329
330 - var $menuDiv = $( '<div>' )
 330+ return $( '<div>' )
331331 .attr( 'id', 'webfonts-fonts' )
332332 .addClass( 'menu' )
333 - .append( $fontsMenu )
334 - .append();
335 - return $menuDiv;
 333+ .append( $fontsMenu );
336334 },
 335+
337336 /**
338337 * Prepare the menu for the webfonts.
339 - * @param config The webfont configuration.
 338+ * @param fonts {Array} List of fonts to be provided as a menu option.
340339 */
341 - buildMenu: function(config) {
342 - var $menuItemsDiv = mw.webfonts.buildMenuItems( config );
 340+ buildMenu: function( fonts ) {
 341+ var $menuItemsDiv = mw.webfonts.buildMenuItems( fonts );
343342 if( $menuItemsDiv === null ) {
344343 return;
345344 }
@@ -363,7 +362,7 @@
364363 $( 'body' ).prepend( $menu );
365364 $li.click( function( event ) {
366365 var menuSide, menuOffset, distanceToEdge;
367 -
 366+
368367 if ( rtlEnv ) {
369368 distanceToEdge = $li.outerWidth() + $li.offset().left;
370369 if ( $menuItemsDiv.outerWidth() > distanceToEdge ) {
@@ -394,22 +393,25 @@
395394 event.stopPropagation();
396395 }
397396 } );
 397+
398398 $( 'html' ).click( function() {
399399 $menu.removeClass( 'open' );
400400 } );
 401+
401402 $menu.click( function( event ) {
402403 event.stopPropagation();
403404 } );
 405+
404406 // Workaround for IE bug - ActiveX components like input fields coming on top of everything.
405407 // @todo Is there a better solution other than hiding it on hover?
406 - if ( $.browser.msie ) {
 408+ if ( $.browser.msie ) {
407409 $( '#webfonts-menu' ).hover( function() {
408410 $( '#searchform' ).css({ visibility: 'hidden' } );
409 - }, function( ) {
 411+ }, function() {
410412 $( '#searchform' ).css( { visibility: 'visible' } );
411413 } );
412414 }
413415 }
414416 };
415417
416 -} ) ( jQuery );
 418+})( jQuery, mediaWiki );
\ No newline at end of file
Index: trunk/extensions/WebFonts/resources/ext.webfonts.fontlist.js
@@ -41,7 +41,7 @@
4242 woff: "Deva/SamyakDevanagari.woff",
4343 version: "1.0"
4444 },
45 -
 45+
4646 "Madan": {
4747 eot: "Deva/madan.eot",
4848 ttf: "Deva/madan.ttf",
@@ -89,7 +89,7 @@
9090 eot: "Khmr/KhmerOS.eot",
9191 ttf: "Khmr/KhmerOS.ttf",
9292 woff: "Khmr/KhmerOS.woff",
93 - svg: "Khmr/KhmerOS.svg",
 93+ svg: "Khmr/KhmerOS.svg",
9494 version: "1.10"
9595 },
9696
@@ -97,7 +97,7 @@
9898 eot: "Khmr/KhmerOSbattambang.eot",
9999 ttf: "Khmr/KhmerOSbattambang.ttf",
100100 woff: "Khmr/KhmerOSbattambang.woff",
101 - svg: "Khmr/KhmerOSbattambang.svg",
 101+ svg: "Khmr/KhmerOSbattambang.svg",
102102 version: "1.10"
103103 },
104104
@@ -105,7 +105,7 @@
106106 eot: "Khmr/KhmerOSbokor.eot",
107107 ttf: "Khmr/KhmerOSbokor.ttf",
108108 woff: "Khmr/KhmerOSbokor.woff",
109 - svg: "Khmr/KhmerOSbokor.svg",
 109+ svg: "Khmr/KhmerOSbokor.svg",
110110 version: "1.10"
111111 },
112112
@@ -113,7 +113,7 @@
114114 eot: "Khmr/KhmerOSfreehand.eot",
115115 ttf: "Khmr/KhmerOSfreehand.ttf",
116116 woff: "Khmr/KhmerOSfreehand.woff",
117 - svg: "Khmr/KhmerOSfreehand.svg",
 117+ svg: "Khmr/KhmerOSfreehand.svg",
118118 version: "1.10"
119119 },
120120
@@ -121,7 +121,7 @@
122122 eot: "Khmr/KhmerOSfasthand.eot",
123123 ttf: "Khmr/KhmerOSfasthand.ttf",
124124 woff: "Khmr/KhmerOSfasthand.woff",
125 - svg: "Khmr/KhmerOSfasthand.svg",
 125+ svg: "Khmr/KhmerOSfasthand.svg",
126126 version: "1.10"
127127 },
128128
@@ -129,7 +129,7 @@
130130 eot: "Khmr/KhmerOSmuol.eot",
131131 ttf: "Khmr/KhmerOSmuol.ttf",
132132 woff: "Khmr/KhmerOSmuol.woff",
133 - svg: "Khmr/KhmerOSmuol.svg",
 133+ svg: "Khmr/KhmerOSmuol.svg",
134134 version: "1.10"
135135 },
136136
@@ -137,7 +137,7 @@
138138 eot: "Khmr/KhmerOSmuollight.eot",
139139 ttf: "Khmr/KhmerOSmuollight.ttf",
140140 woff: "Khmr/KhmerOSmuollight.woff",
141 - svg: "Khmr/KhmerOSmuollight.svg",
 141+ svg: "Khmr/KhmerOSmuollight.svg",
142142 version: "1.10"
143143 },
144144
@@ -145,7 +145,7 @@
146146 eot: "Khmr/KhmerOSmuolpali.eot",
147147 ttf: "Khmr/KhmerOSmuolpali.ttf",
148148 woff: "Khmr/KhmerOSmuolpali.woff",
149 - svg: "Khmr/KhmerOSmuolpali.svg",
 149+ svg: "Khmr/KhmerOSmuolpali.svg",
150150 version: "1.10"
151151 },
152152
@@ -153,7 +153,7 @@
154154 eot: "Khmr/KhmerOSsiemreap.eot",
155155 ttf: "Khmr/KhmerOSsiemreap.ttf",
156156 woff: "Khmr/KhmerOSsiemreap.woff",
157 - svg: "Khmr/KhmerOSsiemreap.svg",
 157+ svg: "Khmr/KhmerOSsiemreap.svg",
158158 version: "1.10"
159159 },
160160
@@ -241,7 +241,7 @@
242242 eot: "Mymr/MasterpieceUniSans.eot",
243243 ttf: "Mymr/MasterpieceUniSans.ttf",
244244 woff: "Mymr/MasterpieceUniSans.woff",
245 - svg: "Mymr/MasterpieceUniSans.svg",
 245+ svg: "Mymr/MasterpieceUniSans.svg",
246246 version: "0.5"
247247 },
248248
@@ -249,7 +249,7 @@
250250 eot: "Mymr/Myanmar3.eot",
251251 ttf: "Mymr/Myanmar3.ttf",
252252 woff: "Mymr/Myanmar3.woff",
253 - svg: "Mymr/Myanmar3.svg",
 253+ svg: "Mymr/Myanmar3.svg",
254254 version: "3.0"
255255 },
256256
@@ -257,7 +257,7 @@
258258 eot: "Mymr/Padauk-Regular.eot",
259259 ttf: "Mymr/Padauk-Regular.ttf",
260260 woff: "Mymr/Padauk-Regular.woff",
261 - svg: "Mymr/Padauk-Regular.svg",
 261+ svg: "Mymr/Padauk-Regular.svg",
262262 version: "2.8"
263263 },
264264
@@ -265,7 +265,7 @@
266266 eot: "Mymr/Yunghkio.eot",
267267 ttf: "Mymr/Yunghkio.ttf",
268268 woff: "Mymr/Yunghkio.woff",
269 - svg: "Mymr/Yunghkio.svg",
 269+ svg: "Mymr/Yunghkio.svg",
270270 version: "1.0"
271271 },
272272
@@ -297,14 +297,14 @@
298298 version: "2.5.0"
299299 },
300300
301 - Thendral: {
 301+ Thendral: {
302302 eot: "Taml/ThendralUni.eot",
303303 ttf: "Taml/ThendralUni.ttf",
304304 woff: "Taml/ThendralUni.woff",
305305 version: "1.0"
306306 },
307307
308 - Thenee: {
 308+ Thenee: {
309309 eot: "Taml/TheneeUni.eot",
310310 ttf: "Taml/TheneeUni.ttf",
311311 woff: "Taml/TheneeUni.woff",
@@ -353,7 +353,7 @@
354354 am: [ "AbyssinicaSIL" ],
355355 as: [ "Lohit Assamese" ],
356356 bh: [ "Lohit Devanagari" ],
357 - bho: [ "Lohit Devanagari" ],
 357+ bho: [ "Lohit Devanagari" ],
358358 bn: [ "Lohit Bengali" ],
359359 bpy: [ "Lohit Bengali" ],
360360 cdo: [ "Charis SIL" ],
@@ -372,7 +372,7 @@
373373 ml: [ "AnjaliOldLipi" ],
374374 mr: [ "Lohit Devanagari" ],
375375 my: [ "Masterpiece Uni Sans", "Padauk-Regular", "Myanmar3", "Yunghkio" ],
376 - ne: [ "Lohit Devanagari", "Madan" ],
 376+ ne: [ "Lohit Devanagari", "Madan" ],
377377 or: [ "Lohit Oriya" , "Utkal" ],
378378 pa: [ "Lohit Punjabi", "Saab" ],
379379 sa: [ "Lohit Devanagari" ],
@@ -383,5 +383,7 @@
384384 ti: [ "AbyssinicaSIL" ]
385385 }
386386 };
387 - $.extend( mw.webfonts.config, config);
 387+
 388+ $.extend( mw.webfonts.config, config );
 389+
388390 } ) ( jQuery );

Follow-up revisions

RevisionCommit summaryAuthorDate
r106772mw.util.addCSS will make IE version <= 8 crash if the css is a fontface defin...santhosh09:07, 20 December 2011
r106992[Core JS] mw.util.addCSS: Insert style tag into dom before setting cssText...krinkle22:08, 21 December 2011
r107378I18ndeploy...nikerabbit13:32, 27 December 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r100677Refactor the setup code with seperate mathods for loading fonts for lang attr...santhosh06:23, 25 October 2011

Comments

#Comment by Santhosh.thottingal (talk | contribs)   04:21, 20 December 2011

Thanks!.

IE crashes if mw.uti.addCSS is used. Generally it works, except for css with @font-face, More: http://stackoverflow.com/questions/7931658/dynamically-adding-font-face-rule-in-ie-8-and-less

#Comment by Krinkle (talk | contribs)   17:47, 20 December 2011

Interesting, I didn't know that. But that StackOverflow thread is about a bug in the addrule method of Internet Explorer's styleSheet object representing a stylesheet that is currently loaded in the document.

mw.util.addCSS doesn't use this method due and isn't modifying existing stylesheets. It adds new stylesheets exactly the way that StackOverflow thread suggests.

Although I didn't know about this particular bug in IE, it was due to the cross-browser differences in this object that I choose to insert new stylesheets (which is slower) instead of touching existing ones.

mw.util.addCSS can be used here perfectly fine as all it does is exactly what you want. It inserts an extra ‎<style> element. See also the source of mw.util.addCSS and documentation.

I'm not going to revert anything as I'm keeping the possibility open something may be broken in mw.util</cde> indeed. Did you verify that using mw.util.addCSS does not work here for IE ?

#Comment by Santhosh.thottingal (talk | contribs)   04:12, 21 December 2011

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

#Comment by Krinkle (talk | contribs)   17:48, 20 December 2011

#Comment by Siebrand (talk | contribs)   17:50, 20 December 2011

Please try again. IRC said this was "</code><code></code></code>"...

#Comment by Krinkle (talk | contribs)   21:50, 21 December 2011

@Santhosh: Thanks, so the bug isn't just when using the styleSheet.addrule method, it also occurs when using cssText. I didn't read the solution thoroughly enough. I'll fix the addCSS function to set t he cssText after DOM insertion, I don't see any reason why we wouldn't do that (aside from it being counter-intuitive, it works in all browsers and avoids this IE bug).

#Comment by Krinkle (talk | contribs)   22:10, 21 December 2011

Filed as bug 33305, and (hopefully) fixed in r106992.

@Santhosh: Could you please test if it works now ? and if so, change code to use mw.util.addCSS. Thanks again!

#Comment by Santhosh.thottingal (talk | contribs)   08:52, 22 December 2011

Thanks Krinkle, I used mw.util.addCSS in r107043 and looks like crash issue is resolved.

#Comment by Krinkle (talk | contribs)   22:42, 22 December 2011

Thank you as well for reporting this!

Status & tagging log