VERIFIED FIXED 3294
String.prototype.replace() fails with function as second param
https://bugs.webkit.org/show_bug.cgi?id=3294
Summary String.prototype.replace() fails with function as second param
Gavin Kistner
Reported 2005-06-07 05:45:17 PDT
ECMAScript-262 rev3 section 15.5.4.11 (page 102) allows the second parameter to the replace() method of String objects to be a function. However, JavaScriptCore does not support this feature (at least in the build of Safari 2.0.412), instead calling .toString() on the second parameter supplied to the replace() method, and using that as the return string. The attached file "ReplaceTest.html" (also viewable at http://phrogz.net/JS/replaceTest.html) performs five tests of the replace method. The severity is normal because a workaround exists - similar functionality can be achieved (more painfully) by using a regexp with the 'g' flag and repeatedly calling .exec, working through the original string and building the output.
Attachments
5 replace() tests, including one for this bug. (4.27 KB, text/html)
2005-06-07 05:46 PDT, Gavin Kistner
no flags
Handle the second argument being a function (3.83 KB, patch)
2005-06-20 02:27 PDT, Anders Carlsson
darin: review-
Address comments (3.76 KB, patch)
2005-06-20 10:09 PDT, Anders Carlsson
no flags
Remove the space after substr (3.75 KB, patch)
2005-06-20 10:14 PDT, Anders Carlsson
darin: review+
Gavin Kistner
Comment 1 2005-06-07 05:46:18 PDT
Created attachment 2127 [details] 5 replace() tests, including one for this bug.
Gavin Kistner
Comment 2 2005-06-07 05:48:23 PDT
(This bug was copied from Apple's Bug Reporter #3749350)
Anders Carlsson
Comment 3 2005-06-20 02:27:33 PDT
Created attachment 2489 [details] Handle the second argument being a function Here's a patch that makes the test cases work for me.
Darin Adler
Comment 4 2005-06-20 08:49:01 PDT
Comment on attachment 2489 [details] Handle the second argument being a function I think the args for the replacement function should be declared inside each if statement. There's no reason to keep the args list around between invocations. And if you name it args, then the code is going to be a bit smaller too. Does this do the right thing when the replacement function raises a JavaScript exception? Extra space after substr before "(". Minor issues; this looks pretty good.
Anders Carlsson
Comment 5 2005-06-20 10:09:29 PDT
Created attachment 2504 [details] Address comments Here's a new patch that addresses the issues. Regarding exceptions, the following code has been tested in kjs and found to be working: function test(s) { throw ReferenceError; } try { testStr="hello"; result = testStr.replace("hello", test); } catch (e) { debug(e); } and gives the result --> (Internal Function) If this isn't the correct behavior, then I suspect the same error applies for compareWithCompareFunctionForQSort in array_object.cpp
Anders Carlsson
Comment 6 2005-06-20 10:14:21 PDT
Created attachment 2505 [details] Remove the space after substr Sorry, forgot the space after substr
Darin Adler
Comment 7 2005-06-20 11:20:09 PDT
Comment on attachment 2505 [details] Remove the space after substr r=me, given that we can land the attached test as a layout test.
Joost de Valk (AlthA)
Comment 8 2005-07-13 23:05:08 PDT
Fixed in 10.4.2
John Sullivan
Comment 9 2005-07-18 08:54:01 PDT
This appears fixed in tip of tree CVS, but does not appear fixed in 10.4.2 (contradicting the comment from Joost de Valk).
Darin Adler
Comment 10 2005-10-09 16:05:17 PDT
In Radar as <rdar://problem/4290603> String.replace() does not support a function as a parameter (3294).
Note You need to log in before you can comment on or make changes to this bug.