r75644 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r75643‎ \| r75644 \| r75645 >
Date:	15:27, 29 October 2010
Author:	krinkle
Status:	resolved (Comments)
Tags:
Comment:	adding layoutversion to $.client and isLayoutVersion to mw.util. Follow-up of r75593
Modified paths:	/trunk/phase3/resources/jquery/jquery.client.js (modified) (history) /trunk/phase3/resources/mediawiki.util/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/jquery/jquery.client.js
—	—	@@ -16,6 +16,7 @@
17	17	* {
18	18	* 'name': 'firefox',
19	19	* 'layout': 'gecko',
	20	+ * 'layoutVersion': '20101026',
20	21	* 'platform': 'linux'
21	22	* 'version': '3.5.1',
22	23	* 'versionBase': '3',
—	—	@@ -68,6 +69,8 @@
69	70	var layouts = ['gecko', 'konqueror', 'msie', 'opera', 'webkit'];
70	71	// Translations for conforming layout names
71	72	var layoutTranslations = [['konqueror', 'khtml'], ['msie', 'trident'], ['opera', 'presto']];
	73	+ // Names of supported layout engines for version number
	74	+ var layoutVersions = ['applewebkit', 'gecko'];
72	75	// Names of known operating systems
73	76	var platforms = ['win', 'mac', 'linux', 'sunos', 'solaris', 'iphone'];
74	77	// Translations for conforming operating system names
—	—	@@ -85,7 +88,7 @@
86	89
87	90	/* Pre-processing */
88	91
89		~~- var userAgent = navigator.userAgent, match, name = uk, layout = uk, platform = uk, version = x;~~
	92	+ var userAgent = navigator.userAgent, match, name = uk, layout = uk, layoutversion = uk, platform = uk, version = x;
90	93	if ( match = new RegExp( '(' + wildUserAgents.join( '\|' ) + ')' ).exec( userAgent ) ) {
91	94	// Takes a userAgent string and translates given text into something we can more easily work with
92	95	userAgent = translate( userAgent, userAgentTranslations );
—	—	@@ -101,6 +104,9 @@
102	105	if ( match = new RegExp( '(' + layouts.join( '\|' ) + ')' ).exec( userAgent ) ) {
103	106	layout = translate( match[1], layoutTranslations );
104	107	}
	108	+ if ( match = new RegExp( '(' + layoutVersions.join( '\|' ) + ')\\\/(\\d+)').exec( navigator.userAgent.toLowerCase() ) ) {
	109	+ layoutversion = parseInt(match[2]);
	110	+ }
105	111	if ( match = new RegExp( '(' + platforms.join( '\|' ) + ')' ).exec( navigator.platform.toLowerCase() ) ) {
106	112	platform = translate( match[1], platformTranslations );
107	113	}
—	—	@@ -124,6 +130,7 @@
125	131	profile = {
126	132	'name': name,
127	133	'layout': layout,
	134	+ 'layoutVersion': layoutversion,
128	135	'platform': platform,
129	136	'version': version,
130	137	'versionBase': ( version !== x ? new String( version ).substr( 0, 1 ) : x ),
Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
—	—	@@ -17,33 +17,34 @@
18	18
19	19	// Populate clientProfile var
20	20	mw.util.clientProfile = $.client.profile();
	21	+ var webkit = navigator.userAgent.toLowerCase().match(/applewebkit\/(\d+)/);
21	22
22	23	// Set tooltipAccessKeyPrefix
23	24
24		~~- // Opera on any platform~~
25		~~- if ( mw.util.isBrowser('opera') ) {~~
26		~~- this.tooltipAccessKeyPrefix = 'shift-esc-';~~
27		-
28		~~- // Chrome on any platform~~
29		~~- } else if ( mw.util.isBrowser('chrome') ) {~~
30		~~- // Chrome on Mac or Chrome on other platform ?~~
31		~~- this.tooltipAccessKeyPrefix = mw.util.isPlatform('mac') ? 'ctrl-option-' : 'alt-';~~
32		-
33		~~- // Non-Windows Safari with webkit_version > 526~~
34		~~- } else if ( !mw.util.isPlatform('win') && mw.util.isBrowser('safari') && webkit_version > 526 ) {~~
35		~~- this.tooltipAccessKeyPrefix = 'ctrl-alt-';~~
36		-
37		~~- // Safari/Konqueror on any platform, or any browser on Mac (but not Safari on Windows)~~
38		~~- } else if ( !( mw.util.isPlatform('win') && mw.util.isBrowser('safari') )~~
39		~~- && ( mw.util.isBrowser('safari')~~
40		~~- \|\| mw.util.isPlatform('mac')~~
41		~~- \|\| mw.util.isBrowser('konqueror') ) ) {~~
42		~~- this.tooltipAccessKeyPrefix = 'ctrl-';~~
43		-
44		~~- // Firefox 2.x~~
45		~~- } else if ( mw.util.isBrowser('firefox') && mw.util.isBrowserVersion('2') ) {~~
46		~~- this.tooltipAccessKeyPrefix = 'alt-shift-';~~
47		~~- }~~
	25	+ // Opera on any platform
	26	+ if ( mw.util.isBrowser('opera') ) {
	27	+ this.tooltipAccessKeyPrefix = 'shift-esc-';
	28	+
	29	+ // Chrome on any platform
	30	+ } else if ( mw.util.isBrowser('chrome') ) {
	31	+ // Chrome on Mac or Chrome on other platform ?
	32	+ this.tooltipAccessKeyPrefix = mw.util.isPlatform('mac') ? 'ctrl-option-' : 'alt-';
	33	+
	34	+ // Non-Windows Safari with webkit_version > 526
	35	+ } else if ( !mw.util.isPlatform('win') && mw.util.isBrowser('safari') && webkit_version > 526 ) {
	36	+ this.tooltipAccessKeyPrefix = 'ctrl-alt-';
	37	+
	38	+ // Safari/Konqueror on any platform, or any browser on Mac (but not Safari on Windows)
	39	+ } else if ( !( mw.util.isPlatform('win') && mw.util.isBrowser('safari') )
	40	+ && ( mw.util.isBrowser('safari')
	41	+ \|\| mw.util.isPlatform('mac')
	42	+ \|\| mw.util.isBrowser('konqueror') ) ) {
	43	+ this.tooltipAccessKeyPrefix = 'ctrl-';
	44	+
	45	+ // Firefox 2.x
	46	+ } else if ( mw.util.isBrowser('firefox') && mw.util.isBrowserVersion('2') ) {
	47	+ this.tooltipAccessKeyPrefix = 'alt-shift-';
	48	+ }
48	49
49	50	// Enable CheckboxShiftClick
50	51	$('input[type=checkbox]:not(.noshiftselect)').checkboxShiftClick();
—	—	@@ -75,7 +76,7 @@
76	77	*
77	78	* @example mw.util.isBrowser( 'safari' );
78	79	* @param String str name of a browser (case insensitive). Check jquery.client.js for possible values
79		~~- * @return Boolean true of the browsername matches the clients browser~~
	80	+ * @return Boolean true if the browsername matches the clients browser
80	81	*/
81	82	'isBrowser' : function( str ) {
82	83	str = (str + '').toLowerCase();
—	—	@@ -87,7 +88,7 @@
88	89	*
89	90	* @example mw.util.isLayout( 'webkit' );
90	91	* @param String str name of a layout engine (case insensitive). Check jquery.client.js for possible values
91		~~- * @return Boolean true of the layout engine matches the clients browser~~
	92	+ * @return Boolean true if the layout engine matches the clients browser
92	93	*/
93	94	'isLayout' : function( str ) {
94	95	str = (str + '').toLowerCase();
—	—	@@ -95,11 +96,22 @@
96	97	},
97	98
98	99	/**
	100	+ * Checks if the current layout engine version matches
	101	+ *
	102	+ * @example mw.util.isLayoutVersion( 533 );
	103	+ * @param Number num version number of a layout engine.
	104	+ * @return Boolean true if the layout engine matches the clients browser
	105	+ */
	106	+ 'isLayoutVersion' : function( num ) {
	107	+ return this.clientProfile.layoutVersion == num;
	108	+ },
	109	+
	110	+ /**
99	111	* Checks if the current layout matches
100	112	*
101	113	* @example mw.util.isPlatform( 'mac' );
102	114	* @param String str name of a platform (case insensitive). Check jquery.client.js for possible values
103		~~- * @return Boolean true of the platform matches the clients platform~~
	115	+ * @return Boolean true if the platform matches the clients platform
104	116	*/
105	117	'isPlatform' : function( str ) {
106	118	str = (str + '').toLowerCase();
—	—	@@ -111,7 +123,7 @@
112	124	*
113	125	* @example mw.util.isBrowserVersion( '5' );
114	126	* @param String str version number without decimals
115		~~- * @return Boolean true of the version number matches the clients browser~~
	127	+ * @return Boolean true if the version number matches the clients browser
116	128	*/
117	129	'isBrowserVersion' : function( str ) {
118	130	return this.clientProfile.versionBase === str;

Past revisions this follows-up on

Revision	Commit summary	Author	Date
r75593	porting is_opera, is_safari_win etc. to mw.util as isBrowser('..'), isPlatfor...	krinkle	23:19, 27 October 2010

Comments

#Comment by Catrope (talk | contribs) 20:24, 31 October 2010

+					var webkit = navigator.userAgent.toLowerCase().match(/applewebkit\/(\d+)/);

Why are you doing additional browser detection outside $.client? Couldn't it be moved into there? The variable is unused too.

#Comment by Catrope (talk | contribs) 20:26, 31 October 2010

Varaible has mysteriously disappeared in r75645.

#Comment by Krinkle (talk | contribs) 21:54, 31 October 2010

Excuse me, I tested it and (obviously) everything was fine but I guess I committed too early (probably fixed something, saved file, removed that line, committed, and thén saved file).

I'll watch the diffs more closely :)

Status & tagging log

20:26, 31 October 2010 Catrope (talk | contribs) changed the status of r75644 [removed: fixme added: resolved]
20:24, 31 October 2010 Catrope (talk | contribs) changed the status of r75644 [removed: new added: fixme]