Skip to main content

Bug Tracker

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:
Component: unfiled Version: 1.9.0
Keywords: Cc:
Blocked by: Blocking:

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 dmethvin comment:1

status: newopen

Agreed on both points, thanks!

Changed January 18, 2013 01:14AM UTC by gibson042 comment:2

priority: undecidedlow
resolution: → fixed
status: openclosed

Changed January 18, 2013 04:10PM UTC by Nao 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 gibson042 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 Nao comment:5

Ah... Yes indeed! Sorry, I didn't realize Sizzle was in its own repo ;)

Then all is good, thanks again for your time!