r91415 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r91414‎ \| r91415 \| r91416 >
Date:	19:11, 4 July 2011
Author:	krinkle
Status:	ok
Tags:
Comment:	jquery.client unit testing - More elaborate caching in the plugin in order for the code to be testable - Added support for passing custom user agents (although the default behavior remains unchanged) - Added support for passing custom profile-objects to $.client.test - Added test case for all supported browsers according to http://www.mediawiki.org/wiki/Compatibility#Browser - Replaced userAgent.toLowerCase() with userAgent since this string was already converted to lower case a few lines up This will likely prevent bugs like bug 27652 and bug 29446.
Modified paths:	/trunk/phase3/resources/jquery/jquery.client.js (modified) (history) /trunk/phase3/tests/qunit/suites/resources/jquery/jquery.client.js (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/qunit/suites/resources/jquery/jquery.client.js
—	—	@@ -5,7 +5,154 @@
6	6	ok( jQuery.client, 'jQuery.client defined' );
7	7	});
8	8
9		~~-test( 'profile', function() {~~
	9	+test( 'profile userAgent support', function() {
	10	+ expect(8);
	11	+
	12	+ // Object keyed by userAgent. Value is an array (human-readable name, client-profile object, navigator.platform value)
	13	+ // Info based on results from http://toolserver.org/~krinkle/testswarm/job/174/
	14	+ var uas = {
	15	+ // Internet Explorer 6
	16	+ // Internet Explorer 7
	17	+ 'Mozilla/4.0 (compatible; MSIE 7.0; Windows NT 5.1)': {
	18	+ title: 'Internet Explorer 7',
	19	+ platform: 'Win32',
	20	+ profile: {
	21	+ "name": "msie",
	22	+ "layout": "trident",
	23	+ "layoutVersion": "unknown",
	24	+ "platform": "win",
	25	+ "version": "7.0",
	26	+ "versionBase": "7",
	27	+ "versionNumber": 7
	28	+ }
	29	+ },
	30	+ // Internet Explorer 8
	31	+ // Internet Explorer 9
	32	+ // Internet Explorer 10
	33	+ // Firefox 2
	34	+ // Firefox 3.5
	35	+ 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1.19) Gecko/20110420 Firefox/3.5.19': {
	36	+ title: 'Firefox 3.5',
	37	+ platform: 'MacIntel',
	38	+ profile: {
	39	+ "name": "firefox",
	40	+ "layout": "gecko",
	41	+ "layoutVersion": 20110420,
	42	+ "platform": "mac",
	43	+ "version": "3.5.19",
	44	+ "versionBase": "3",
	45	+ "versionNumber": 3.5
	46	+ }
	47	+ },
	48	+ // Firefox 3.6
	49	+ 'Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110422 Ubuntu/10.10 (maverick) Firefox/3.6.17': {
	50	+ title: 'Firefox 3.6',
	51	+ platform: 'Linux i686',
	52	+ profile: {
	53	+ "name": "firefox",
	54	+ "layout": "gecko",
	55	+ "layoutVersion": 20110422,
	56	+ "platform": "linux",
	57	+ "version": "3.6.17",
	58	+ "versionBase": "3",
	59	+ "versionNumber": 3.6
	60	+ }
	61	+ },
	62	+ // Firefox 4
	63	+ 'Mozilla/5.0 (Windows NT 6.0; rv:2.0.1) Gecko/20100101 Firefox/4.0.1': {
	64	+ title: 'Firefox 4',
	65	+ platform: 'Win32',
	66	+ profile: {
	67	+ "name": "firefox",
	68	+ "layout": "gecko",
	69	+ "layoutVersion": 20100101,
	70	+ "platform": "win",
	71	+ "version": "4.0.1",
	72	+ "versionBase": "4",
	73	+ "versionNumber": 4
	74	+ }
	75	+ },
	76	+ // Firefox 5
	77	+ // Safari 3
	78	+ // Safari 4
	79	+ 'Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_7; nl-nl) AppleWebKit/531.22.7 (KHTML, like Gecko) Version/4.0.5 Safari/531.22.7': {
	80	+ title: 'Safari 4',
	81	+ platform: 'MacIntel',
	82	+ profile: {
	83	+ "name": "safari",
	84	+ "layout": "webkit",
	85	+ "layoutVersion": 531,
	86	+ "platform": "mac",
	87	+ "version": "4.0.5",
	88	+ "versionBase": "4",
	89	+ "versionNumber": 4
	90	+ }
	91	+ },
	92	+ 'Mozilla/5.0 (Windows; U; Windows NT 6.0; cs-CZ) AppleWebKit/533.21.1 (KHTML, like Gecko) Version/4.0.5 Safari/531.22.7': {
	93	+ title: 'Safari 4',
	94	+ platform: 'Win32',
	95	+ profile: {
	96	+ "name": "safari",
	97	+ "layout": "webkit",
	98	+ "layoutVersion": 533,
	99	+ "platform": "win",
	100	+ "version": "4.0.5",
	101	+ "versionBase": "4",
	102	+ "versionNumber": 4
	103	+ }
	104	+ },
	105	+ // Safari 5
	106	+ // Opera 10
	107	+ // Chrome 5
	108	+ // Chrome 6
	109	+ // Chrome 7
	110	+ // Chrome 8
	111	+ // Chrome 9
	112	+ // Chrome 10
	113	+ // Chrome 11
	114	+ // Chrome 12
	115	+ 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_5_8) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.112 Safari/534.30': {
	116	+ title: 'Chrome 12',
	117	+ platform: 'MacIntel',
	118	+ profile: {
	119	+ "name": "chrome",
	120	+ "layout": "webkit",
	121	+ "layoutVersion": 534,
	122	+ "platform": "mac",
	123	+ "version": "12.0.742.112",
	124	+ "versionBase": "12",
	125	+ "versionNumber": 12
	126	+ }
	127	+ },
	128	+ 'Mozilla/5.0 (X11; Linux i686) AppleWebKit/534.30 (KHTML, like Gecko) Chrome/12.0.742.68 Safari/534.30': {
	129	+ title: 'Chrome 12',
	130	+ platform: 'Linux i686',
	131	+ profile: {
	132	+ "name": "chrome",
	133	+ "layout": "webkit",
	134	+ "layoutVersion": 534,
	135	+ "platform": "linux",
	136	+ "version": "12.0.742.68",
	137	+ "versionBase": "12",
	138	+ "versionNumber": 12
	139	+ }
	140	+ }
	141	+ };
	142	+
	143	+ // Generate a client profile object and compare recursively
	144	+ var uaTest = function( rawUserAgent, data ) {
	145	+ var ret = $.client.profile( {
	146	+ userAgent: rawUserAgent,
	147	+ platform: data.platform
	148	+ } );
	149	+ deepEqual( ret, data.profile, 'Client profile support check for ' + data.title + ' (' + data.platform + '): ' + rawUserAgent );
	150	+ };
	151	+
	152	+ // Loop through and run tests
	153	+ $.each( uas, uaTest );
	154	+} );
	155	+
	156	+test( 'profile return validation for current user agent', function() {
10	157	expect(7);
11	158	var p = $.client.profile();
12	159	var unknownOrType = function( val, type, summary ) {
Index: trunk/phase3/resources/jquery/jquery.client.js
—	—	@@ -5,16 +5,22 @@
6	6
7	7	/* Private Members */
8	8
9		~~- var profile;~~
	9	+ /**
	10	+ * @var profileCache {Object} Keyed by userAgent string,
	11	+ * value is the parsed $.client.profile object for that user agent.
	12	+ */
	13	+ var profileCache = {};
10	14
11	15	/* Public Methods */
12	16
13	17	$.client = {
14	18
15	19	/**
16		~~- * Returns an object containing information about the browser~~
	20	+ * Get an object containing information about the client.
17	21	*
18		~~- * The resulting client object will be in the following format:~~
	22	+ * @param nav {Object} An object with atleast a 'userAgent' and 'platform' key.=
	23	+ * Defaults to the global Navigator object.
	24	+ * @return {Object} The resulting client object will be in the following format:
19	25	* {
20	26	* 'name': 'firefox',
21	27	* 'layout': 'gecko',
—	—	@@ -25,9 +31,12 @@
26	32	* 'versionNumber': 3.5,
27	33	* }
28	34	*/
29		~~- profile: function() {~~
	35	+ profile: function( nav ) {
	36	+ if ( nav === undefined ) {
	37	+ nav = window.navigator;
	38	+ }
30	39	// Use the cached version if possible
31		~~- if ( profile === undefined ) {~~
	40	+ if ( profileCache[nav.userAgent] === undefined ) {
32	41
33	42	/* Configuration */
34	43
—	—	@@ -90,7 +99,7 @@
91	100
92	101	/* Pre-processing */
93	102
94		~~- var userAgent = navigator.userAgent,~~
	103	+ var ua = nav.userAgent,
95	104	match,
96	105	name = uk,
97	106	layout = uk,
—	—	@@ -98,28 +107,28 @@
99	108	platform = uk,
100	109	version = x;
101	110
102		~~- if ( match = new RegExp( '(' + wildUserAgents.join( '\|' ) + ')' ).exec( userAgent ) ) {~~
	111	+ if ( match = new RegExp( '(' + wildUserAgents.join( '\|' ) + ')' ).exec( ua ) ) {
103	112	// Takes a userAgent string and translates given text into something we can more easily work with
104		~~- userAgent = translate( userAgent, userAgentTranslations );~~
	113	+ ua = translate( ua, userAgentTranslations );
105	114	}
106	115	// Everything will be in lowercase from now on
107		~~- userAgent = userAgent.toLowerCase();~~
	116	+ ua = ua.toLowerCase();
108	117
109	118	/* Extraction */
110	119
111		~~- if ( match = new RegExp( '(' + names.join( '\|' ) + ')' ).exec( userAgent ) ) {~~
	120	+ if ( match = new RegExp( '(' + names.join( '\|' ) + ')' ).exec( ua ) ) {
112	121	name = translate( match[1], nameTranslations );
113	122	}
114		~~- if ( match = new RegExp( '(' + layouts.join( '\|' ) + ')' ).exec( userAgent ) ) {~~
	123	+ if ( match = new RegExp( '(' + layouts.join( '\|' ) + ')' ).exec( ua ) ) {
115	124	layout = translate( match[1], layoutTranslations );
116	125	}
117		~~- if ( match = new RegExp( '(' + layoutVersions.join( '\|' ) + ')\\\/(\\d+)').exec( navigator.userAgent.toLowerCase() ) ) {~~
	126	+ if ( match = new RegExp( '(' + layoutVersions.join( '\|' ) + ')\\\/(\\d+)').exec( ua ) ) {
118	127	layoutversion = parseInt( match[2], 10 );
119	128	}
120		~~- if ( match = new RegExp( '(' + platforms.join( '\|' ) + ')' ).exec( navigator.platform.toLowerCase() ) ) {~~
	129	+ if ( match = new RegExp( '(' + platforms.join( '\|' ) + ')' ).exec( nav.platform.toLowerCase() ) ) {
121	130	platform = translate( match[1], platformTranslations );
122	131	}
123		~~- if ( match = new RegExp( '(' + versionPrefixes.join( '\|' ) + ')' + versionSuffix ).exec( userAgent ) ) {~~
	132	+ if ( match = new RegExp( '(' + versionPrefixes.join( '\|' ) + ')' + versionSuffix ).exec( ua ) ) {
124	133	version = match[3];
125	134	}
126	135
—	—	@@ -131,13 +140,13 @@
132	141	}
133	142	// Expose Opera 10's lies about being Opera 9.8
134	143	if ( name === 'opera' && version >= 9.8) {
135		~~- version = userAgent.match( /version\/([0-9\.]*)/i )[1] \|\| 10;~~
	144	+ version = ua.match( /version\/([0-9\.]*)/i )[1] \|\| 10;
136	145	}
137	146	var versionNumber = parseFloat( version, 10 ) \|\| 0.0;
138	147
139	148	/* Caching */
140	149
141		~~- profile = {~~
	150	+ profileCache[nav.userAgent] = {
142	151	'name': name,
143	152	'layout': layout,
144	153	'layoutVersion': layoutversion,
—	—	@@ -147,7 +156,7 @@
148	157	'versionNumber': versionNumber
149	158	};
150	159	}
151		~~- return profile;~~
	160	+ return profileCache[nav.userAgent];
152	161	},
153	162
154	163	/**
—	—	@@ -171,12 +180,14 @@
172	181	* }
173	182	* }
174	183	*
175		~~- * @param map Object of browser support map~~
	184	+ * @param map {Object} Browser support map
	185	+ * @param profile {Object} (optional) a client-profile object.
176	186	*
177	187	* @return Boolean true if browser known or assumed to be supported, false if blacklisted
178	188	*/
179		~~- test: function( map ) {~~
180		~~- var profile = $.client.profile();~~
	189	+ test: function( map, profile ) {
	190	+ profile = $.isPlainObject( profile ) ? profile : $.client.profile();
	191	+
181	192	var dir = $( 'body' ).is( '.rtl' ) ? 'rtl' : 'ltr';
182	193	// Check over each browser condition to determine if we are running in a compatible client
183	194	if ( typeof map[dir] !== 'object' \|\| typeof map[dir][profile.name] === 'undefined' ) {

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r83383	Fixed (bug 27652) [jQuery.client] versionBase is wrong for versions higher wi...	krinkle	14:34, 6 March 2011

Status & tagging log

19:59, 9 August 2011 Aaron Schulz (talk | contribs) changed the status of r91415 [removed: new added: ok]