r109454 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r109453‎ | r109454 | r109455 >
Date:23:24, 18 January 2012
Author:amire80
Status:resolved (Comments)
Tags:i18nreview 
Comment:
Added function browserIsBlacklisted, to avoid running tests on blacklisted browsers.
More constants to make code less repetitive.
Added tests for reset() - some fail for strange reasons.
Some cleanup.
Modified paths:
  • /trunk/extensions/WebFonts/tests/qunit/ext.webfonts.tests.js (modified) (history)

Diff [purge]

Index: trunk/extensions/WebFonts/tests/qunit/ext.webfonts.tests.js
@@ -3,13 +3,20 @@
44 test( '-- Initial check', function() {
55 expect(1);
66
7 - ok( mw.webfonts, 'mw.webfonts is defined' );
8 -
 7+ if ( browserIsBlacklisted() ) {
 8+ ok( mw.webfonts === undefined, 'mw.webfonts is not defined because we are running in a blacklisted browser' );
 9+ } else {
 10+ ok( mw.webfonts, 'mw.webfonts is defined and the browser is supported' );
 11+ }
912 } );
1013
11 -test( '-- Web font application to body', function() {
12 - expect( 17 );
 14+test( '-- Application of a web font to the page and its removal', function() {
 15+ if ( browserIsBlacklisted() ) {
 16+ return;
 17+ }
1318
 19+ expect( 25 );
 20+
1421 var invalidFont = 'NonExistingFont';
1522 assertTrue( mw.webfonts.set( invalidFont ) === undefined, 'A non-existent font is not initialized' );
1623 // TODO: test that the right thing was written to the log
@@ -38,20 +45,35 @@
3946 deepEqual( oldConfig, mw.webfonts.oldconfig, 'Previous body css was saved properly' );
4047
4148 // Font application
42 - equal( $body.css( 'font-family' ), "'" + teluguFont + "', " + fallbackFonts, 'The web font was applied to font-family of body' );
43 - equal( $inputElement.css( 'font-family' ), "'" + teluguFont + "', " + fallbackFonts, 'The web font was applied to font-family of input' );
44 - equal( $selectElement.css( 'font-family' ), "'" + teluguFont + "', " + fallbackFonts, 'The web font was applied to font-family of select' );
45 - equal( $textareaElement.css( 'font-family' ), "'" '"' + teluguFont + "', " + fallbackFonts, 'The web font was applied to font-family of textarea' );
 49+ var expectedFontFamilyValue = "'" + teluguFont + "', " + fallbackFonts;
 50+ equal( $body.css( 'font-family' ), expectedFontFamilyValue, 'The web font was applied to font-family of body' );
 51+ equal( $inputElement.css( 'font-family' ), expectedFontFamilyValue, 'The web font was applied to font-family of input' );
 52+ equal( $selectElement.css( 'font-family' ), expectedFontFamilyValue, 'The web font was applied to font-family of select' );
 53+ equal( $textareaElement.css( 'font-family' ), expectedFontFamilyValue, 'The web font was applied to font-family of textarea' );
4654
47 - // Restore <body>
48 - $body.attr( 'lang', bodyLang );
49 - ok( mw.webfonts.reset(), 'Reset body after testing font application' );
 55+ // Reset everything
 56+ ok( mw.webfonts.set( false ) === undefined, 'Reset body after testing font application' );
 57+ equals( $body.css( 'font-family' ), oldConfig.fontFamily, 'Previous font-family for body was restored' );
 58+ equals( $body.css( 'font-size' ), oldConfig.fontSize, 'Previous font-size for body was restored' );
 59+ equals( $inputElement.css( 'font-family' ), oldConfig.fontFamily, 'Previous font-family for body was restored' );
 60+ equals( $inputElement.css( 'font-size' ), oldConfig.fontSize, 'Previous font-size for body was restored' );
 61+ equals( $selectElement.css( 'font-family' ), oldConfig.fontFamily, 'Previous font-family for the select element was restored' );
 62+ equals( $selectElement.css( 'font-size' ), oldConfig.fontSize, 'Previous font-size for the select element restored' );
 63+ equals( $textareaElement.css( 'font-family' ), oldConfig.fontFamily, 'Previous font-family for the textarea element was restored' );
 64+ equals( $textareaElement.css( 'font-size' ), oldConfig.fontSize, 'Previous font-size for the textarea element was restored' );
 65+
5066 ok( $inputElement.remove(), 'The input test element was removed from body' );
5167 ok( $selectElement.remove(), 'The select test element was removed from body' );
5268 ok( $textareaElement.remove(), 'The textarea test element was removed from body' );
 69+
 70+ $body.attr( 'lang', bodyLang );
5371 } );
5472
5573 test( '-- Dynamic font loading', function() {
 74+ if ( browserIsBlacklisted() ) {
 75+ return;
 76+ }
 77+
5678 expect( 7 );
5779
5880 var validFontName = mw.webfonts.config.languages.hi[0];
@@ -68,6 +90,10 @@
6991 } );
7092
7193 test( '-- Dynamic font loading based on lang attribute', function() {
 94+ if ( browserIsBlacklisted() ) {
 95+ return;
 96+ }
 97+
7298 expect( 15 );
7399
74100 mw.webfonts.fonts = [];
@@ -96,13 +122,18 @@
97123 assertTrue( $.inArray( tamilFont, mw.webfonts.fonts ) >= 0 , 'Tamil font loaded' );
98124 assertTrue( isFontFaceLoaded( tamilFont ), 'New css rule font-face was added to the document for Tamil font' );
99125
100 - ok( mw.webfonts.reset(), 'Reset webfonts' );
101 - assertFalse( $testElement.hasClass( 'webfonts-lang-attr' ), 'The element has no webfonts-lang-attr since we reset it' );
 126+ ok( mw.webfonts.reset(), 'Reset webfonts after testing application by lang' );
 127+ assertFalse( $testElement.hasClass( 'webfonts-lang-attr' ), 'The testing element has no webfonts-lang-attr since we reset it' );
 128+ // equals( $( 'body' ).find( '*[lang]' ).length, 0, 'There are no elements with the webfonts-lang-attr class' );
102129
103130 ok( $testElement.remove(), 'The test element was removed from body' );
104131 } );
105132
106133 test( '-- Dynamic font loading based on font-family style attribute', function() {
 134+ if ( browserIsBlacklisted() ) {
 135+ return;
 136+ }
 137+
107138 expect( 14 );
108139
109140 mw.webfonts.fonts = [];
@@ -111,21 +142,21 @@
112143 assertTrue( $testElement !== [], 'The test element is defined' );
113144
114145 var latinWebFont = 'RufScript';
115 - var fallbackFonts = ', Arial, Helvetica, sans';
116 - $testElement.attr( 'style','font-family: ' + latinWebFont + fallbackFonts );
 146+ var fallbackFonts = 'Helvetica, Arial, sans-serif';
 147+ $testElement.attr( 'style','font-family: ' + latinWebFont + ', ' + fallbackFonts );
117148 assertTrue( $.inArray( latinWebFont, mw.webfonts.fonts ) === -1 , 'Latin font not loaded yet' );
118149 ok( mw.webfonts.loadFontsForFontFamilyStyle(), 'Loaded fonts from font-family' );
119150 assertTrue( $.inArray( latinWebFont, mw.webfonts.fonts ) >= 0 , 'Latin font loaded' );
120151 assertTrue( isFontFaceLoaded( latinWebFont ), 'New css rule added to the document for Latin' );
121152
122153 var invalidFont = 'NonExistingFont';
123 - $testElement.attr( 'style','font-family: ' + invalidFont + fallbackFonts );
 154+ $testElement.attr( 'style','font-family: ' + invalidFont + ', ' + fallbackFonts );
124155 ok( mw.webfonts.loadFontsForFontFamilyStyle(), 'Attempted to load non-existing fonts specified in font-family' );
125156 assertTrue( $.inArray( invalidFont, mw.webfonts.fonts ) === -1 , 'Font not loaded since it is not existing, including fallback fonts' );
126157 assertFalse( isFontFaceLoaded( invalidFont ), 'No new css rule added to the document since the font does not exist' );
127158
128159 var malayalamFont = mw.webfonts.config.languages.ml[0];
129 - $testElement.attr( 'style', 'font-family: ' + invalidFont + ', ' + malayalamFont + fallbackFonts );
 160+ $testElement.attr( 'style', 'font-family: ' + invalidFont + ', ' + malayalamFont + ', ' + fallbackFonts );
130161 assertTrue( $.inArray( malayalamFont, mw.webfonts.fonts ) === -1 , 'Fallback font not loaded yet' );
131162 ok( mw.webfonts.loadFontsForFontFamilyStyle(), 'Loading fonts from font-family' );
132163 assertTrue( $.inArray( malayalamFont, mw.webfonts.fonts ) >= 0 , 'A fallback font was loaded' );
@@ -135,24 +166,31 @@
136167 } );
137168
138169 test( '-- Build the menu', function() {
 170+ if ( browserIsBlacklisted() ) {
 171+ return;
 172+ }
 173+
139174 expect( 8 );
 175+
140176 var oldFonts = mw.webfonts.fonts;
141177 var fonts = [];
142 - assertFalse( mw.webfonts.buildMenu( fonts ) , 'Build the menu with empty fonts list' );
 178+ assertFalse( mw.webfonts.buildMenu( fonts ), 'Build the menu with empty fonts list' );
143179 fonts = mw.webfonts.config.languages.hi;
144 - ok( mw.webfonts.buildMenu( fonts ) , 'Build the menu with hindi fonts list' );
 180+ ok( mw.webfonts.buildMenu( fonts ) , 'Build the menu with Hindi fonts list' );
145181 equals( $( 'li#pt-webfont' ).length , 1, 'There should be one and only one menu at any time' );
146 - ok( mw.webfonts.buildMenu( fonts ) , 'Build the menu with hindi fonts list again' );
 182+ ok( mw.webfonts.buildMenu( fonts ) , 'Build the menu with Hindi fonts list again' );
147183 equals( $( 'li#pt-webfont' ).length , 1, 'There should be one and only one menu at any time' );
148 - equals( $( 'ul#webfonts-fontsmenu li' ).length , fonts.length + 2 , 'Number of menu items is number of availables fonts, a help link and reset item' );
149 - equals ( $( 'li.webfont-help-item').length , 1, 'Help link exists' );
 184+ equals( $( 'ul#webfonts-fontsmenu li' ).length, fonts.length + 2, 'Number of menu items is number of availables fonts, a help link and reset item' );
 185+ equals ( $( 'li.webfont-help-item').length, 1, 'Help link exists' );
150186 if (oldFonts.length)
151 - assertTrue( mw.webfonts.buildMenu( oldFonts ) , 'Restore the menu' );
 187+ assertTrue( mw.webfonts.buildMenu( oldFonts ), 'Restore the menu' );
152188 else {
153 - assertFalse( mw.webfonts.buildMenu( oldFonts ) , 'Restore the menu' );
 189+ assertFalse( mw.webfonts.buildMenu( oldFonts ), 'Restore the menu' );
154190 }
155191 } );
156192
 193+// Tests end here
 194+
157195 isFontFaceLoaded = function( fontFamilyName ) {
158196 var lastStyleIndex = document.styleSheets.length - 1;
159197
@@ -169,3 +207,14 @@
170208
171209 return false;
172210 }
 211+
 212+browserIsBlacklisted = function() {
 213+ var ua = navigator.userAgent;
 214+ if ( ( /MSIE 6/i.test( ua ) )
 215+ || ( /MSIE 8/i.test( ua ) && /Windows NT 5.1/i.test( ua ) ) )
 216+ {
 217+ return true;
 218+ }
 219+
 220+ return false;
 221+}

Follow-up revisions

RevisionCommit summaryAuthorDate
r109545Using the browser checking function from mw.webfonts. Converting the font-fam...amire8016:30, 19 January 2012
r109551A little less illogical test for unsupported browser.amire8016:57, 19 January 2012

Comments

#Comment by Santhosh.thottingal (talk | contribs)   04:36, 19 January 2012

mw.webfonts will not be undefined if the browser is blacklisted. Because we are doing the browser check in mw.webfonts.setup and that is called from ext.webfonts.init.js

#Comment by Nikerabbit (talk | contribs)   09:26, 19 January 2012

If that is the case, then we can even have browserIsBlacklisted in mw.webfonts. The list should not be duplicated in two places.

#Comment by Amire80 (talk | contribs)   16:58, 19 January 2012

Addressed in r109545.

#Comment by Amire80 (talk | contribs)   16:59, 19 January 2012

Addressed very basically in r109551. Can you think of anything better?

Maybe just not running the rest of tests in an unsupported browser is good enough.

Status & tagging log