Selecting OptGroup label does not deselect selected item

https://bugs.webkit.org/show_bug.cgi?id=257553 Reviewed by Aditya Keerthi. This patch aligns WebKit with Blink / Chromium and Gecko / Firefox. Merge: https://src.chromium.org/viewvc/blink?view=revision&revision=171470 In 'listbox' selected item should not get deselected, upon clicking on 'optgroup' label. This patch adds return for 'selected' item to not update if there is 'optGroup'. * Source/WebCore/html/HTMLSelectElement.cpp: (HTMLSelectElement::updateSelectedState): As above * LayoutTests/fast/forms/select/optgroup-clicking.html: Add new sub-test * LayoutTests/fast/forms/select/optgroup-clicking-expected.txt: Add new sub-test expectation Canonical link: https://commits.webkit.org/264767@main
WebKit · Jun 1, 2023 · 734687d · 734687d
1 parent 50f865f
commit 734687d
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 3 deletions.
diff --git a/LayoutTests/fast/forms/select/optgroup-clicking-expected.txt b/LayoutTests/fast/forms/select/optgroup-clicking-expected.txt
@@ -7,6 +7,9 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 Click enabled option
 PASS $("listbox").selectedIndex is 1
 
+Click on optgroup, should not deselect selectedIndex
+PASS $("listbox").selectedIndex is 1
+
 Click disabled option - doesn't change selectedIndex
 PASS $("listbox").selectedIndex is 1
 

diff --git a/LayoutTests/fast/forms/select/optgroup-clicking.html b/LayoutTests/fast/forms/select/optgroup-clicking.html
@@ -1,7 +1,7 @@
 <!DOCTYPE html>
 <html>
 <body onload="test()">
-<script src="../../../resources/js-test-pre.js"></script>
+<script src="../../../resources/js-test.js"></script>
 <script src="../resources/common.js"></script>
 
 <form id="form">
@@ -62,6 +62,12 @@
     eventSender.mouseUp();
     shouldBe('$("listbox").selectedIndex', '1');
 
+    debug("\nClick on optgroup, should not deselect selectedIndex");
+    mouseMoveToIndexInListbox(0, 'listbox'); // Select on optgroup
+    eventSender.mouseDown();
+    eventSender.mouseUp();
+    shouldBe('$("listbox").selectedIndex', '1');
+
     debug("\nClick disabled option - doesn't change selectedIndex");
     mouseMoveToIndexInListbox(5 + 2, 'listbox'); // +2 for optgroup's
     eventSender.mouseDown();
@@ -78,6 +84,5 @@
     finishJSTest();
 }
 </script>
-<script src="../../../resources/js-test-post.js"></script>
 </body>
 </html>
diff --git a/Source/WebCore/html/HTMLSelectElement.cpp b/Source/WebCore/html/HTMLSelectElement.cpp
@@ -1317,6 +1317,10 @@ void HTMLSelectElement::updateSelectedState(int listIndex, bool multi, bool shif
     if (listIndex < 0 || listIndex >= listSize)
         return;
 
+    auto& clickedElement = *items[listIndex];
+    if (is<HTMLOptGroupElement>(clickedElement))
+        return;
+
     // Save the selection so it can be compared to the new selection when
     // dispatching change events during mouseup, or after autoscroll finishes.
     saveLastSelection();
@@ -1326,7 +1330,6 @@ void HTMLSelectElement::updateSelectedState(int listIndex, bool multi, bool shif
     bool shiftSelect = m_multiple && shift;
     bool multiSelect = m_multiple && multi && !shift;
 
-    auto& clickedElement = *items[listIndex];
     if (is<HTMLOptionElement>(clickedElement)) {
         // Keep track of whether an active selection (like during drag
         // selection), should select or deselect.