From 6c75a93ec06c3dac0ae01c0d1d71cbc5a822a28a Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 27 Oct 2024 16:39:07 +0100 Subject: [PATCH] LibWeb: Fix select element state update in three ways 1. We were not propagating selectedness updates from option to select if the option was inside an optgroup. 2. When two or more options were selected, we were always favoring the last one in tree order, instead of the last one that got checked. 3. We were neglecting to return in the `display size is 1` case when all elements were disabled. This was covered by some of the :has() selector tests. :^) --- .../invalidation/has-with-pseudo-class.txt | 22 +++++++++---------- .../LibWeb/HTML/HTMLOptionElement.cpp | 4 ++-- .../LibWeb/HTML/HTMLSelectElement.cpp | 7 ++++-- .../Libraries/LibWeb/HTML/HTMLSelectElement.h | 2 +- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/selectors/invalidation/has-with-pseudo-class.txt b/Tests/LibWeb/Text/expected/wpt-import/css/selectors/invalidation/has-with-pseudo-class.txt index dc21349e346..84e15b7c132 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/selectors/invalidation/has-with-pseudo-class.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/selectors/invalidation/has-with-pseudo-class.txt @@ -6,29 +6,29 @@ Rerun Found 41 tests -23 Pass -18 Fail +31 Pass +10 Fail Details Result Test Name MessagePass Before set checked on checkbox, testing subject Pass Set checked on checkbox, testing subject Pass Unset checked on checkbox, testing subject -Fail Set select on option assert_equals: expected "rgb(255, 0, 0)" but got "rgb(128, 128, 128)" +Pass Set select on option Pass Reset select Pass Before set disabled on checkbox, testing subject Pass Set disabled on checkbox, testing subject -Fail Unset disabled on checkbox, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Unset disabled on checkbox, testing subject Pass Before set disabled on checkbox, testing subject3 Pass Set disabled on checkbox, testing subject3 Pass Unset disabled on checkbox, testing subject3 -Fail Before set disabled on option, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Before set disabled on option, testing subject Pass Set disabled on option, testing subject -Fail Unset disabled on option, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Unset disabled on option, testing subject Pass Before set disabled on option, testing subject3 Pass Set disabled on option, testing subject3 Pass Unset disabled on option, testing subject3 -Fail Before set disabled on optgroup, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Before set disabled on optgroup, testing subject Pass Set disabled on optgroup, testing subject -Fail Unset disabled on optgroup, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Unset disabled on optgroup, testing subject Fail Before set disabled on optgroup, testing subject2 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" Pass Set disabled on optgroup, testing subject2 Fail Unset disabled on optgroup, testing subject2 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" @@ -38,9 +38,9 @@ Pass Unset disabled on optgroup, testing subject3 Fail Before set disabled on optgroup, testing subject4 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" Pass Set disabled on optgroup, testing subject4 Fail Unset disabled on optgroup, testing subject4 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" -Fail Before setting value of text_input, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" -Fail Set value of text_input, testing subject assert_equals: expected "rgb(255, 255, 0)" but got "rgb(255, 0, 0)" -Fail Clear value of text_input, testing subject assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 0, 0)" +Pass Before setting value of text_input, testing subject +Fail Set value of text_input, testing subject assert_equals: expected "rgb(255, 255, 0)" but got "rgb(128, 128, 128)" +Pass Clear value of text_input, testing subject Fail Before setting value of text_input, testing subject2 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" Pass Set value of text_input, testing subject2 Fail Clear value of text_input, testing subject2 assert_equals: expected "rgb(128, 128, 128)" but got "rgb(255, 255, 0)" diff --git a/Userland/Libraries/LibWeb/HTML/HTMLOptionElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLOptionElement.cpp index bd6f8acec99..94020b54f4c 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLOptionElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLOptionElement.cpp @@ -138,8 +138,8 @@ int HTMLOptionElement::index() const void HTMLOptionElement::ask_for_a_reset() { // If an option element in the list of options asks for a reset, then run that select element's selectedness setting algorithm. - if (is(parent_element())) { - static_cast(parent())->update_selectedness(); + if (auto* select = first_ancestor_of_type()) { + select->update_selectedness(this); } } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp index 8e130016999..335e66ebf3d 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp @@ -558,7 +558,7 @@ void HTMLSelectElement::update_inner_text_element() } // https://html.spec.whatwg.org/multipage/form-elements.html#selectedness-setting-algorithm -void HTMLSelectElement::update_selectedness() +void HTMLSelectElement::update_selectedness(JS::GCPtr last_selected_option) { if (has_attribute(AttributeNames::multiple)) return; @@ -581,9 +581,10 @@ void HTMLSelectElement::update_selectedness() if (!option_element->disabled()) { option_element->set_selected_internal(true); update_inner_text_element(); - return; + break; } } + return; } } @@ -601,6 +602,8 @@ void HTMLSelectElement::update_selectedness() // then set the selectedness of all but the last option element with its selectedness set to true // in the list of options in tree order to false. for (auto const& option_element : list_of_options()) { + if (option_element == last_selected_option) + continue; if (number_of_selected == 1) { break; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h index fc56c3a4987..5959b985048 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h @@ -92,7 +92,7 @@ public: void did_select_item(Optional const& id); - void update_selectedness(); + void update_selectedness(JS::GCPtr last_selected_option = nullptr); private: HTMLSelectElement(DOM::Document&, DOM::QualifiedName);