Skip to content
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

Make jQuery.trim consistent with ECMAScript 5 String.prototype.trim cross browser. #896

Closed
wants to merge 1 commit into from

Conversation

wwalser
Copy link

@wwalser wwalser commented Aug 16, 2012

This adds 41 bytes when gzipped.

There is also a bit of history around the trim function and the way it's done. Most recently 26bdbb8 and back in 1.4.3 there was this http://forum.jquery.com/topic/faster-jquery-trim#14737000000647377.

See http://jsfiddle.net/MHzar/1/ in Chrome or Firefox then in IE8 for a difference.


Section 15.5.4.20 of ECMAScript 5 standard, definition of String.prototype.trim()
15.5.4.20 String.prototype.trim ( )

3. Let T be a String value that is a copy of S with both leading and trailing white space removed. The definition
of white space is the union of WhiteSpace and LineTerminator.

Section 7.2 defines White Space as

Code                  Unit                                  Value
\u0009 -              Tab -                                 <TAB>
\u000B -              Vertical Tab -                        <VT>
\u000C -              Form Feed -                           <FF>
\u0020 -              Space -                               <SP>
\u00A0 -              No-break space -                      <NBSP>
\uFEFF -              Byte Order Mark -                     <BOM>
Other category (Zs) - Any other Unicode "space separator" - <USP>

Note the bit about "Any other Unicode 'space separator'".
Unicode Zs or Space Separators (http://www.fileformat.info/info/unicode/category/Zs/list.htm)
\uFEFF
\u1680
\u180E
\u2000-\u200A
\u202F
\u205F
\u3000

@dmethvin
Copy link
Member

I appreciate strict adherence to standards as much as the next guy, but jQuery's internal utility methods are often intended for specific situations inside the library where the complex edge cases don't matter. If people absolutely need the ES5 behavior they should be using a shim. Do you have some cases where jQuery is internally using $.trim and doing the wrong thing when trimming strings?

@wwalser
Copy link
Author

wwalser commented Aug 16, 2012

http://api.jquery.com/jQuery.trim/

It's a publicly documented API so any web application that pulls in jQuery shouldn't feel the need to implement a trim that works properly themselves. It seems appropriate that a publicly documented API of jQuery which defers to String.prototype.trim would normalize cross browser behavior in browsers where trim doesn't work natively.

I didn't implement this out of a desire for strict adherence to standards, but because it's bitten a team I work with multiple times recently. The areas that it's been problematic has been specifically related to Byte Order Marks which all of the Content Editable libraries (TinyMCE, CKEditor, Aloha etc.) use internally as a zero width white space when they need to mark various things.

At the end of the day I don't mind one way or the other, just an annoyance that's cropped up multiple times for me recently. I understand your desire to keep file size small, everything counts.

@dmethvin
Copy link
Member

You're defining "works properly" to be a full shim implementation of String.prototype.trim, that's not what the docs say $.trim does. It's pretty darn ambiguous if you ask me. If you've run into just the BOM problem, then make a case to solve just that problem. It would probably take a lot fewer than 41 gzipped bytes, right? I'm not opposed to landing something here, we had a similar PR in gh-722 but the author went dark on us.

@wwalser
Copy link
Author

wwalser commented Aug 17, 2012

Not sure if you can change which branch a pull request is based on. I opened #897.

Cheers,
Wes

@wwalser wwalser closed this Aug 17, 2012
@mplungjan
Copy link

@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.

None yet

3 participants