Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from

Conversation

gibson042
Copy link
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

// 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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Member Author

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
Copy link
Member

Yeah, this is a lot more direct.

@@ -605,7 +605,7 @@ jQuery.extend({
},

// Use native String.trim function wherever possible
trim: core_trim ?
trim: core_trim && !core_trim.call("\uFEFF\xA0") ?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gibson042 this is really nice :)

@dmethvin
Copy link
Member

Landed in 9e246dd .

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

\o/

@lock lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants