Fix #12350: jQuery.trim should remove BOM #902

Closed
wants to merge 2 commits into
from

Projects

None yet

4 participants

@gibson042
Member

Now with Safari 5.0 support!

Sizes - compared to master @ ac043b1

    258567       (+41)  dist/jquery.js                                         
     92624       (+12)  dist/jquery.min.js                                     
     33133        (+3)  dist/jquery.min.js.gz
@gibson042 gibson042 commented on an outdated diff Aug 20, 2012
@@ -37,8 +37,8 @@ var
core_rnotwhite = /\S/,
core_rspace = /\s+/,
- // IE doesn't match non-breaking spaces with \s
- rtrim = core_rnotwhite.test("\xA0") ? (/^[\s\xA0]+|[\s\xA0]+$/g) : /^\s+|\s+$/g,
+ // Make sure we trim BOM and NBSP (here's looking at you, Safari 5.0 and IE)
+ rtrim = core_rnotwhite.test("\uFEFF") ? /^[\s\xA0\uFEFF]+|[\s\xA0\uFEFF]+$/g : /^\s+|\s+$/g,
@gibson042
gibson042 Aug 20, 2012 Member

This should work in all supported browsers. Testing against "\xA0\uFEFF" would be more correct technically, but cost 4 bytes gzipped.

Cue someone showing an implementation recognizing BOM but not NBSP...

@gibson042
Member

Updated at the behest of my conscience (a.k.a. @jaubourg). There's really no reason to conditionally define rtrim now that we can't trust String.prototype.trim.

@dmethvin
Member

Yeah, this is a lot more direct.

@rwaldron rwaldron commented on the diff Aug 21, 2012
src/core.js
@@ -605,7 +605,7 @@ jQuery.extend({
},
// Use native String.trim function wherever possible
- trim: core_trim ?
+ trim: core_trim && !core_trim.call("\uFEFF\xA0") ?
@rwaldron
rwaldron Aug 21, 2012 Member

@gibson042 this is really nice :)

@dmethvin
Member

Landed in 9e246dd .

@dmethvin dmethvin closed this Aug 21, 2012
@jaubourg
Member

\o/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment