REGRESSION (254522@main): HTMLSelectElement's value setter sets incorrect values if there are grouped options

pxlcoder · pxlcoder · commit 6b4b5fc1f0f6 · 2023-01-23T16:12:35.000-08:00
https://bugs.webkit.org/show_bug.cgi?id=251024 rdar://103520364 Reviewed by Ryosuke Niwa. While updating `HTMLSelectElement::listItems` to use `WeakPtr`, 254522@main also modified `HTMLSelectElement::setValue` to use `Vector::findIf` to obtain the index of the selected option. However, `listItems` includes `<optgroup>` elements, resulting in the index of the selected option being offset by the number of `<optgroup>` elements before it. `<optgroup>` elements should be ignored when determining the index of the selected option. To fix, restore the range-based for loop that was used prior to 254522@main. * LayoutTests/fast/forms/select-optgroup-set-value-expected.txt: Added. * LayoutTests/fast/forms/select-optgroup-set-value.html: Added. * Source/WebCore/html/HTMLSelectElement.cpp: (WebCore::HTMLSelectElement::setValue): Canonical link: https://commits.webkit.org/259249@main
diff --git a/LayoutTests/fast/forms/select-optgroup-set-value-expected.txt b/LayoutTests/fast/forms/select-optgroup-set-value-expected.txt
@@ -0,0 +1,11 @@
+Tests that setting the value of a <select> element to an <option> within an <optgroup> sets the correct value.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS select.value is "A"
+PASS select.value is "B"
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/select-optgroup-set-value.html b/LayoutTests/fast/forms/select-optgroup-set-value.html
@@ -0,0 +1,24 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+</head>
+<body>
+    <select id="select">
+        <optgroup>
+            <option value="A">A</option>
+            <option value="B">B</option>
+            <option value="C">C</option>
+        </optgroup>
+    </select>
+</body>
+<script>
+
+description("Tests that setting the value of a &lt;select&gt; element to an &lt;option&gt; within an &lt;optgroup&gt; sets the correct value.");
+
+shouldBeEqualToString("select.value", "A");
+select.value = "B";
+shouldBeEqualToString("select.value", "B");
+
+</script>
+</html>
diff --git a/Source/WebCore/html/HTMLSelectElement.cpp b/Source/WebCore/html/HTMLSelectElement.cpp
@@ -259,14 +259,18 @@ String HTMLSelectElement::value() const
 void HTMLSelectElement::setValue(const String& value)
 {
     // Find the option with value() matching the given parameter and make it the current selection.
-    auto optionIndex = listItems().findIf([&] (const auto& item) {
-        if (auto* optionElement = dynamicDowncast<HTMLOptionElement>(item.get()))
-            return optionElement->value() == value;
-
-        return false;
-    });
+    unsigned optionIndex = 0;
+    for (auto& item : listItems()) {
+        if (auto* option = dynamicDowncast<HTMLOptionElement>(item.get())) {
+            if (option->value() == value) {
+                setSelectedIndex(optionIndex);
+                return;
+            }
+            ++optionIndex;
+        }
+    }
 
-    setSelectedIndex(optionIndex);
+    setSelectedIndex(-1);
 }
 
 bool HTMLSelectElement::hasPresentationalHintsForAttribute(const QualifiedName& name) const
