103351 - 'abc'.replace(undefined, 'Z') should be 'abc', not 'Zabc'

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Phil Schwartau]

This bug grew out of bug 83293. 

Bug 83293 addressed this: 'abc'.replace('', 'Z') should be 'Zabc'
SpiderMonkey gave no return.

Here we consider this:    'abc'.replace(undefined, 'Z') should be 'abc'
SpiderMonkey returns 'Zabc'

Both bugs derive from ECMA-262 Final Edition, Section 15.5.4.11
String.prototype.replace (searchValue, replaceValue)  

The Final Edition states that searchValue, if not of type RegExp,
should be converted to String(searchValue), and the replace() method
should then search for the first occurrence of this string and replace it.

So, since String(undefined) == 'undefined', and there is no
occurrence of 'undefined' in the string 'abc', we should have:

             'abc'.replace(undefined, 'Z') == 'abc'

Comment 1

[Phil Schwartau]

Note: the ECMA Final Edition differs vis-a-vis the ECMA Final Draft on the
syntax for the replace() method, and is inconsistent with the Final Edition
syntax for the related methods search(), match() of String.prototype
in the case where searchValue is not of type RegExp. See bug 83293 for details.

The testcase for this bug is

              js/tests/ecma_3/String/regress-83293.js

Comment 2

[Kenton Hanson (gone)]

Retargeting for 9.8.

Comment 3

[Kenton Hanson (gone)]

targeting 9.9.

Comment 4

[rogerl (gone)]

Attachment: proposed

Hoping kenton doesn't mind the intrusion, I was working on related stuff and
believe simply taking out the special handling for 'undefined' will do the
job...

Comment 5

[Brendan Eich [:brendan]]

Comment on attachment 68823 [details] [diff] [review]
proposed

Patch is not up to date with respect to the cvs trunk, but no worries -- just
update before you commit, no need for another patch.  sr=[e-mail].

/be

Comment 6

[Kenton Hanson (gone)]

Comment on attachment 68823 [details] [diff] [review]
proposed

r=khanson

Comment 7

[rogerl (gone)]

Fix checked in.

Comment 8

[Phil Schwartau]

Marking Verified FIXED. The testcase above, ecma_3/String/regress-83293.js,
now passes in the debug/optimized JS shell on WinNT, Linux, and Mac9.1.