Side navigation
#13251 closed bug (fixed)
Opened January 17, 2013 05:34PM UTC
Closed January 18, 2013 01:14AM UTC
Last modified January 18, 2013 06:24PM UTC
substring() in v1.9
| Reported by: | Nao | Owned by: | |
|---|---|---|---|
| Priority: | low | Milestone: | 1.next |
| Component: | unfiled | Version: | 1.9.0 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Hey there,
I noticed that jQuery 1.9 (and 1.8, maybe earlier, not in 1.5 though) has this line:
name = jQuery.camelCase( name.substring(5) );
Given that with a single argument given, substring() behaves the same as substr(), wouldn't it be better to use substr(), which is already used twice in other areas of jQuery? This would ensure optimal compression...
Very minor thing, I know, but a byte is a byte :)
PS: also noticed the copyright date in jquery-1.9.js is 2012, instead of 2013. I suppose someone will notice at some point anyway!
Attachments (0)
Change History (5)
Changed January 17, 2013 05:51PM UTC by comment:1
| status: | new → open |
|---|
Changed January 18, 2013 01:14AM UTC by comment:2
| milestone: | None → 1.next |
|---|---|
| priority: | undecided → low |
| resolution: | → fixed |
| status: | open → closed |
Changed January 18, 2013 04:10PM UTC by comment:3
Thank you for committing the fix :)
I looked at it and saw you replaced with a .slice(), which makes more sense now that I've read the MDN page for that, eh.
While I'm at it then -- why not change the two remaining .substr() calls in jQuery to .slice()?
They will both accept the change without any issues, there's not even the need to update parameters. Actually, if I'm not mistaken, "''result.substr( result.length - check.length )''" could be shortened to "''result.slice( -check.length )''", but I don't know if it compresses as well.
Changed January 18, 2013 04:58PM UTC by comment:4
The referenced commits include Sizzle updates that address the remaining substr calls, and in fact the subtraction operation was replaced with a negative index.
Changed January 18, 2013 06:24PM UTC by comment:5
Ah... Yes indeed! Sorry, I didn't realize Sizzle was in its own repo ;)
https://github.com/jquery/sizzle/commit/200c8724121819f4a6b1f1d155ef9361d0a27518
Then all is good, thanks again for your time!
Agreed on both points, thanks!