From e46fa3ac8b7abc6712f5a196db7970a88553d5d6 Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 3 Apr 2021 14:54:04 +0200 Subject: [PATCH] LibJS: Keep RegExp.exec() results in correct order By using regex::AllFlags::SkipTrimEmptyMatches we get a null string for unmatched capture groups, which we then turn into an undefined entry in the result array instead of putting all matches first and appending undefined for the remaining number of capture groups - e.g. for /foo(ba((r)|(z)))/.exec("foobaz") we now return ["foobaz", "baz", "z", undefined, "z"] and not [ ["foobaz", "baz", "z", "z", undefined] Fixes part of #6042. Also happens to fix selecting an element by ID using jQuery's $("#foo"). --- Userland/Libraries/LibJS/Runtime/RegExpObject.cpp | 4 ++-- Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp | 7 +++---- .../Tests/builtins/RegExp/RegExp.prototype.exec.js | 10 ++++++++++ 3 files changed, 15 insertions(+), 6 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp b/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp index b3f49a2ca7c..ed54cee5727 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpObject.cpp @@ -39,8 +39,8 @@ static Flags options_from(const String& flags, VM& vm, GlobalObject& global_obje Flags options { // JS regexps are all 'global' by default as per our definition, but the "global" flag enables "stateful". // FIXME: Enable 'BrowserExtended' only if in a browser context. - { (regex::ECMAScriptFlags)regex::AllFlags::Global | ECMAScriptFlags::BrowserExtended }, - {}, + .effective_flags = { (regex::ECMAScriptFlags)regex::AllFlags::Global | (regex::ECMAScriptFlags)regex::AllFlags::SkipTrimEmptyMatches | regex::ECMAScriptFlags::BrowserExtended }, + .declared_flags = {}, }; for (auto ch : flags) { diff --git a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp index d1c1886dce0..f808400ee70 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp @@ -1,6 +1,6 @@ /* * Copyright (c) 2020, Matthew Olsson - * Copyright (c) 2020, Linus Groh + * Copyright (c) 2020-2021, Linus Groh * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -194,10 +194,9 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::exec) for (size_t i = 0; i < result.n_capture_groups; ++i) { auto capture_value = js_undefined(); - if (result.capture_group_matches[0].size() > i) { - auto& capture = result.capture_group_matches[0][i]; + auto& capture = result.capture_group_matches[0][i + 1]; + if (!capture.view.is_null()) capture_value = js_string(vm, capture.view.to_string()); - } array->indexed_properties().put(array, i + 1, capture_value); } diff --git a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.exec.js b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.exec.js index 3cdd5dd8e33..4ec24c1645b 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.exec.js +++ b/Userland/Libraries/LibJS/Tests/builtins/RegExp/RegExp.prototype.exec.js @@ -28,6 +28,16 @@ test("basic unnamed captures", () => { expect(res[2]).toBe(undefined); expect(res.groups).toBe(undefined); expect(res.index).toBe(0); + + re = /(foo)?(bar)/; + res = re.exec("bar"); + + expect(res.length).toBe(3); + expect(res[0]).toBe("bar"); + expect(res[1]).toBe(undefined); + expect(res[2]).toBe("bar"); + expect(res.groups).toBe(undefined); + expect(res.index).toBe(0); }); test("basic named captures", () => {