Bug 9234 – Implement $&, $' and $` replacement codes in String.prototype.replace

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

According to Ecma-262 15.5.4.11, if replaceValue is a string, then $& should be replaced with the matched substring, $` with everything preceding the matched substring, and $' with everything following the matched substring. For example,

"abcdefg".replace(/d(e)/, "[$&]") == "abc[de]fg" (currently achievable using $0)
"abcdefg".replace(/d(e)/, "[$`]") == "abc[abc]fg"
"abcdefg".replace(/d(e)/, "[$']") == "abc[fg]fg"

Created attachment 8665 [details]
Patch without test case and change log

This passes all JavaScriptCore tests and WebCore layout test, but I didn't know what kind of test to attach to it. Does it need to be a JSC test?

[Geoffrey Garen]

Comment on attachment 8665 [details]
Patch without test case and change log

for (int i = 0; (i = substitutedReplacement.find(UString("$"), i)) != -1; i++) {

This line confused the dickens out of me, because 'i = 0' has no meaning, and 'i++' has meaning only as a side-effect in the test that follows it. I know it's not something you changed, but consider one of these (maybe my perception is skewed):

int i = -1;
while ((i = substitutedReplacement.find(UString("$"), i + 1)) != -1)

OR

for (int i = substitutedReplacement.find(UString("$")); i != -1; i = substitutedReplacement.find(UString("$"), i + 1))

This comment would change, too:
      i += backrefLength - 1; // -1 offsets i++ (--> 'i + 1')


Anyway, looks good. We discussed how to write a layout test, so I'm going to clear the review bit for now.

Created attachment 8671 [details]
Patch including change log and test

This patch also adds double-digit back references ($nn) as required by the spec.

[David Kilzer (:ddkilzer)]

Committed revision 14705.

*** Bug 7919 has been marked as a duplicate of this bug. ***