r75593 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r75592‎ \| r75593 \| r75594 >
Date:	23:19, 27 October 2010
Author:	krinkle
Status:	resolved (Comments)
Tags:
Comment:	porting is_opera, is_safari_win etc. to mw.util as isBrowser('..'), isPlatform('...') etc.
Modified paths:	/trunk/phase3/resources/mediawiki.util/mediawiki.util.js (modified) (history) /trunk/phase3/resources/mediawiki.util/mediawiki.util.test.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.test.js
—	—	@@ -78,35 +78,42 @@
79	79	'\\.st\\{e\\}\\$st (string)');
80	80	mw.test.addTest('typeof $.fn.checkboxShiftClick',
81	81	'function (string)');
	82	+ mw.test.addTest('typeof mw.util.isBrowser( \'safari\' )',
	83	+ 'boolean (string)');
	84	+ mw.test.addTest('typeof mw.util.isLayout( \'webKit\' )',
	85	+ 'boolean (string)');
	86	+ mw.test.addTest('typeof mw.util.isPlatform( \'MAC\' )',
	87	+ 'boolean (string)');
	88	+ mw.test.addTest('typeof mw.util.isBrowserVersion( \'5\' )',
	89	+ 'boolean (string)');
82	90	mw.test.addTest('typeof mw.util.rawurlencode',
83	91	'function (string)');
84		~~- mw.test.addTest('mw.util.rawurlencode(\'Test: A&B/Here\')',~~
	92	+ mw.test.addTest('mw.util.rawurlencode( \'Test: A&B/Here\' )',
85	93	'Test%3A%20A%26B%2FHere (string)');
86	94	mw.test.addTest('typeof mw.util.wfGetlink',
87	95	'function (string)');
88	96	mw.test.addTest('typeof mw.util.getParamValue',
89	97	'function (string)');
90		~~- mw.test.addTest('mw.util.getParamValue(\'action\')',~~
	98	+ mw.test.addTest('mw.util.getParamValue( \'action\' )',
91	99	'mwutiltest (string)');
92	100	mw.test.addTest('typeof mw.util.htmlEscape',
93	101	'function (string)');
94		~~- mw.test.addTest('mw.util.htmlEscape(\'<a href="http://mw.org/?a=b&c=d">link</a>\')',~~
	102	+ mw.test.addTest('mw.util.htmlEscape( \'<a href="http://mw.org/?a=b&c=d">link</a>\' )',
95	103	'<a href="http://mw.org/?a=b&c=d">link</a> (string)');
96	104	mw.test.addTest('typeof mw.util.htmlUnescape',
97	105	'function (string)');
98		~~- mw.test.addTest('mw.util.htmlUnescape(\'<a href="http://mw.org/?a=b&c=d">link</a>\')',~~
	106	+ mw.test.addTest('mw.util.htmlUnescape( \'<a href="http://mw.org/?a=b&c=d">link</a>\' )',
99	107	'<a href="http://mw.org/?a=b&c=d">link</a> (string)');
100		~~- mw.test.addTest('typeof mw.util.tooltipAccessKeyRegexp',~~
101		~~- 'function (string)');~~
	108	+ mw.test.addTest('mw.util.tooltipAccessKeyRegexp.constructor.name',
	109	+ 'RegExp (string)');
102	110	mw.test.addTest('typeof mw.util.updateTooltipAccessKeys',
103	111	'function (string)');
104	112	mw.test.addTest('typeof mw.util.addPortletLink',
105	113	'function (string)');
106	114	mw.test.addTest('typeof mw.util.addPortletLink("p-tb", "http://mediawiki.org/", "MediaWiki.org", "t-mworg", "Go to MediaWiki.org ", "m", "#t-print")',
107	115	'object (string)');
108		~~- mw.test.addTest('a = mw.util.addPortletLink("p-tb", "http://mediawiki.org/", "MediaWiki.org", "t-mworg", "Go to MediaWiki.org ", "m", "#t-print"); if(a){ a.outerHTML; }',~~
109		~~- '<li id="t-mworg"><span><a href="http://mediawiki.org/" accesskey="m" title="Go to MediaWiki.org [ctrl-alt-m]">MediaWiki.org</a></span></li> (string)',~~
110		~~- 'href="http://mediawiki.org/"');~~
	116	+ mw.test.addTest('a = mw.util.addPortletLink("p-tb", "http://mediawiki.org/", "MediaWiki.org", "t-mworg", "Go to MediaWiki.org ", "m", "#t-print"); $(a).text();',
	117	+ 'MediaWiki.org (string)');
111	118
112	119	// Run tests and compare results
113	120	var exec,
Index: trunk/phase3/resources/mediawiki.util/mediawiki.util.js
—	—	@@ -12,35 +12,50 @@
13	13	if ( this.initialised === false ) {
14	14	this.initialised = true;
15	15
16		~~- // Set tooltipAccessKeyPrefix~~
17		~~- if ( is_opera ) {~~
18		~~- this.tooltipAccessKeyPrefix = 'shift-esc-';~~
19		~~- } else if ( is_chrome ) {~~
20		~~- this.tooltipAccessKeyPrefix = is_chrome_mac ? 'ctrl-option-' : 'alt-';~~
21		~~- } else if ( !is_safari_win && is_safari && webkit_version > 526 ) {~~
22		~~- this.tooltipAccessKeyPrefix = 'ctrl-alt-';~~
23		~~- } else if ( !is_safari_win && ( is_safari~~
24		~~- \|\| clientPC.indexOf('mac') !== -1~~
25		~~- \|\| clientPC.indexOf('konqueror') !== -1 ) ) {~~
26		~~- this.tooltipAccessKeyPrefix = 'ctrl-';~~
27		~~- } else if ( is_ff2 ) {~~
28		~~- this.tooltipAccessKeyPrefix = 'alt-shift-';~~
29		~~- }~~
30		-
31	16	// Any initialisation after the DOM is ready
32	17	$(function () {
	18	+
	19	+ // Populate clientProfile var
	20	+ mw.util.clientProfile = $.client.profile();
33	21
	22	+ // Set tooltipAccessKeyPrefix
	23	+
	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	+ }
	48	+
34	49	// Enable CheckboxShiftClick
35	50	$('input[type=checkbox]:not(.noshiftselect)').checkboxShiftClick();
36	51
37		~~- // Fill bodyContant var~~
	52	+ // Fill $content var
38	53	if ( $('#bodyContent').length ) {
39	54	mw.util.$content = $('#bodyContent');
40	55	} else if ( $('#article').length ) {
41	56	mw.util.$content = $('#article');
42	57	} else {
43	58	mw.util.$content = $('#content');
44		~~- }~~
	59	+ }
45	60	});
46	61
47	62
—	—	@@ -51,7 +66,58 @@
52	67
53	68	/* Main body */
54	69
	70	+ // Holds result of $.client.profile()
	71	+ // Populated by init()
	72	+ 'clientProfile' : {},
	73	+
55	74	/**
	75	+ * Checks if the current browser matches
	76	+ *
	77	+ * @example mw.util.isBrowser( 'safari' );
	78	+ * @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	+ */
	81	+ 'isBrowser' : function( str ) {
	82	+ str = (str + '').toLowerCase();
	83	+ return this.clientProfile.name == str;
	84	+ },
	85	+
	86	+ /**
	87	+ * Checks if the current layout matches
	88	+ *
	89	+ * @example mw.util.isLayout( 'webkit' );
	90	+ * @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	+ */
	93	+ 'isLayout' : function( str ) {
	94	+ str = (str + '').toLowerCase();
	95	+ return this.clientProfile.layout == str;
	96	+ },
	97	+
	98	+ /**
	99	+ * Checks if the current layout matches
	100	+ *
	101	+ * @example mw.util.isPlatform( 'mac' );
	102	+ * @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
	104	+ */
	105	+ 'isPlatform' : function( str ) {
	106	+ str = (str + '').toLowerCase();
	107	+ return this.clientProfile.platform == str;
	108	+ },
	109	+
	110	+ /**
	111	+ * Checks if the current browser version matches
	112	+ *
	113	+ * @example mw.util.isBrowserVersion( '5' );
	114	+ * @param String str version number without decimals
	115	+ * @return Boolean true of the version number matches the clients browser
	116	+ */
	117	+ 'isBrowserVersion' : function( str ) {
	118	+ return this.clientProfile.versionBase === str;
	119	+ },
	120	+
	121	+ /**
56	122	* Encodes the string like PHP's rawurlencode
57	123	*
58	124	* @param String str string to be encoded
—	—	@@ -193,7 +259,6 @@
194	260	// Populated by init()
195	261	'$content' : null,
196	262
197		-
198	263	/**
199	264	* Add a link to a portlet menu on the page, such as:
200	265	*

Follow-up revisions

Revision	Commit summary	Author	Date
r75644	adding layoutversion to $.client and isLayoutVersion to mw.util. Follow-up of...	krinkle	15:27, 29 October 2010
r75645	white-space cleanup throughout $.client and mw.util and mw.util.test (r75593)	krinkle	15:30, 29 October 2010
r75653	$.client has been enhanced and made easier since the initial version, may asw...	krinkle	17:11, 29 October 2010

Comments

#Comment by Catrope (talk | contribs) 15:43, 28 October 2010

+					mw.util.clientProfile = $.client.profile();
 
+					// Set tooltipAccessKeyPrefix
+					
+						// Opera on any platform
+						if ( mw.util.isBrowser('opera') ) {

Why the sudden indentation jump?

+						} else if ( !mw.util.isPlatform('win') && mw.util.isBrowser('safari') && webkit_version > 526 ) {

Aren't you able to get webkit_version from $.client?

Status & tagging log

13:07, 1 November 2010 Catrope (talk | contribs) changed the status of r75593 [removed: fixme added: resolved]
15:43, 28 October 2010 Catrope (talk | contribs) changed the status of r75593 [removed: new added: fixme]