r93012 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r93011‎ | r93012 | r93013 >
Date:20:22, 24 July 2011
Author:krinkle
Status:ok
Tags:
Comment:
More mediawiki.js cleanup
- Convert prototype object modifications into object literal. Saves bandwidth (less characters) and speeds up execution (no need to access 2 level deep object member repetitively). Local testing (Chrome Web Inspector) shows 14.80KB to 14.67KB (non-cached, debug=false), and execution time on cached request 32ms to 25ms.
- Un-indent 1 tab for d.setTime (Follows up r92964)
- Whitespace consistency
- Move var statements to top of loader's addScript

Follows up: r92933
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -29,79 +29,81 @@
3030 return this;
3131 }
3232
33 - /**
34 - * Get the value of one or multiple a keys.
35 - *
36 - * If called with no arguments, all values will be returned.
37 - *
38 - * @param selection mixed String key or array of keys to get values for.
39 - * @param fallback mixed Value to use in case key(s) do not exist (optional).
40 - * @return mixed If selection was a string returns the value or null,
41 - * If selection was an array, returns an object of key/values (value is null if not found),
42 - * If selection was not passed or invalid, will return the 'values' object member (be careful as
43 - * objects are always passed by reference in JavaScript!).
44 - * @return Values as a string or object, null if invalid/inexistant.
45 - */
46 - Map.prototype.get = function( selection, fallback ) {
47 - if ( $.isArray( selection ) ) {
48 - selection = $.makeArray( selection );
49 - var results = {};
50 - for ( var i = 0; i < selection.length; i++ ) {
51 - results[selection[i]] = this.get( selection[i], fallback );
52 - }
53 - return results;
54 - } else if ( typeof selection === 'string' ) {
55 - if ( this.values[selection] === undefined ) {
56 - if ( fallback !== undefined ) {
57 - return fallback;
 33+ Map.prototype = {
 34+ /**
 35+ * Get the value of one or multiple a keys.
 36+ *
 37+ * If called with no arguments, all values will be returned.
 38+ *
 39+ * @param selection mixed String key or array of keys to get values for.
 40+ * @param fallback mixed Value to use in case key(s) do not exist (optional).
 41+ * @return mixed If selection was a string returns the value or null,
 42+ * If selection was an array, returns an object of key/values (value is null if not found),
 43+ * If selection was not passed or invalid, will return the 'values' object member (be careful as
 44+ * objects are always passed by reference in JavaScript!).
 45+ * @return Values as a string or object, null if invalid/inexistant.
 46+ */
 47+ get: function( selection, fallback ) {
 48+ if ( $.isArray( selection ) ) {
 49+ selection = $.makeArray( selection );
 50+ var results = {};
 51+ for ( var i = 0; i < selection.length; i++ ) {
 52+ results[selection[i]] = this.get( selection[i], fallback );
5853 }
59 - return null;
 54+ return results;
 55+ } else if ( typeof selection === 'string' ) {
 56+ if ( this.values[selection] === undefined ) {
 57+ if ( fallback !== undefined ) {
 58+ return fallback;
 59+ }
 60+ return null;
 61+ }
 62+ return this.values[selection];
6063 }
61 - return this.values[selection];
62 - }
63 - if ( selection === undefined ) {
64 - return this.values;
65 - } else {
66 - return null; // invalid selection key
67 - }
68 - };
 64+ if ( selection === undefined ) {
 65+ return this.values;
 66+ } else {
 67+ return null; // invalid selection key
 68+ }
 69+ },
6970
70 - /**
71 - * Sets one or multiple key/value pairs.
72 - *
73 - * @param selection mixed String key or array of keys to set values for.
74 - * @param value mixed Value to set (optional, only in use when key is a string)
75 - * @return bool This returns true on success, false on failure.
76 - */
77 - Map.prototype.set = function( selection, value ) {
78 - if ( $.isPlainObject( selection ) ) {
79 - for ( var s in selection ) {
80 - this.values[s] = selection[s];
 71+ /**
 72+ * Sets one or multiple key/value pairs.
 73+ *
 74+ * @param selection mixed String key or array of keys to set values for.
 75+ * @param value mixed Value to set (optional, only in use when key is a string)
 76+ * @return bool This returns true on success, false on failure.
 77+ */
 78+ set: function( selection, value ) {
 79+ if ( $.isPlainObject( selection ) ) {
 80+ for ( var s in selection ) {
 81+ this.values[s] = selection[s];
 82+ }
 83+ return true;
 84+ } else if ( typeof selection === 'string' && value !== undefined ) {
 85+ this.values[selection] = value;
 86+ return true;
8187 }
82 - return true;
83 - } else if ( typeof selection === 'string' && value !== undefined ) {
84 - this.values[selection] = value;
85 - return true;
86 - }
87 - return false;
88 - };
 88+ return false;
 89+ },
8990
90 - /**
91 - * Checks if one or multiple keys exist.
92 - *
93 - * @param selection mixed String key or array of keys to check
94 - * @return boolean Existence of key(s)
95 - */
96 - Map.prototype.exists = function( selection ) {
97 - if ( typeof selection === 'object' ) {
98 - for ( var s = 0; s < selection.length; s++ ) {
99 - if ( !( selection[s] in this.values ) ) {
100 - return false;
 91+ /**
 92+ * Checks if one or multiple keys exist.
 93+ *
 94+ * @param selection mixed String key or array of keys to check
 95+ * @return boolean Existence of key(s)
 96+ */
 97+ exists: function( selection ) {
 98+ if ( typeof selection === 'object' ) {
 99+ for ( var s = 0; s < selection.length; s++ ) {
 100+ if ( !( selection[s] in this.values ) ) {
 101+ return false;
 102+ }
101103 }
 104+ return true;
 105+ } else {
 106+ return selection in this.values;
102107 }
103 - return true;
104 - } else {
105 - return selection in this.values;
106108 }
107109 };
108110
@@ -124,93 +126,95 @@
125127 return this;
126128 }
127129
128 - /**
129 - * Appends (does not replace) parameters for replacement to the .parameters property.
130 - *
131 - * @param parameters Array
132 - * @return Message
133 - */
134 - Message.prototype.params = function( parameters ) {
135 - for ( var i = 0; i < parameters.length; i++ ) {
136 - this.parameters.push( parameters[i] );
137 - }
138 - return this;
139 - };
 130+ Message.prototype = {
 131+ /**
 132+ * Appends (does not replace) parameters for replacement to the .parameters property.
 133+ *
 134+ * @param parameters Array
 135+ * @return Message
 136+ */
 137+ params: function( parameters ) {
 138+ for ( var i = 0; i < parameters.length; i++ ) {
 139+ this.parameters.push( parameters[i] );
 140+ }
 141+ return this;
 142+ },
140143
141 - /**
142 - * Converts message object to it's string form based on the state of format.
143 - *
144 - * @return string Message as a string in the current form or <key> if key does not exist.
145 - */
146 - Message.prototype.toString = function() {
147 - if ( !this.map.exists( this.key ) ) {
148 - // Return <key> if key does not exist
149 - return '<' + this.key + '>';
150 - }
151 - var text = this.map.get( this.key ),
152 - parameters = this.parameters;
 144+ /**
 145+ * Converts message object to it's string form based on the state of format.
 146+ *
 147+ * @return string Message as a string in the current form or <key> if key does not exist.
 148+ */
 149+ toString: function() {
 150+ if ( !this.map.exists( this.key ) ) {
 151+ // Return <key> if key does not exist
 152+ return '<' + this.key + '>';
 153+ }
 154+ var text = this.map.get( this.key ),
 155+ parameters = this.parameters;
153156
154 - text = text.replace( /\$(\d+)/g, function( string, match ) {
155 - var index = parseInt( match, 10 ) - 1;
156 - return index in parameters ? parameters[index] : '$' + match;
157 - } );
 157+ text = text.replace( /\$(\d+)/g, function( string, match ) {
 158+ var index = parseInt( match, 10 ) - 1;
 159+ return index in parameters ? parameters[index] : '$' + match;
 160+ } );
158161
159 - if ( this.format === 'plain' ) {
 162+ if ( this.format === 'plain' ) {
 163+ return text;
 164+ }
 165+ if ( this.format === 'escaped' ) {
 166+ // According to Message.php this needs {{-transformation, which is
 167+ // still todo
 168+ return mw.html.escape( text );
 169+ }
 170+
 171+ /* This should be fixed up when we have a parser
 172+ if ( this.format === 'parse' && 'language' in mw ) {
 173+ text = mw.language.parse( text );
 174+ }
 175+ */
160176 return text;
161 - }
162 - if ( this.format === 'escaped' ) {
163 - // According to Message.php this needs {{-transformation, which is
164 - // still todo
165 - return mw.html.escape( text );
166 - }
 177+ },
167178
168 - /* This should be fixed up when we have a parser
169 - if ( this.format === 'parse' && 'language' in mw ) {
170 - text = mw.language.parse( text );
171 - }
172 - */
173 - return text;
174 - };
 179+ /**
 180+ * Changes format to parse and converts message to string
 181+ *
 182+ * @return {string} String form of parsed message
 183+ */
 184+ parse: function() {
 185+ this.format = 'parse';
 186+ return this.toString();
 187+ },
175188
176 - /**
177 - * Changes format to parse and converts message to string
178 - *
179 - * @return {string} String form of parsed message
180 - */
181 - Message.prototype.parse = function() {
182 - this.format = 'parse';
183 - return this.toString();
184 - };
 189+ /**
 190+ * Changes format to plain and converts message to string
 191+ *
 192+ * @return {string} String form of plain message
 193+ */
 194+ plain: function() {
 195+ this.format = 'plain';
 196+ return this.toString();
 197+ },
185198
186 - /**
187 - * Changes format to plain and converts message to string
188 - *
189 - * @return {string} String form of plain message
190 - */
191 - Message.prototype.plain = function() {
192 - this.format = 'plain';
193 - return this.toString();
194 - };
 199+ /**
 200+ * Changes the format to html escaped and converts message to string
 201+ *
 202+ * @return {string} String form of html escaped message
 203+ */
 204+ escaped: function() {
 205+ this.format = 'escaped';
 206+ return this.toString();
 207+ },
195208
196 - /**
197 - * Changes the format to html escaped and converts message to string
198 - *
199 - * @return {string} String form of html escaped message
200 - */
201 - Message.prototype.escaped = function() {
202 - this.format = 'escaped';
203 - return this.toString();
 209+ /**
 210+ * Checks if message exists
 211+ *
 212+ * @return {string} String form of parsed message
 213+ */
 214+ exists: function() {
 215+ return this.map.exists( this.key );
 216+ }
204217 };
205218
206 - /**
207 - * Checks if message exists
208 - *
209 - * @return {string} String form of parsed message
210 - */
211 - Message.prototype.exists = function() {
212 - return this.map.exists( this.key );
213 - };
214 -
215219 /* Public Members */
216220
217221 /*
@@ -360,7 +364,7 @@
361365 return [a < 10 ? '0' + a : a, b < 10 ? '0' + b : b, c < 10 ? '0' + c : c].join( '' );
362366 },
363367 d = new Date();
364 - d.setTime( timestamp * 1000 );
 368+ d.setTime( timestamp * 1000 );
365369 return [
366370 pad( d.getUTCFullYear(), d.getUTCMonth() + 1, d.getUTCDate() ), 'T',
367371 pad( d.getUTCHours(), d.getUTCMinutes(), d.getUTCSeconds() ), 'Z'
@@ -501,11 +505,10 @@
502506 } ) );
503507 }
504508 } else if ( typeof style === 'string' ) {
505 - getMarker().before( mw.html.element(
506 - 'style',
507 - { 'type': 'text/css', 'media': media },
508 - new mw.html.Cdata( style )
509 - ) );
 509+ getMarker().before( mw.html.element( 'style', {
 510+ 'type': 'text/css',
 511+ 'media': media
 512+ }, new mw.html.Cdata( style ) ) );
510513 }
511514 }
512515 }
@@ -673,12 +676,12 @@
674677 * @param callback Function: Optional callback which will be run when the script is done
675678 */
676679 function addScript( src, callback ) {
 680+ var done = false, script;
677681 if ( ready ) {
678682 // jQuery's getScript method is NOT better than doing this the old-fassioned way
679683 // because jQuery will eval the script's code, and errors will not have sane
680684 // line numbers.
681 - var done = false,
682 - script = document.createElement( 'script' );
 685+ script = document.createElement( 'script' );
683686 script.setAttribute( 'src', src );
684687 script.setAttribute( 'type', 'text/javascript' );
685688 if ( $.isFunction( callback ) ) {
@@ -1163,8 +1166,7 @@
11641167 // Alias $j to jQuery for backwards compatibility
11651168 window.$j = jQuery;
11661169
1167 -/* Auto-register from pre-loaded startup scripts */
1168 -
 1170+// Auto-register from pre-loaded startup scripts
11691171 if ( jQuery.isFunction( startUp ) ) {
11701172 startUp();
11711173 delete startUp;

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r92933Minor mw.loader fixes...krinkle09:09, 23 July 2011
r92964fu r92933 - fix breakagejeroendedauw20:56, 23 July 2011

Status & tagging log