Closed Bug 612838 Opened 14 years ago Closed 14 years ago

indexOf gives wrong result for empty string and position > length

Categories

(Core :: JavaScript Engine, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla2.0b8
Tracking Status
blocking2.0 --- beta9+

People

(Reporter: jandem, Assigned: jandem)

References

Details

Attachments

(1 file)

Consider the following code with SM output:
"123".indexOf("", 0) result: 0
"123".indexOf("", 2) result: 2
"123".indexOf("", 3) result: 3
"123".indexOf("", 4) result: 0

I think the last one should also return 3, like in Chrome, Safari and Opera.
blocking2.0: --- → ?
Note that this also fails in Firefox 3.6.
Assignee: general → jandemooij
Status: NEW → ASSIGNED
Attached patch PatchSplinter Review
This fixes the problem, passes tests and adds tests. brendan can you review or should i ask someone else?
Attachment #491155 - Flags: review?(brendan)
Looks like a regression from bug 511777, the old code also used textlen in this case.
Blocks: 511777
The "regression" I believe was actually a bugfix: 15.5.4.7 step 5 says that, if the start position is greater than the length of the string, it is clamped the length of the string, not set to 0.  I wonder if the other three mimicked our old, incorrect behavior.
(In reply to comment #4)
> The "regression" I believe was actually a bugfix: 15.5.4.7 step 5 says that, if
> the start position is greater than the length of the string, it is clamped the
> length of the string, not set to 0.

Well the problem is that it's set to 0. This patch clamps it to the length of the string, so that it returns 3 for the example in comment 0.
Oh goodness, I read the patch backwards, excuse me ;)
Comment on attachment 491155 [details] [diff] [review]
Patch

Jan, could you please add this to your .hgrc?

[diff]
git = 1
showfunc = true
unified = 8

It looks like you don't have showfunc = true at least.

Patch is fine, thanks a lot -- good to have test coverage gap filler too.

/be
Attachment #491155 - Flags: review?(brendan) → review+
(In reply to comment #7)
> Jan, could you please add this to your .hgrc? 

Cool, done.
Keywords: checkin-needed
blocking2.0: ? → beta9+
http://hg.mozilla.org/mozilla-central/rev/13f5b057c586

Please include the bug number in your patch comments in the future.  I failed to catch this, so right now there is no way to link back from the landed patch to this bug. :(
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Flags: in-testsuite+
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b8
(In reply to comment #9)
> Please include the bug number in your patch comments in the future.  I failed
> to catch this, so right now there is no way to link back from the landed patch
> to this bug. :(

OK, will do that next time; thanks for committing this.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: