r75294 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r75293‎ | r75294 | r75295 >
Date:21:08, 23 October 2010
Author:krinkle
Status:resolved (Comments)
Tags:
Comment:
added string trimming for older browsers
Modified paths:
  • /trunk/phase3/resources/mediawiki/mediawiki.js (modified) (history)
  • /trunk/phase3/resources/mediawiki/mediawiki.util.js (modified) (history)

Diff [purge]

Index: trunk/phase3/resources/mediawiki/mediawiki.util.js
@@ -1,4 +1,3 @@
2 -/*jslint white: true, browser: true, nomen: true, eqeqeq: true, plusplus: true, bitwise: true, regexp: true, newcap: true */
32 /*
43 * Utilities
54 */
@@ -44,6 +43,13 @@
4544 return $box;
4645 };
4746
 47+ // Prototype enhancements
 48+ if (typeof String.prototype.ucFirst === 'undefined') {
 49+ String.prototype.ucFirst = function () {
 50+ return this.substr(0, 1).toUpperCase() + this.substr(1, this.length);
 51+ };
 52+ }
 53+
4854 // Any initialisation after the DOM is ready
4955 $(function () {
5056 $('input[type=checkbox]:not(.noshiftselect)').enableCheckboxShiftClick();
@@ -88,7 +94,30 @@
8995 return wgServer + wgArticlePath.replace('$1', this.wikiUrlencode(str));
9096 },
9197
 98+ /**
 99+ * Check is a variable is empty. Support for strings, booleans, arrays and objects.
 100+ * String "0" is considered empty. String containing only whitespace (ie. " ") is considered not empty.
 101+ *
 102+ * @param Mixed v the variable to check for empty ness
 103+ */
 104+ 'isEmpty' : function (v) {
 105+ var key;
 106+ if (v === "" || v === 0 || v === "0" || v === null || v === false || typeof v === 'undefined') {
 107+ return true;
 108+ }
 109+ if (v.length === 0) {
 110+ return true;
 111+ }
 112+ if (typeof v === 'object') {
 113+ for (key in v) {
 114+ return false;
 115+ }
 116+ return true;
 117+ }
 118+ return false;
 119+ },
92120
 121+
93122 /**
94123 * Grabs the url parameter value for the given parameter
95124 * Returns null if not found
Index: trunk/phase3/resources/mediawiki/mediawiki.js
@@ -2,30 +2,39 @@
33 * JavaScript backwards-compatibility and support
44 */
55
6 -// Make calling .indexOf() on an array work on older browsers
7 -if ( typeof Array.prototype.indexOf === 'undefined' ) {
8 - Array.prototype.indexOf = function( needle ) {
9 - for ( var i = 0; i < this.length; i++ ) {
10 - if ( this[i] === needle ) {
11 - return i;
12 - }
13 - }
14 - return -1;
 6+// New fallback String trimming functionality, was introduced natively in JavaScript 1.8.1
 7+if (typeof String.prototype.trimx === 'undefined') {
 8+// Add removing trailing and leading whitespace functionality cross-browser
 9+// See also: http://blog.stevenlevithan.com/archives/faster-trim-javascript
 10+ String.prototype.trim = function () {
 11+ return this.replace(/^\s+|\s+$/g, '');
1512 };
1613 }
 14+if (typeof String.prototype.trimLeft === 'undefined') {
 15+ String.prototype.trimLeft = function () {
 16+ return this.replace(/^\s\s*/, "");
 17+ };
 18+}
 19+
 20+if (typeof String.prototype.trimRight === 'undefined') {
 21+ String.prototype.trimRight = function () {
 22+ return this.replace(/\s\s*$/, "");
 23+ };
 24+}
 25+
1726 // Add array comparison functionality
18 -if ( typeof Array.prototype.compare === 'undefined' ) {
19 - Array.prototype.compare = function( against ) {
20 - if ( this.length != against.length ) {
 27+if (typeof Array.prototype.compare === 'undefined') {
 28+ Array.prototype.compare = function (against) {
 29+ if (this.length !== against.length) {
2130 return false;
2231 }
23 - for ( var i = 0; i < against.length; i++ ) {
24 - if ( this[i].compare ) {
25 - if ( !this[i].compare( against[i] ) ) {
 32+ for (var i = 0; i < against.length; i++) {
 33+ if (this[i].compare) {
 34+ if (!this[i].compare(against[i])) {
2635 return false;
2736 }
2837 }
29 - if ( this[i] !== against[i] ) {
 38+ if (this[i] !== against[i]) {
3039 return false;
3140 }
3241 }
@@ -33,6 +42,18 @@
3443 };
3544 }
3645
 46+// Make calling .indexOf() on an array work on older browsers
 47+if (typeof Array.prototype.indexOf === 'undefined') {
 48+ Array.prototype.indexOf = function (needle) {
 49+ for (var i = 0; i < this.length; i++) {
 50+ if (this[i] === needle) {
 51+ return i;
 52+ }
 53+ }
 54+ return -1;
 55+ };
 56+}
 57+
3758 /*
3859 * Core MediaWiki JavaScript Library
3960 */

Follow-up revisions

RevisionCommit summaryAuthorDate
r76320As per r 75486 CR comments, no prototyping in mw core....krinkle18:13, 8 November 2010

Comments

#Comment by Catrope (talk | contribs)   12:52, 27 October 2010
+if (typeof String.prototype.trimx === 'undefined') {
+	String.prototype.trim = function () {

Typo in trimx vs. trim?

+				// Prototype enhancements
+				if (typeof String.prototype.ucFirst === 'undefined') {
+					String.prototype.ucFirst = function () {
+						return this.substr(0, 1).toUpperCase() + this.substr(1, this.length);
+					};
+				}

Why put this in util while all other prototype enhancements are in mediawiki.js?

-if ( typeof Array.prototype.compare === 'undefined' ) {
-	Array.prototype.compare = function( against ) {
-		if ( this.length != against.length ) {
+if (typeof Array.prototype.compare === 'undefined') {
+	Array.prototype.compare = function (against) {
+		if (this.length !== against.length) {

It'd be nice if you didn't replace "correctly" indented code with "wrongly" indented code.

#Comment by Catrope (talk | contribs)   12:55, 27 October 2010

trimx/trim fixed in r75325.

#Comment by Krinkle (talk | contribs)   14:21, 27 October 2010

Sorry about the trimx, I forgot to undo that while debugging (fixed in the next commit as you mentioned) About the indention, I wanted to ask about that. Is there a guideline somewhere ? I use JSLint's 'strict whitepace' setting with indenting set to 'tab'.

The reason I had the prototype for ucFirst in mw.util is because it's not a fallback for older browsers but a utility not present in any browser by default. But I see no problem in keeping them all together in mediawiki.js either, I'll put it there from now on (and move this one).

Thanks for the feedback.

#Comment by Catrope (talk | contribs)   14:22, 27 October 2010

The whitespace convention is not really defined anywhere, but follows the spirit of MediaWiki's PHP coding style guidelines, see Manual:Coding conventions.

#Comment by Krinkle (talk | contribs)   15:40, 27 October 2010

Moved prototypes to mediawiki.js in r75552

Status & tagging log