From e0d26fff8c2101d0fe49ebbf6e725f33d5167fde Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 6 Jul 2021 13:19:59 -0400 Subject: [PATCH] LibJS: Replace strings with the search value coerced to a string This only causes 1 new test262 test to pass. Other tests that rely on this coercion fail due to receiving an unexpected value for 'this' when invoking a functional replacement. For example: String/prototype/replaceAll/replaceValue-call-matching-empty.js Receives 'undefined' for 'this' in the functional replacement invocation but is expected to receive the global 'this'. --- .../LibJS/Runtime/StringPrototype.cpp | 4 ++-- .../String/String.prototype.replace.js | 21 +++++++++++++++++++ .../String/String.prototype.replaceAll.js | 21 +++++++++++++++++++ 3 files changed, 44 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp index 3c89d74ec7..e44bdc676e 100644 --- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp @@ -819,7 +819,7 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::replace) String replacement; if (replace_value.is_function()) { - auto result = vm.call(replace_value.as_function(), js_undefined(), search_value, Value(position.value()), js_string(vm, string)); + auto result = vm.call(replace_value.as_function(), js_undefined(), js_string(vm, search_string), Value(position.value()), js_string(vm, string)); if (vm.exception()) return {}; @@ -915,7 +915,7 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::replace_all) String replacement; if (replace_value.is_function()) { - auto result = vm.call(replace_value.as_function(), js_undefined(), search_value, Value(position), js_string(vm, string)); + auto result = vm.call(replace_value.as_function(), js_undefined(), js_string(vm, search_string), Value(position), js_string(vm, string)); if (vm.exception()) return {}; diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replace.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replace.js index 46530ce135..306bff3608 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replace.js +++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replace.js @@ -176,3 +176,24 @@ test("replacement value is evaluated before searching the source string", () => expect(newString).toBe(""); expect(calls).toBe(2); }); + +test("search value is coerced to a string", () => { + var calls = 0; + var coerced; + + var searchValue = { + toString: function () { + calls += 1; + return "x"; + }, + }; + + var replaceValue = function (matched) { + coerced = matched; + return "abc"; + }; + + var newString = "x".replace(searchValue, replaceValue); + expect(newString).toBe("abc"); + expect(coerced).toBe("x"); +}); diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js index 7358c05f42..d2a6d3be9f 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js +++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.replaceAll.js @@ -122,3 +122,24 @@ test("replacement value is evaluated before searching the source string", () => expect(newString).toBe(""); expect(calls).toBe(2); }); + +test("search value is coerced to a string", () => { + var calls = 0; + var coerced; + + var searchValue = { + toString: function () { + calls += 1; + return "x"; + }, + }; + + var replaceValue = function (matched) { + coerced = matched; + return "abc"; + }; + + var newString = "x".replaceAll(searchValue, replaceValue); + expect(newString).toBe("abc"); + expect(coerced).toBe("x"); +});