r102673 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r102672‎ | r102673 | r102674 >
Date:18:48, 10 November 2011
Author:krinkle
Status:ok
Tags:
Comment:
[WebFonts] minor whitespace and conventions
- single quotes
- spacing
- sentence case comments (//foo => // Foo)
- removing redundant comments (such as "Version number", or "reset the font size")
- 'font-xxx' => fontXxx. In JavaScript (almost, if not all) native variables have a camelCase accessor. jQuery supports mapping of 'foo-xxx' to fooXxx, but when accessing it as an object it's easier/shorter/faster to use camelCase (<code>foo['font-family']</code> vs. <code>foo.fontFamily</code>). Also that way you can use the same naming with and without jQuery (since 'font-family' only works inside jQuery functions)
Modified paths:
  • /trunk/extensions/WebFonts/resources/ext.webfonts.js (modified) (history)

Diff [purge]

Index: trunk/extensions/WebFonts/resources/ext.webfonts.js
@@ -4,49 +4,48 @@
55 if ( typeof font !== 'string' ) {
66 return font;
77 }
8 - return "webfont-"+font.toLowerCase().replace(/[_ ]/g, '-').replace(/[^-a-z]/g, '');
 8+ return 'webfont-' + font.toLowerCase().replace(/[_ ]/g, '-' ).replace(/[^-a-z]/g, '' );
99 }
1010
1111 $.webfonts = {
1212
1313 oldconfig: false,
1414 config: $.webfonts.config,
15 - /* Version number */
16 - version: "0.1.2",
 15+ version: '0.1.2',
1716 fonts : [],
1817 set: function( font ) {
19 - if ( !font || font === "none" ) {
 18+ if ( !font || font === 'none' ) {
2019 $.webfonts.reset();
2120 return;
2221 }
2322
2423 if ( !( font in $.webfonts.config.fonts ) ) {
25 - mw.log( "Requested unknown font", font );
 24+ mw.log( 'Requested unknown font', font );
2625 return;
2726 }
2827 var config = $.webfonts.config.fonts[font];
2928
30 - //load the style sheet for the font
 29+ // Load the style sheet for the font
3130 $.webfonts.addFont( font );
3231
33 - //save the current font and its size. Used for reset.
 32+ // Save the current font and its size. Used for reset.
3433 if ( !$.webfonts.oldconfig ) {
3534 var $body = $( 'body' );
3635 $.webfonts.oldconfig = {
37 - "font-family": $body.css( 'font-family' ),
38 - "font-size": $body.css( 'font-size' )
 36+ fontFamily: $body.css( 'font-family' ),
 37+ fontSize: $body.css( 'font-size' )
3938 };
4039 }
4140
42 - //Set the font, fallback fonts.Need to change the fonts of Input Select and Textarea explicitly.
43 - $( 'body, input, select, textarea' ).css( 'font-family', "'"+ font +"', Helvetica, Arial, sans-serif" );
 41+ // Set the font, fallback fonts.Need to change the fonts of Input Select and Textarea explicitly.
 42+ $( 'body, input, select, textarea' ).css( 'font-family', '"' + font + '", Helvetica, Arial, sans-serif' );
4443
4544 if ( 'normalization' in config ) {
4645 $( document ).ready( function() {
4746 $.webfonts.normalize( config.normalization );
4847 } );
4948 }
50 - //set the font option in cookie
 49+ // Store the font choise in cookie
5150 $.cookie( 'webfonts-font', font, { 'path': '/', 'expires': 30 } );
5251
5352 // If we had reset the fonts for tags with lang attribute, apply the fonts again.
@@ -58,17 +57,16 @@
5958 */
6059 reset: function() {
6160 $( 'body' ).css( {
62 - 'font-family': $.webfonts.oldconfig["font-family"],
63 - //reset the font size from old configuration
64 - 'font-size': $.webfonts.oldconfig[ "font-size" ]
 61+ fontFamily: $.webfonts.oldconfig.fontFamily,
 62+ fontSize: $.webfonts.oldconfig.fontSize
6563 });
66 - //we need to reset the fonts of Input and Select explicitly.
67 - $( 'input, select' ).css( 'font-family', $.webfonts.oldconfig["font-family"] );
 64+ // We need to reset the font family of Input and Select explicitly.
 65+ $( 'input, select' ).css( 'font-family', $.webfonts.oldconfig.fontFamily );
6866
6967 // Reset the fonts applied for tags with lang attribute.
7068 $( '.webfonts-lang-attr' ).css( 'font-family', 'none' ).removeClass( 'webfonts-lang-attr' );
7169
72 - //remove the cookie
 70+ // Remove the cookie
7371 $.cookie( 'webfonts-font', 'none', { 'path': '/', 'expires': 30 } );
7472 },
7573
@@ -78,8 +76,8 @@
7977 */
8078 normalize: function( normalizationRules ) {
8179 $.each( normalizationRules, function( search, replace ) {
82 - var searchPattern = new RegExp( search,"g" );
83 - return $( '*' ).each(function() {
 80+ var searchPattern = new RegExp( search, 'g' );
 81+ return $( '*' ).each( function() {
8482 var node = this.firstChild,
8583 val, newVal;
8684 if ( node ) {
@@ -97,35 +95,39 @@
9896 } );
9997 },
10098
101 - /*
102 - * Construct the css required for the fontfamily, inject it to the head of the body
 99+ /**
 100+ * Construct the CSS required for the font-family, inject it to the head of the body
103101 * so that it gets loaded.
104 - * @param fontfamily The fontfamily name
 102+ * @param fontFamily The font-family name
105103 */
106 - loadcss: function( fontfamily ) {
107 - var fontconfig = $.webfonts.config.fonts[fontfamily];
108 - var base = mw.config.get( "wgExtensionAssetsPath" ) + "/WebFonts/fonts/";
 104+ loadCSS: function( fontFamily ) {
 105+ var fontconfig = $.webfonts.config.fonts[fontFamily];
 106+ var base = mw.config.get( 'wgExtensionAssetsPath' ) + '/WebFonts/fonts/';
109107 var styleString =
110108 "<style type='text/css'>\n@font-face {\n"
111 - + "\tfont-family: '"+fontfamily+"';\n";
 109+ + "\tfont-family: '"+fontFamily+"';\n";
 110+
112111 if ( 'eot' in fontconfig ) {
113112 styleString += "\tsrc: url('"+base+fontconfig.eot+"');\n";
114113 }
 114+
115115 styleString += "\tsrc: ";
116116 // If the font is present locally, use it.
117117 var ua = navigator.userAgent;
118118 if( ua.match( /Android 2\.3/ ) == null ) {
119119 // Android 2.3.x does not respect local() syntax.
120120 // http://code.google.com/p/android/issues/detail?id=10609
121 - styleString += "local('"+fontfamily+"'),";
 121+ styleString += "local('"+fontFamily+"'),";
122122 }
123123
124124 if ( 'woff' in fontconfig ) {
125125 styleString += "\t\turl('"+base+fontconfig.woff+"') format('woff'),";
126126 }
 127+
127128 if ( 'svg' in fontconfig ) {
128 - styleString += "\t\turl('"+base+fontconfig.svg+"#"+fontfamily+"') format('svg'),";
 129+ styleString += "\t\turl('"+base+fontconfig.svg+"#"+fontFamily+"') format('svg'),";
129130 }
 131+
130132 if ( 'ttf' in fontconfig ) {
131133 styleString += "\t\turl('"+base+fontconfig.ttf+"') format('truetype');\n";
132134 }
@@ -137,18 +139,18 @@
138140
139141 },
140142
141 - /*
 143+ /**
142144 * Add a font to the page.
143145 * This method ensures that css are not duplicated and
144146 * keep track of added fonts.
145 - * @param fontFamilyName The fontfamily name
 147+ * @param fontFamilyName {String} The font-family name
146148 */
147149 addFont: function( fontFamilyName ) {
148 - // avoid duplication
149 - if ( $.inArray( fontFamilyName, $.webfonts.fonts ) < 0 ){
150 - // check whether the requested font is available.
 150+ // Avoid duplicate loading
 151+ if ( $.inArray( fontFamilyName, $.webfonts.fonts ) === -1 ) {
 152+ // Check whether the requested font is available.
151153 if ( fontFamilyName in $.webfonts.config.fonts ) {
152 - $.webfonts.loadcss( fontFamilyName );
 154+ $.webfonts.loadCSS( fontFamilyName );
153155 $.webfonts.fonts.push( fontFamilyName );
154156 }
155157 }
@@ -166,8 +168,8 @@
167169 for ( var i = 0; i < requested.length; i++ ) {
168170 if ( requested[i] in languages ) {
169171 var fonts = languages[requested[i]];
170 - for ( var j = 0; j < fonts.length; j++) {
171 - if ( $.inArray(fonts[j], config) === -1 ) {
 172+ for ( var j = 0; j < fonts.length; j++ ) {
 173+ if ( $.inArray( fonts[j], config ) === -1 ) {
172174 config.push( fonts[j] );
173175 }
174176 }
@@ -180,7 +182,7 @@
181183 var cookieFont = $.cookie( 'webfonts-font' );
182184 var selectedFont = null;
183185 // check whether this font is for the current userlang/contentlang
184 - if ( $.inArray( cookieFont, config ) !== -1){
 186+ if ( $.inArray( cookieFont, config ) !== -1 ) {
185187 selectedFont = cookieFont;
186188 }
187189 else{
@@ -190,13 +192,14 @@
191193 }
192194 if ( selectedFont && selectedFont !== 'none' ) {
193195 $.webfonts.set( selectedFont );
194 - //mark it as checked
 196+ // Mark it as checked
195197 $( '#'+fontID( selectedFont ) ).prop( 'checked', true );
196198 }
197199
198200 $.webfonts.loadFontsForFontFamilyStyle();
199201 $.webfonts.loadFontsForLangAttr();
200 - if ( $('.webfonts-lang-attr').length && !$( '#webfonts-fontsmenu' ).length ) {
 202+
 203+ if ( $( '.webfonts-lang-attr' ).length && !$( '#webfonts-fontsmenu' ).length ) {
201204 // We need to show the reset option even if there is no font to show
202205 // for the language, if there is lang attr based font embedding.
203206 $.webfonts.buildMenu( config );
@@ -209,14 +212,13 @@
210213 */
211214 loadFontsForLangAttr: function() {
212215 var languages = $.webfonts.config.languages;
213 - //if there are tags with lang attribute,
 216+ // If there are tags with lang attribute,
214217 $( 'body' ).find( '*[lang]' ).each( function( index ) {
215 - //check the availability of font.
216 - //add a font-family style if it does not have any
217 - if( languages[this.lang] && ( !this.style.fontFamily || this.style.fontFamily == "none" ) ) {
 218+ // .. check the availability of font, add a font-family style if it does not have any
 219+ if( languages[this.lang] && ( !this.style.fontFamily || this.style.fontFamily === 'none' ) ) {
218220 fontFamily = languages[this.lang][0];
219221 $.webfonts.addFont( fontFamily );
220 - $(this).css('font-family', fontFamily).addClass('webfonts-lang-attr');
 222+ $(this).css( 'font-family', fontFamily ).addClass( 'webfonts-lang-attr' );
221223 }
222224 });
223225
@@ -228,13 +230,13 @@
229231 */
230232 loadFontsForFontFamilyStyle: function() {
231233 var languages = $.webfonts.config.languages;
232 - //if there are tags with font-family style definition, get a list of fonts to be loaded
233 - $( 'body' ).find( '*[style]' ).each(function( index ) {
 234+ // If there are tags with font-family style definition, get a list of fonts to be loaded
 235+ $( 'body' ).find( '*[style]' ).each( function( index ) {
234236 if( this.style.fontFamily ) {
235 - var fontFamilyItems = this.style.fontFamily.split( "," );
 237+ var fontFamilyItems = this.style.fontFamily.split( ',' );
236238 $.each( fontFamilyItems, function( index, fontFamily ) {
237 - //remove the ' characters if any.
238 - fontFamily = fontFamily.replace(/'/g, '');
 239+ // Remove the ' characters if any.
 240+ fontFamily = fontFamily.replace( /'/g, '' );
239241 $.webfonts.addFont( fontFamily );
240242 });
241243 }
@@ -249,18 +251,18 @@
250252 buildMenu: function(config) {
251253 var haveSchemes = false;
252254 // Build font dropdown
253 - var $fontsMenu = $( '<ul>' ).attr('id','webfonts-fontsmenu');
254 - $fontsMenu.delegate( 'input:radio', 'change', function( event ) {
 255+ var $fontsMenu = $( '<ul>' ).attr( 'id', 'webfonts-fontsmenu' );
 256+ $fontsMenu.delegate( 'input:radio', 'change', function( e ) {
255257 $.webfonts.set( $(this).val() );
256258 } );
257259 for ( var scheme in config ) {
258260 var $fontLink = $( '<input type="radio" />' )
259 - .attr( "name", "font" )
260 - .attr( "id", fontID( config[scheme] ) )
 261+ .attr( 'name', 'font' )
 262+ .attr( 'id', fontID( config[scheme] ) )
261263 .val( config[scheme] );
262264
263265 var $fontLabel = $( '<label>' )
264 - .attr( "for",fontID(config[scheme] ) )
 266+ .attr( 'for',fontID(config[scheme] ) )
265267 .append( $fontLink )
266268 .append( config[scheme] );
267269
@@ -274,24 +276,24 @@
275277
276278 }
277279
278 - if ( !haveSchemes && !$('.webfonts-lang-attr').length ) {
 280+ if ( !haveSchemes && !$( '.webfonts-lang-attr' ).length ) {
279281 // No schemes available, and no tags with lang attr
280282 // with fonts loaded. Don't show the menu.
281283 return;
282284 }
283285
284286 var $resetLink = $( '<input type="radio" />' )
285 - .attr( "name","font" )
286 - .attr( "value","webfont-none" )
287 - .attr( "id","webfont-none" )
288 - .click( function( event ) {
 287+ .attr( 'name', 'font' )
 288+ .attr( 'value', 'webfont-none' )
 289+ .attr( 'id', 'webfont-none' )
 290+ .click( function( e ) {
289291 $.webfonts.set( 'none' );
290292 });
291293
292294 var $resetLabel = $( '<label>' )
293 - .attr( "for","webfont-none" )
 295+ .attr( 'for', 'webfont-none' )
294296 .append( $resetLink )
295 - .append( mw.msg("webfonts-reset") );
 297+ .append( mw.msg( 'webfonts-reset' ) );
296298
297299 var $resetLinkItem = $( '<li>' )
298300 .val( 'none' )
@@ -299,30 +301,34 @@
300302
301303 $fontsMenu.append( $resetLinkItem );
302304
303 - var $menuDiv = $( '<div>' ).attr('id','webfonts-fonts')
304 - .addClass( 'menu' )
305 - .append( $fontsMenu )
306 - .append();
 305+ var $menuDiv = $( '<div>' )
 306+ .attr( 'id', 'webfonts-fonts' )
 307+ .addClass( 'menu' )
 308+ .append( $fontsMenu )
 309+ .append();
307310
308 - var $div = $( '<div>' ).attr( 'id','webfonts-menu' )
 311+ var $div = $( '<div>' )
 312+ .attr( 'id', 'webfonts-menu' )
309313 .addClass( 'webfontMenu' )
310 - .append( $( '<a>' ).prop( 'href', '#' ).append( mw.msg( "webfonts-load" ) ) )
 314+ .append( $( '<a>' ).prop( 'href', '#' ).append( mw.msg( 'webfonts-load' ) ) )
311315 .append( $menuDiv );
312316
313 - //this is the fonts link
314 - var $li = $( '<li>' ).attr( 'id','pt-webfont' )
 317+ // This is the fonts link
 318+ var $li = $( '<li>' )
 319+ .attr( 'id', 'pt-webfont' )
315320 .append( $div );
316321
317 - //if rtl, add to the right of top personal links. Else, to the left
318 - var fn = $( 'body' ).hasClass( 'rtl' ) ? "append" : "prepend";
 322+ // If RTL, add to the right of top personal links. Else, to the left
 323+ var fn = $( 'body' ).hasClass( 'rtl' ) ? 'append' : 'prepend';
319324 $( '#p-personal ul:first' )[fn]( $li );
320 - //workaround for IE bug - activex components like input fields coming on top of everything.
321 - //TODO: is there a better solution other than hiding it on hover?
 325+
 326+ // Workaround for IE bug - ActiveX components like input fields coming on top of everything.
 327+ // @todo Is there a better solution other than hiding it on hover?
322328 if ( $.browser.msie ) {
323 - $( '#webfonts-menu' ).hover( function( ) {
324 - $( '#searchform' ).css({ visibility: "hidden" } );
 329+ $( '#webfonts-menu' ).hover( function() {
 330+ $( '#searchform' ).css({ visibility: 'hidden' } );
325331 }, function( ) {
326 - $( '#searchform' ).css( { visibility: "visible" } );
 332+ $( '#searchform' ).css( { visibility: 'visible' } );
327333 } );
328334 }
329335 }

Status & tagging log