LibJS: Implement normative change in String.prototype.substr

And add spec comments while we're in the neighborhood.

2 files changed, 35 insertions, 3 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
index 6a2847da6b..8df88dfd85 100644
--- a/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/StringPrototype.cpp
@@ -568,26 +568,40 @@ JS_DEFINE_NATIVE_FUNCTION(StringPrototype::substring)
 // B.2.2.1 String.prototype.substr ( start, length ), https://tc39.es/ecma262/#sec-string.prototype.substr
 JS_DEFINE_NATIVE_FUNCTION(StringPrototype::substr)
 {
+    // 1. Let O be ? RequireObjectCoercible(this value).
+    // 2. Let S be ? ToString(O).
     auto string = TRY(utf16_string_from(vm));
+
+    // 3. Let size be the length of S.
     auto size = string.length_in_code_units();
 
+    // 4. Let intStart be ? ToIntegerOrInfinity(start).
     auto int_start = TRY(vm.argument(0).to_integer_or_infinity(vm));
+
+    // 5. If intStart is -∞, set intStart to 0.
     if (Value(int_start).is_negative_infinity())
         int_start = 0;
+    // 6. Else if intStart < 0, set intStart to max(size + intStart, 0).
     else if (int_start < 0)
         int_start = max(size + int_start, 0);
+    // 7. Else, set intStart to min(intStart, size).
+    else
+        int_start = min(int_start, size);
 
+    // 8. If length is undefined, let intLength be size; otherwise let intLength be ? ToIntegerOrInfinity(length).
     auto length = vm.argument(1);
-
     auto int_length = length.is_undefined() ? size : TRY(length.to_integer_or_infinity(vm));
-    if (Value(int_start).is_positive_infinity() || (int_length <= 0) || Value(int_length).is_positive_infinity())
-        return js_string(vm, String::empty());
 
+    // 9. Set intLength to the result of clamping intLength between 0 and size.
+    int_length = clamp(int_length, 0, size);
+
+    // 10. Let intEnd be min(intStart + intLength, size).
     auto int_end = min((i32)(int_start + int_length), size);
 
     if (int_start >= int_end)
         return js_string(vm, String::empty());
 
+    // 11. Return the substring of S from intStart to intEnd.
     return js_string(vm, string.substring_view(int_start, int_end - int_start));
 }
 
diff --git a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
index 9c0273f762..50b9f8ac64 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/String/String.prototype.substr.js
@@ -21,6 +21,24 @@ test("basic functionality", () => {
     expect("hello friends".substr(-3, -5)).toBe("");
 });
 
+test("Non numeric values", () => {
+    expect("a".substr(0, Infinity)).toBe("a");
+    expect("a".substr(0, -Infinity)).toBe("");
+    expect("abc".substr(0, Infinity)).toBe("abc");
+    expect("abc".substr(0, -Infinity)).toBe("");
+    expect("abc".substr(Infinity, Infinity)).toBe("");
+    expect("abc".substr(Infinity)).toBe("");
+    expect("abc".substr(-Infinity)).toBe("abc");
+    expect("abc".substr(-Infinity, 1)).toBe("a");
+    expect("abc".substr(-Infinity, Infinity)).toBe("abc");
+
+    expect("abc".substr(NaN)).toBe("abc");
+    expect("abc".substr(NaN, NaN)).toBe("");
+    expect("abc".substr(0, NaN)).toBe("");
+    expect("abc".substr(NaN, Infinity)).toBe("abc");
+    expect("abc".substr(NaN, -Infinity)).toBe("");
+});
+
 test("UTF-16", () => {
     var s = "😀";
     expect(s).toHaveLength(2);