189898 - Broken String.replace: "XaXY".replace("XY", "--") gives --aXY

Status: VERIFIED FIXED

(Rhino Graveyard :: Core, defect)

Description

[Igor Bukanov]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.0.1) Gecko/20021003
Build Identifier: The third release candidadte of Rhino 1.5R4

The following scripts demonstartes the problem:

var result = "XaXY".replace("XY", "--");	

var expect = "Xa--"

var ok = result == expect;

if (ok) {
	print("OK");
} else {
	print("FAILED: expected='"+expect+"', result='"+result+"'");
}


It looks like replace in general always match the first character if it starts
the match string

Reproducible: Always

Steps to Reproduce:
Run the above script in the rhino shell
Actual Results:  
FAILED: expected='Xa--', result='--aXY'


Expected Results:  
OK

Comment 1

[Igor Bukanov]

CC to Roger, this is a regression since 1.5R3

Comment 2

[Igor Bukanov]

Shorter title

Comment 3

[Phil Schwartau]

Confirming and reassigning to Roger.

CURRENT SPIDERMONKEY (as is; without patch for bug 85721)
js> "XaXY".replace("XY", "--")
Xa--

CURRENT RHINO
js> "XaXY".replace("XY", "--")
--aXY

and yet:

js> "XaXY".search("XY")
2

Comment 4

[Phil Schwartau]

FWIW:

Note there is an important difference between the replace() method
and the search() and match() methods of String.prototype, when the
first parameter provided is a string and not a regexp. 

This was a last-minute change made in the ECMA-262 Edition 3 spec.
As late as the Final Draft for Edition 3, the spec used to require
these to be equivalent:

   str.replace(strA, strB) == str.replace(new RegExp(strA),strB)


However, in bug 83293 Jim Ley pointed out that ECMA-262 Edition 3 Final 
changed on this. String.prototype.replace (searchValue, replaceValue),
if provided a searchValue that is not a RegExp, is NOT to replace it with

                    new RegExp(searchValue)
but rather:
                        String(searchValue)


This puts the replace() method at variance with search() and match(),
which continue to follow the RegExp conversion of the Final Draft.

See ECMA-262 Edition 3 Final, 15.5.4.11 String.prototype.replace

This is of relevance if strings contain any regexp meta-characters
like '$', '^', etc.  If such a string is provided as a first parameter
to the replace() method, these characters are to be interpreted as 
string literals, unlike the case for search() and match().

Finally, note the place to get the ECMA-262 Edition 3 Final is at
http://www.mozilla.org/js/language (also note the list of errata).
I am told that the version at the ECMA website itself is not current!

Comment 5

[Phil Schwartau]

Testcase added to JS testsuite:

      mozilla/js/tests/ecma_3/String/regress-189898.js

Currently passing in SpiderMonkey, failing in Rhino -

Comment 6

[Igor Bukanov]

Note for String.prototype.replace semantics in MSIE:

www.microsoft.com contains JavaScript with expressions like:

<html-fragment1>.replace(<html-fragment2>, <html-fragment3>) 

where <html-fragment2> contains at least . and ? and often JavaScript fragments
from even handlers with +, [] etc.. So it is essential there that
<html-fragment2> is treated as a string, not RegExp.

Comment 7

[rogerl (gone)]

The REOP_FLAT node being generated for the 'flat' entry-point used by replace
was constructed wrong. I also added error checks for bad quantifiers from
SpiderMonkey bug 188206

Comment 8

[Phil Schwartau]

Verified FIXED.

The testcase for this bug is now passing in both the interpreted
and compiled modes of Rhino:

      mozilla/js/tests/ecma_3/String/regress-189898.js


Furthermore, as Roger indicated, Rhino is also now passing the
testcase for bug 188206:

      mozilla/js/tests/ecma_3/RegExp/regress-188206.js

This test deliberately misuses regexp quantifiers and checks
for SyntaxErrors to result. This is now working in Rhino -

Comment 9

[Igor Bukanov]

Yes, now www.microsoft.com works with the Rhino CVS version in our browser.

Comment 10

[Simon Paquet [:sipaq]]

Status CLOSED is deprecated as per bug 169885

Comment 11

[Simon Paquet [:sipaq]]

Re-Resolving

Comment 12

[Simon Paquet [:sipaq]]

verifying former CLOSED bugs

Comment 13

[Igor Bukanov]

Trageting as resolved against 1.5R5

Comment 14

[Igor Bukanov]

Fixing wrong milestone assignment