Permalink
Browse files

Landing a faster trim method. Based upon the work by Travis Hardiman …

…and DBJDBJ. More details here: http://forum.jquery.com/topic/faster-jquery-trim Fixes #2279, #4452, and #4835.
  • Loading branch information...
jeresig committed Mar 9, 2010
1 parent 0a307b3 commit 141ad3c3e21e7734e67e37b5fb39782fe11b3c18
Showing with 36 additions and 24 deletions.
  1. +23 −4 src/core.js
  2. +1 −14 src/support.js
  3. +12 −6 test/unit/core.js
View
@@ -27,7 +27,8 @@ var jQuery = function( selector, context ) {
rnotwhite = /\S/,
// Used for trimming whitespace
- rtrim = /^(\s|\u00A0)+|(\s|\u00A0)+$/g,
+ trimLeft = /^\s+/,
+ trimRight = /\s+$/,
// Match a standalone tag
rsingleTag = /^<(\w+)\s*\/?>(?:<\/\1>)?$/,
@@ -567,9 +568,20 @@ jQuery.extend({
return object;
},
- trim: function( text ) {
- return (text || "").replace( rtrim, "" );
- },
+ // Use native String.trim function wherever possible
+ trim: String.trim ?
+ function( text ) {
+ return text == null ?
+ "" :
+ String.trim( text );
+ } :
+
+ // Otherwise use our own trimming functionality
+ function( text ) {
+ return text == null ?
+ "" :
+ text.toString().replace( trimLeft, "" ).replace( trimRight, "" );
+ },
// results is for internal usage only
makeArray: function( array, results ) {
@@ -720,6 +732,13 @@ if ( indexOf ) {
};
}
+// Verify that \s matches non-breaking spaces
+// (IE fails on this test)
+if ( !/\s/.test( "\xA0" ) ) {
+ trimLeft = /^[\s\xA0]+/;
+ trimRight = /[\s\xA0]+$/;
+}
+
// All jQuery objects should point back to these
rootjQuery = jQuery(document);
View
@@ -56,10 +56,7 @@
// (WebKit defaults to false instead of true, IE too, if it's in an optgroup)
optSelected: document.createElement("select").appendChild( document.createElement("option") ).selected,
- parentNode: div.removeChild( div.appendChild( document.createElement("div") ) ).parentNode === null,
-
// Will be defined later
- deleteExpando: true,

This comment has been minimized.

Show comment Hide comment
@jitter

jitter Oct 16, 2010

Contributor

Looks like the deleteExpando property got removed by accident. Check also bug #7209

@jitter

jitter Oct 16, 2010

Contributor

Looks like the deleteExpando property got removed by accident. Check also bug #7209

checkClone: false,
scriptEval: false,
noCloneEvent: true,
@@ -69,7 +66,7 @@
script.type = "text/javascript";
try {
script.appendChild( document.createTextNode( "window." + id + "=1;" ) );
- } catch( scriptError ) {}
+ } catch(e) {}
root.insertBefore( script, root.firstChild );
@@ -81,15 +78,6 @@
delete window[ id ];
}
- // Test to see if it's possible to delete an expando from an element
- // Fails in Internet Explorer
- try {
- delete script.test;
-
- } catch( expandoError ) {
- jQuery.support.deleteExpando = false;
- }
-
root.removeChild( script );
if ( div.attachEvent && div.fireEvent ) {
@@ -120,7 +108,6 @@
document.body.appendChild( div );
jQuery.boxModel = jQuery.support.boxModel = div.offsetWidth === 2;
document.body.removeChild( div ).style.display = 'none';
-
div = null;
});
View
@@ -201,14 +201,20 @@ test("noConflict", function() {
});
test("trim", function() {
- expect(4);
+ expect(9);
- var nbsp = String.fromCharCode(160);
+ var nbsp = String.fromCharCode(160);
- equals( jQuery.trim("hello "), "hello", "trailing space" );
- equals( jQuery.trim(" hello"), "hello", "leading space" );
- equals( jQuery.trim(" hello "), "hello", "space on both sides" );
- equals( jQuery.trim(" " + nbsp + "hello " + nbsp + " "), "hello", "&nbsp;" );
+ equals( jQuery.trim("hello "), "hello", "trailing space" );
+ equals( jQuery.trim(" hello"), "hello", "leading space" );
+ equals( jQuery.trim(" hello "), "hello", "space on both sides" );
+ equals( jQuery.trim(" " + nbsp + "hello " + nbsp + " "), "hello", "&nbsp;" );
+
+ equals( jQuery.trim(), "", "Nothing in." );
+ equals( jQuery.trim( undefined ), "", "Undefined" );
+ equals( jQuery.trim( null ), "", "Null" );
+ equals( jQuery.trim( 5 ), "5", "Number" );
+ equals( jQuery.trim( false ), "false", "Boolean" );
});
test("isPlainObject", function() {

6 comments on commit 141ad3c

@DBJDBJ

This comment has been minimized.

Show comment Hide comment
@DBJDBJ

DBJDBJ Mar 10, 2010

@john : thanks for commendation ;o)

one note ? to avoid all browsers ES5 transitional period pains, I would rather test for trim() existence like so :

 var native_trim = "function" === typeof "".trim ;

Some browsers support and implement String.prototype.trim, but not String.trim, I think ?

Thanks: DBJ

@john : thanks for commendation ;o)

one note ? to avoid all browsers ES5 transitional period pains, I would rather test for trim() existence like so :

 var native_trim = "function" === typeof "".trim ;

Some browsers support and implement String.prototype.trim, but not String.trim, I think ?

Thanks: DBJ

@DBJDBJ

This comment has been minimized.

Show comment Hide comment
@DBJDBJ

DBJDBJ Mar 10, 2010

Also , I seem to remember there is a trim left and/or trim right somewhere in jQuery which now can be changed to use trimLeft ot trimRight , regular expressions ?

Thanks: Dusan

Also , I seem to remember there is a trim left and/or trim right somewhere in jQuery which now can be changed to use trimLeft ot trimRight , regular expressions ?

Thanks: Dusan

@jeresig

This comment has been minimized.

Show comment Hide comment
@jeresig

jeresig Mar 10, 2010

Member

@DBJDBJ: I'm not sure what you mean regarding String.prototype.trim - we don't use it, we're only using the native String.trim method, so it's best for us to check to see if that method exists. Also, there is no left/right trim in jQuery - only the one trim.

Member

jeresig replied Mar 10, 2010

@DBJDBJ: I'm not sure what you mean regarding String.prototype.trim - we don't use it, we're only using the native String.trim method, so it's best for us to check to see if that method exists. Also, there is no left/right trim in jQuery - only the one trim.

@DBJDBJ

This comment has been minimized.

Show comment Hide comment
@DBJDBJ

DBJDBJ Mar 10, 2010

Sorry I was to busy for a comprehensive comment.

" .... Also, there is no left/right trim in jQuery - only the one trim....

inside jQuery.extend we have this comment :
// Trim whitespace, otherwise indexOf won't work as expected
And this declaration :
rleadingWhitespace = /^\s+/,
Which now obviously should be removed and (tne new and correct)
trimLeft
Should be used, I think ?

Second. String.prototype.trim ---- v.s.--- String.trim
15.5.4.20 String.prototype.trim ( )
The above is the only legal ES5 , trim() . There is no String.trim() in ES5. This is why I would rather not use it ....

Thanks: DBJ

Sorry I was to busy for a comprehensive comment.

" .... Also, there is no left/right trim in jQuery - only the one trim....

inside jQuery.extend we have this comment :
// Trim whitespace, otherwise indexOf won't work as expected
And this declaration :
rleadingWhitespace = /^\s+/,
Which now obviously should be removed and (tne new and correct)
trimLeft
Should be used, I think ?

Second. String.prototype.trim ---- v.s.--- String.trim
15.5.4.20 String.prototype.trim ( )
The above is the only legal ES5 , trim() . There is no String.trim() in ES5. This is why I would rather not use it ....

Thanks: DBJ

@DBJDBJ

This comment has been minimized.

Show comment Hide comment
@DBJDBJ

DBJDBJ Mar 10, 2010

Results from http://jsbin.com/ehoje/12
Opera
userAgent :Opera/9.80 (Windows NT 6.1; U; en) Presto/2.6.22 Version/10.50
String.trim() → undefined
String.protoype.trim() → function trim() { [native code] }
Safari
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/531.21.8 (KHTML, like Gecko) Version/4.0.4 Safari/531.21.10
String.trim() → undefined
String.protoype.trim() → undefined
Chrome
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/533.2 (KHTML, like Gecko) Chrome/5.0.342.2 Safari/533.2
String.trim() → undefined
String.protoype.trim() → function trim() { [native code] }
FireFox
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2) Gecko/20100115 Firefox/3.6
String.trim() → function trim() { [native code] }
String.protoype.trim() → function trim() { [native code] }
With IE I have not bothered ...

I really think we should not rely on String.trim() , being universaly avaialble

Thanks: DBJ

Results from http://jsbin.com/ehoje/12
Opera
userAgent :Opera/9.80 (Windows NT 6.1; U; en) Presto/2.6.22 Version/10.50
String.trim() → undefined
String.protoype.trim() → function trim() { [native code] }
Safari
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/531.21.8 (KHTML, like Gecko) Version/4.0.4 Safari/531.21.10
String.trim() → undefined
String.protoype.trim() → undefined
Chrome
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US) AppleWebKit/533.2 (KHTML, like Gecko) Chrome/5.0.342.2 Safari/533.2
String.trim() → undefined
String.protoype.trim() → function trim() { [native code] }
FireFox
userAgent :Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:1.9.2) Gecko/20100115 Firefox/3.6
String.trim() → function trim() { [native code] }
String.protoype.trim() → function trim() { [native code] }
With IE I have not bothered ...

I really think we should not rely on String.trim() , being universaly avaialble

Thanks: DBJ

@DBJDBJ

This comment has been minimized.

Show comment Hide comment
@DBJDBJ

DBJDBJ Mar 11, 2010

I hope it is not patronising if I offer this : http://gist.github.com/329172

Thanks : DBJ

I hope it is not patronising if I offer this : http://gist.github.com/329172

Thanks : DBJ

Please sign in to comment.