LibWeb: Refactor SelectItem to allow selecting options without value

Currently the `<select>` dropdown IPC uses the option value attr to find which option is selected. This won't work when options don't have values or when multiple options have the same value. Also the `SelectItem` contained so weird recursive structures that are impossible to create with HTML. So I refactored `SelectItem` as a variant, and gave the options a unique id. The id is send back to `HTMLSelectElement` so it can find out exactly which option element is selected.
Author: https://github.com/bplaat Commit: https://github.com/SerenityOS/serenity/commit/4408581ee0 Pull-request: https://github.com/SerenityOS/serenity/pull/23511 Reviewed-by: https://github.com/trflynn89
2024-09-11 05:25:32 +03:00 · 2024-04-03 19:19:08 +02:00 · 2024-04-03 19:19:08 +02:00 · 4408581ee0 · 2024-07-18 02:47:59 +09:00
commit 4408581ee0
parent 94d72c174a
21 changed files with 243 additions and 152 deletions
--- a/Base/res/html/misc/select.html
+++ b/Base/res/html/misc/select.html
@ -28,6 +28,17 @@
        Value: <span id="a-value">?</span>
    </p>

+    <p>Basic select with no values:</p>
+    <p>
+        <select>
+            <option value="">One</option>
+            <option value="">Two</option>
+            <option value="">Three</option>
+            <option value="">Four</option>
+            <option value="">Five</option>
+        </select>
+    </p>
+
    <p>Basic select with separators:</p>
    <p>
        <select onchange="document.getElementById('b-value').textContent = this.value" style="width: 50%;">
--- a/Ladybird/AppKit/UI/LadybirdWebView.mm
+++ b/Ladybird/AppKit/UI/LadybirdWebView.mm
@ -710,9 +710,35 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_
    m_web_view_bridge->on_request_select_dropdown = [self](Gfx::IntPoint content_position, i32 minimum_width, Vector<Web::HTML::SelectItem> items) {
        [self.select_dropdown removeAllItems];
        self.select_dropdown.minimumWidth = minimum_width;
+
+        auto add_menu_item = [self](Web::HTML::SelectItemOption const& item_option) {
+            NSMenuItem* menuItem = [[NSMenuItem alloc]
+                initWithTitle:Ladybird::string_to_ns_string(item_option.label)
+                       action:@selector(selectDropdownAction:)
+                keyEquivalent:@""];
+            menuItem.representedObject = [NSNumber numberWithUnsignedInt:item_option.id];
+            menuItem.state = item_option.selected ? NSControlStateValueOn : NSControlStateValueOff;
+            [self.select_dropdown addItem:menuItem];
+        };
+
        for (auto const& item : items) {
-            [self selectDropdownAdd:self.select_dropdown
-                               item:item];
+            if (item.has<Web::HTML::SelectItemOptionGroup>()) {
+                auto const& item_option_group = item.get<Web::HTML::SelectItemOptionGroup>();
+                NSMenuItem* subtitle = [[NSMenuItem alloc]
+                    initWithTitle:Ladybird::string_to_ns_string(item_option_group.label)
+                           action:nil
+                    keyEquivalent:@""];
+                [self.select_dropdown addItem:subtitle];
+
+                for (auto const& item_option : item_option_group.items)
+                    add_menu_item(item_option);
+            }
+
+            if (item.has<Web::HTML::SelectItemOption>())
+                add_menu_item(item.get<Web::HTML::SelectItemOption>());
+
+            if (item.has<Web::HTML::SelectItemSeparator>())
+                [self.select_dropdown addItem:[NSMenuItem separatorItem]];
        }

        auto device_pixel_ratio = m_web_view_bridge->device_pixel_ratio();
@ -819,40 +845,10 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_
    };
 }

- (void)selectDropdownAdd:(NSMenu*)menu item:(Web::HTML::SelectItem const&)item
-{
-    if (item.type == Web::HTML::SelectItem::Type::OptionGroup) {
-        NSMenuItem* subtitle = [[NSMenuItem alloc]
-            initWithTitle:Ladybird::string_to_ns_string(item.label.value_or(""_string))
-                   action:nil
-            keyEquivalent:@""];
-        subtitle.enabled = false;
-        [menu addItem:subtitle];
-
-        for (auto const& item : *item.items) {
-            [self selectDropdownAdd:menu
-                               item:item];
-        }
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Option) {
-        NSMenuItem* menuItem = [[NSMenuItem alloc]
-            initWithTitle:Ladybird::string_to_ns_string(item.label.value_or(""_string))
-                   action:@selector(selectDropdownAction:)
-            keyEquivalent:@""];
-        [menuItem setRepresentedObject:Ladybird::string_to_ns_string(item.value.value_or(""_string))];
-        [menuItem setEnabled:YES];
-        [menuItem setState:item.selected ? NSControlStateValueOn : NSControlStateValueOff];
-        [menu addItem:menuItem];
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Separator) {
-        [menu addItem:[NSMenuItem separatorItem]];
-    }
-}
-
 - (void)selectDropdownAction:(NSMenuItem*)menuItem
 {
-    auto value = Ladybird::ns_string_to_string([menuItem representedObject]);
-    m_web_view_bridge->select_dropdown_closed(value);
+    NSNumber* data = [menuItem representedObject];
+    m_web_view_bridge->select_dropdown_closed([data unsignedIntValue]);
 }

 - (void)menuDidClose:(NSMenu*)menu
--- a/Ladybird/Qt/Tab.cpp
+++ b/Ladybird/Qt/Tab.cpp
@ -331,8 +331,32 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St
    view().on_request_select_dropdown = [this](Gfx::IntPoint content_position, i32 minimum_width, Vector<Web::HTML::SelectItem> items) {
        m_select_dropdown->clear();
        m_select_dropdown->setMinimumWidth(minimum_width / view().device_pixel_ratio());
+
+        auto add_menu_item = [this](Web::HTML::SelectItemOption const& item_option) {
+            QAction* action = new QAction(qstring_from_ak_string(item_option.label), this);
+            action->setCheckable(true);
+            action->setChecked(item_option.selected);
+            action->setData(QVariant(static_cast<uint>(item_option.id)));
+            QObject::connect(action, &QAction::triggered, this, &Tab::select_dropdown_action);
+            m_select_dropdown->addAction(action);
+        };
+
        for (auto const& item : items) {
-            select_dropdown_add_item(m_select_dropdown, item);
+            if (item.has<Web::HTML::SelectItemOptionGroup>()) {
+                auto const& item_option_group = item.get<Web::HTML::SelectItemOptionGroup>();
+                QAction* subtitle = new QAction(qstring_from_ak_string(item_option_group.label), this);
+                subtitle->setDisabled(true);
+                m_select_dropdown->addAction(subtitle);
+
+                for (auto const& item_option : item_option_group.items)
+                    add_menu_item(item_option);
+            }
+
+            if (item.has<Web::HTML::SelectItemOption>())
+                add_menu_item(item.get<Web::HTML::SelectItemOption>());
+
+            if (item.has<Web::HTML::SelectItemSeparator>())
+                m_select_dropdown->addSeparator();
        }

        m_select_dropdown->exec(view().map_point_to_global_position(content_position));
@ -705,35 +729,10 @@ Tab::~Tab()
    delete m_inspector_widget;
 }

-void Tab::select_dropdown_add_item(QMenu* menu, Web::HTML::SelectItem const& item)
-{
-    if (item.type == Web::HTML::SelectItem::Type::OptionGroup) {
-        QAction* subtitle = new QAction(qstring_from_ak_string(item.label.value_or(""_string)), this);
-        subtitle->setDisabled(true);
-        menu->addAction(subtitle);
-
-        for (auto const& item : *item.items) {
-            select_dropdown_add_item(menu, item);
-        }
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Option) {
-        QAction* action = new QAction(qstring_from_ak_string(item.label.value_or(""_string)), this);
-        action->setCheckable(true);
-        action->setChecked(item.selected);
-        action->setData(QVariant(qstring_from_ak_string(item.value.value_or(""_string))));
-        QObject::connect(action, &QAction::triggered, this, &Tab::select_dropdown_action);
-        menu->addAction(action);
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Separator) {
-        menu->addSeparator();
-    }
-}
-
 void Tab::select_dropdown_action()
 {
    QAction* action = qobject_cast<QAction*>(sender());
-    auto value = action->data().value<QString>();
-    view().select_dropdown_closed(ak_string_from_qstring(value));
+    view().select_dropdown_closed(action->data().value<uint>());
 }

 void Tab::update_reset_zoom_button()
--- a/Ladybird/Qt/Tab.h
+++ b/Ladybird/Qt/Tab.h
@ -65,8 +65,6 @@ signals:
    void audio_play_state_changed(int id, Web::HTML::AudioPlayState);

 private:
-    void select_dropdown_add_item(QMenu* menu, Web::HTML::SelectItem const& item);
-
    virtual void resizeEvent(QResizeEvent*) override;
    virtual bool event(QEvent*) override;

--- a/Tests/LibWeb/Text/expected/select.txt
+++ b/Tests/LibWeb/Text/expected/select.txt
@ -3,3 +3,4 @@
 3. "two"
 4. 3
 5. "three"
+6. "Three"
--- a/Tests/LibWeb/Text/input/select.html
+++ b/Tests/LibWeb/Text/input/select.html
@ -9,7 +9,7 @@
        // 1. Get select value of unedited
        testPart(() => {
            const select = document.createElement('select');
-            // FIXME: Remove selected attribute (currently this is needed because the DIN children are not loaded to select first item in test run)
+            // FIXME: Remove selected attribute (currently this is needed because the children are not loaded to select first item in test run)
            select.innerHTML = `
                <option value="one" selected>One</option>
                <option value="two">Two</option>
@ -21,7 +21,7 @@
        // 2. Get select selectedIndex
        testPart(() => {
            const select = document.createElement('select');
-            // FIXME: Remove selected attribute (currently this is needed because the DIN children are not loaded to select first item in test run)
+            // FIXME: Remove selected attribute (currently this is needed because the children are not loaded to select first item in test run)
            select.innerHTML = `
                <option value="one" selected>One</option>
                <option value="two">Two</option>
@ -63,5 +63,16 @@
            `;
            return select.value;
        });
+
+        // 6. Get select value without option values
+        testPart(() => {
+            const select = document.createElement('select');
+            select.innerHTML = `
+                <option>One</option>
+                <option>Two</option>
+                <option selected>Three</option>
+            `;
+            return select.value;
+        });
    });
 </script>
--- a/Userland/Applications/Browser/Tab.cpp
+++ b/Userland/Applications/Browser/Tab.cpp
@ -653,8 +653,33 @@ Tab::Tab(BrowserWindow& window)
        m_select_dropdown_closed_by_action = false;
        m_select_dropdown->remove_all_actions();
        m_select_dropdown->set_minimum_width(minimum_width);
+
+        auto add_menu_item = [this](Web::HTML::SelectItemOption const& item_option) {
+            auto action = GUI::Action::create(item_option.label.to_byte_string(), [this, item_option](GUI::Action&) {
+                m_select_dropdown_closed_by_action = true;
+                view().select_dropdown_closed(item_option.id);
+            });
+            action->set_checkable(true);
+            action->set_checked(item_option.selected);
+            m_select_dropdown->add_action(action);
+        };
+
        for (auto const& item : items) {
-            select_dropdown_add_item(*m_select_dropdown, item);
+            if (item.has<Web::HTML::SelectItemOptionGroup>()) {
+                auto const& item_option_group = item.get<Web::HTML::SelectItemOptionGroup>();
+                auto subtitle = GUI::Action::create(item_option_group.label.to_byte_string(), nullptr);
+                subtitle->set_enabled(false);
+                m_select_dropdown->add_action(subtitle);
+
+                for (auto const& item_option : item_option_group.items)
+                    add_menu_item(item_option);
+            }
+
+            if (item.has<Web::HTML::SelectItemOption>())
+                add_menu_item(item.get<Web::HTML::SelectItemOption>());
+
+            if (item.has<Web::HTML::SelectItemSeparator>())
+                m_select_dropdown->add_separator();
        }

        m_select_dropdown->popup(view().screen_relative_rect().location().translated(content_position));
@ -794,31 +819,6 @@ Tab::Tab(BrowserWindow& window)
    };
 }

-void Tab::select_dropdown_add_item(GUI::Menu& menu, Web::HTML::SelectItem const& item)
-{
-    if (item.type == Web::HTML::SelectItem::Type::OptionGroup) {
-        auto subtitle = GUI::Action::create(MUST(ByteString::from_utf8(item.label.value_or(""_string))), nullptr);
-        subtitle->set_enabled(false);
-        menu.add_action(subtitle);
-
-        for (auto const& item : *item.items) {
-            select_dropdown_add_item(menu, item);
-        }
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Option) {
-        auto action = GUI::Action::create(MUST(ByteString::from_utf8(item.label.value_or(""_string))), [this, item](GUI::Action&) {
-            m_select_dropdown_closed_by_action = true;
-            view().select_dropdown_closed(item.value.value_or(""_string));
-        });
-        action->set_checkable(true);
-        action->set_checked(item.selected);
-        menu.add_action(action);
-    }
-    if (item.type == Web::HTML::SelectItem::Type::Separator) {
-        menu.add_separator();
-    }
-}
-
 void Tab::update_reset_zoom_button()
 {
    auto zoom_level = view().zoom_level();
--- a/Userland/Applications/Browser/Tab.h
+++ b/Userland/Applications/Browser/Tab.h
@ -109,8 +109,6 @@ private:
    void update_status(Optional<String> text_override = {}, i32 count_waiting = 0);
    void close_sub_widgets();

-    void select_dropdown_add_item(GUI::Menu& menu, Web::HTML::SelectItem const& item);
-
    WebView::History m_history;

    RefPtr<WebView::OutOfProcessWebView> m_web_content_view;
--- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
@ -46,6 +46,17 @@ void HTMLSelectElement::visit_edges(Cell::Visitor& visitor)
    visitor.visit(m_options);
    visitor.visit(m_inner_text_element);
    visitor.visit(m_chevron_icon_element);
+
+    for (auto const& item : m_select_items) {
+        if (item.has<SelectItemOption>())
+            visitor.visit(item.get<SelectItemOption>().option_element);
+
+        if (item.has<SelectItemOptionGroup>()) {
+            auto item_option_group = item.get<SelectItemOptionGroup>();
+            for (auto const& item : item_option_group.items)
+                visitor.visit(item.option_element);
+        }
+    }
 }

 void HTMLSelectElement::adjust_computed_style(CSS::StyleProperties& style)
@ -213,7 +224,12 @@ WebIDL::ExceptionOr<void> HTMLSelectElement::set_value(String const& value)
    for (auto const& option_element : list_of_options())
        option_element->set_selected(option_element->value() == value);
    update_inner_text_element();
+    queue_input_and_change_events();
+    return {};
+}

+void HTMLSelectElement::queue_input_and_change_events()
+{
    // When the user agent is to send select update notifications, queue an element task on the user interaction task source given the select element to run these steps:
    queue_an_element_task(HTML::Task::Source::UserInteraction, [this] {
        // FIXME: 1. Set the select element's user interacted to true.
@ -229,7 +245,6 @@ WebIDL::ExceptionOr<void> HTMLSelectElement::set_value(String const& value)
        change_event->set_bubbles(true);
        dispatch_event(*change_event);
    });
-    return {};
 }

 void HTMLSelectElement::set_is_open(bool open)
@ -246,7 +261,7 @@ bool HTMLSelectElement::has_activation_behavior() const
    return true;
 }

-static Optional<String> strip_newlines(Optional<String> string)
+static String strip_newlines(Optional<String> string)
 {
    // FIXME: Move this to a more general function
    if (!string.has_value())
@ -266,48 +281,65 @@ static Optional<String> strip_newlines(Optional<String> string)
 void HTMLSelectElement::activation_behavior(DOM::Event const&)
 {
    // Populate select items
-    Vector<SelectItem> items;
+    m_select_items.clear();
+    u32 id_counter = 1;
    for (auto const& child : children_as_vector()) {
        if (is<HTMLOptGroupElement>(*child)) {
            auto& opt_group_element = verify_cast<HTMLOptGroupElement>(*child);
-            Vector<SelectItem> opt_group_items;
+            Vector<SelectItemOption> option_group_items;
            for (auto const& child : opt_group_element.children_as_vector()) {
                if (is<HTMLOptionElement>(*child)) {
                    auto& option_element = verify_cast<HTMLOptionElement>(*child);
-                    auto option_value = option_element.value();
-                    opt_group_items.append(SelectItem { SelectItem::Type::Option, strip_newlines(option_element.text_content()), option_value, {}, option_element.selected() });
-                }
-                if (is<HTMLHRElement>(*child)) {
-                    opt_group_items.append(SelectItem { SelectItem::Type::Separator });
+                    option_group_items.append(SelectItemOption { id_counter++, strip_newlines(option_element.text_content()), option_element.value(), option_element.selected(), option_element });
                }
            }
-            items.append(SelectItem { SelectItem::Type::OptionGroup, opt_group_element.get_attribute(AttributeNames::label), {}, opt_group_items });
+            m_select_items.append(SelectItemOptionGroup { opt_group_element.get_attribute(AttributeNames::label).value_or(String {}), option_group_items });
        }

        if (is<HTMLOptionElement>(*child)) {
            auto& option_element = verify_cast<HTMLOptionElement>(*child);
-            auto option_value = option_element.value();
-            items.append(SelectItem { SelectItem::Type::Option, strip_newlines(option_element.text_content()), option_value, {}, option_element.selected() });
-        }
-        if (is<HTMLHRElement>(*child)) {
-            items.append(SelectItem { SelectItem::Type::Separator });
+            m_select_items.append(SelectItemOption { id_counter++, strip_newlines(option_element.text_content()), option_element.value(), option_element.selected(), option_element });
        }
+
+        if (is<HTMLHRElement>(*child))
+            m_select_items.append(SelectItemSeparator {});
    }

    // Request select dropdown
    auto weak_element = make_weak_ptr<HTMLSelectElement>();
    auto rect = get_bounding_client_rect();
    auto position = document().navigable()->to_top_level_position(Web::CSSPixelPoint { rect->x(), rect->y() });
-    document().page().did_request_select_dropdown(weak_element, position, CSSPixels(rect->width()), items);
+    document().page().did_request_select_dropdown(weak_element, position, CSSPixels(rect->width()), m_select_items);
    set_is_open(true);
 }

-void HTMLSelectElement::did_select_value(Optional<String> value)
+void HTMLSelectElement::did_select_item(Optional<u32> const& id)
 {
    set_is_open(false);
-    if (value.has_value()) {
-        MUST(set_value(*value));
+
+    if (!id.has_value())
+        return;
+
+    for (auto const& option_element : list_of_options())
+        option_element->set_selected(false);
+
+    for (auto const& item : m_select_items) {
+        if (item.has<SelectItemOption>()) {
+            auto const& item_option = item.get<SelectItemOption>();
+            if (item_option.id == *id)
+                item_option.option_element->set_selected(true);
+        }
+        if (item.has<SelectItemOptionGroup>()) {
+            auto item_option_group = item.get<SelectItemOptionGroup>();
+            for (auto const& item_option : item_option_group.items) {
+                if (item_option.id == *id)
+                    item_option.option_element->set_selected(true);
+            }
+        }
    }
+
+    update_inner_text_element();
+    queue_input_and_change_events();
 }

 void HTMLSelectElement::form_associated_element_was_inserted()
--- a/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h
+++ b/Userland/Libraries/LibWeb/HTML/HTMLSelectElement.h
@ -12,6 +12,7 @@
 #include <LibWeb/HTML/FormAssociatedElement.h>
 #include <LibWeb/HTML/HTMLElement.h>
 #include <LibWeb/HTML/HTMLOptionsCollection.h>
+#include <LibWeb/HTML/SelectItem.h>

 namespace Web::HTML {

@ -78,7 +79,7 @@ public:
    virtual void form_associated_element_was_inserted() override;
    virtual void form_associated_element_was_removed(DOM::Node*) override;

-    void did_select_value(Optional<String> value);
+    void did_select_item(Optional<u32> const& id);

 private:
    HTMLSelectElement(DOM::Document&, DOM::QualifiedName);
@ -93,9 +94,11 @@ private:

    void create_shadow_tree_if_needed();
    void update_inner_text_element();
+    void queue_input_and_change_events();

    JS::GCPtr<HTMLOptionsCollection> m_options;
    bool m_is_open { false };
+    Vector<SelectItem> m_select_items;
    JS::GCPtr<DOM::Element> m_inner_text_element;
    JS::GCPtr<DOM::Element> m_chevron_icon_element;
 };
--- a/Userland/Libraries/LibWeb/HTML/SelectItem.cpp
+++ b/Userland/Libraries/LibWeb/HTML/SelectItem.cpp
@ -9,23 +9,49 @@
 #include <LibIPC/Encoder.h>

 template<>
-ErrorOr<void> IPC::encode(Encoder& encoder, Web::HTML::SelectItem const& select_item)
+ErrorOr<void> IPC::encode(Encoder& encoder, Web::HTML::SelectItemOption const& item)
 {
-    TRY(encoder.encode(select_item.type));
-    TRY(encoder.encode(select_item.label));
-    TRY(encoder.encode(select_item.value));
-    TRY(encoder.encode(select_item.items));
-    TRY(encoder.encode(select_item.selected));
+    TRY(encoder.encode(item.id));
+    TRY(encoder.encode(item.label));
+    TRY(encoder.encode(item.value));
+    TRY(encoder.encode(item.selected));
    return {};
 }

 template<>
-ErrorOr<Web::HTML::SelectItem> IPC::decode(Decoder& decoder)
+ErrorOr<Web::HTML::SelectItemOption> IPC::decode(Decoder& decoder)
 {
-    auto type = TRY(decoder.decode<Web::HTML::SelectItem::Type>());
-    auto label = TRY(decoder.decode<Optional<String>>());
-    auto value = TRY(decoder.decode<Optional<String>>());
-    auto items = TRY(decoder.decode<Optional<Vector<Web::HTML::SelectItem>>>());
+    auto id = TRY(decoder.decode<u32>());
+    auto label = TRY(decoder.decode<String>());
+    auto value = TRY(decoder.decode<String>());
    auto selected = TRY(decoder.decode<bool>());
-    return Web::HTML::SelectItem { type, move(label), move(value), move(items), selected };
+    return Web::HTML::SelectItemOption { id, move(label), move(value), selected };
+}
+
+template<>
+ErrorOr<void> IPC::encode(Encoder& encoder, Web::HTML::SelectItemOptionGroup const& item)
+{
+    TRY(encoder.encode(item.label));
+    TRY(encoder.encode(item.items));
+    return {};
+}
+
+template<>
+ErrorOr<Web::HTML::SelectItemOptionGroup> IPC::decode(Decoder& decoder)
+{
+    auto label = TRY(decoder.decode<String>());
+    auto items = TRY(decoder.decode<Vector<Web::HTML::SelectItemOption>>());
+    return Web::HTML::SelectItemOptionGroup { move(label), move(items) };
+}
+
+template<>
+ErrorOr<void> IPC::encode(Encoder&, Web::HTML::SelectItemSeparator const&)
+{
+    return {};
+}
+
+template<>
+ErrorOr<Web::HTML::SelectItemSeparator> IPC::decode(Decoder&)
+{
+    return Web::HTML::SelectItemSeparator {};
 }
--- a/Userland/Libraries/LibWeb/HTML/SelectItem.h
+++ b/Userland/Libraries/LibWeb/HTML/SelectItem.h
@ -8,31 +8,47 @@

 #include <AK/String.h>
 #include <LibIPC/Forward.h>
+#include <LibWeb/HTML/HTMLOptionElement.h>

 namespace Web::HTML {

-struct SelectItem {
-    enum class Type {
-        OptionGroup,
-        Option,
-        Separator,
-    };
-
-    Type type;
-    Optional<String> label = {};
-    Optional<String> value = {};
-    Optional<Vector<SelectItem>> items = {};
-    bool selected = false;
+struct SelectItemOption {
+    u32 id { 0 };
+    String label {};
+    String value {};
+    bool selected { false };
+    JS::GCPtr<HTMLOptionElement> option_element {};
 };

+struct SelectItemOptionGroup {
+    String label = {};
+    Vector<SelectItemOption> items = {};
+};
+
+struct SelectItemSeparator { };
+
+using SelectItem = Variant<SelectItemOption, SelectItemOptionGroup, SelectItemSeparator>;
+
 }

 namespace IPC {

 template<>
-ErrorOr<void> encode(Encoder&, Web::HTML::SelectItem const&);
+ErrorOr<void> encode(Encoder&, Web::HTML::SelectItemOption const&);

 template<>
-ErrorOr<Web::HTML::SelectItem> decode(Decoder&);
+ErrorOr<Web::HTML::SelectItemOption> decode(Decoder&);
+
+template<>
+ErrorOr<void> encode(Encoder&, Web::HTML::SelectItemOptionGroup const&);
+
+template<>
+ErrorOr<Web::HTML::SelectItemOptionGroup> decode(Decoder&);
+
+template<>
+ErrorOr<void> encode(Encoder&, Web::HTML::SelectItemSeparator const&);
+
+template<>
+ErrorOr<Web::HTML::SelectItemSeparator> decode(Decoder&);

 }
--- a/Userland/Libraries/LibWeb/Page/Page.cpp
+++ b/Userland/Libraries/LibWeb/Page/Page.cpp
@ -377,14 +377,14 @@ void Page::did_request_select_dropdown(WeakPtr<HTML::HTMLSelectElement> target,
    }
 }

-void Page::select_dropdown_closed(Optional<String> value)
+void Page::select_dropdown_closed(Optional<u32> const& selected_item_id)
 {
    if (m_pending_non_blocking_dialog == PendingNonBlockingDialog::Select) {
        m_pending_non_blocking_dialog = PendingNonBlockingDialog::None;

        if (m_pending_non_blocking_dialog_target) {
            auto& select_element = verify_cast<HTML::HTMLSelectElement>(*m_pending_non_blocking_dialog_target);
-            select_element.did_select_value(move(value));
+            select_element.did_select_item(selected_item_id);
            m_pending_non_blocking_dialog_target.clear();
        }
    }
--- a/Userland/Libraries/LibWeb/Page/Page.h
+++ b/Userland/Libraries/LibWeb/Page/Page.h
@ -141,7 +141,7 @@ public:
    void file_picker_closed(Span<HTML::SelectedFile> selected_files);

    void did_request_select_dropdown(WeakPtr<HTML::HTMLSelectElement> target, Web::CSSPixelPoint content_position, Web::CSSPixels minimum_width, Vector<Web::HTML::SelectItem> items);
-    void select_dropdown_closed(Optional<String> value);
+    void select_dropdown_closed(Optional<u32> const& selected_item_id);

    enum class PendingNonBlockingDialog {
        None,
--- a/Userland/Libraries/LibWebView/ViewImplementation.cpp
+++ b/Userland/Libraries/LibWebView/ViewImplementation.cpp
@ -290,9 +290,9 @@ void ViewImplementation::file_picker_closed(Vector<Web::HTML::SelectedFile> sele
    client().async_file_picker_closed(page_id(), move(selected_files));
 }

-void ViewImplementation::select_dropdown_closed(Optional<String> value)
+void ViewImplementation::select_dropdown_closed(Optional<u32> const& selected_item_id)
 {
-    client().async_select_dropdown_closed(page_id(), value);
+    client().async_select_dropdown_closed(page_id(), selected_item_id);
 }

 void ViewImplementation::toggle_media_play_state()
--- a/Userland/Libraries/LibWebView/ViewImplementation.h
+++ b/Userland/Libraries/LibWebView/ViewImplementation.h
@ -95,7 +95,7 @@ public:
    void prompt_closed(Optional<String> response);
    void color_picker_update(Optional<Color> picked_color, Web::HTML::ColorPickerUpdateState state);
    void file_picker_closed(Vector<Web::HTML::SelectedFile> selected_files);
-    void select_dropdown_closed(Optional<String> value);
+    void select_dropdown_closed(Optional<u32> const& selected_item_id);

    void toggle_media_play_state();
    void toggle_media_mute_state();
--- a/Userland/Services/WebContent/ConnectionFromClient.cpp
+++ b/Userland/Services/WebContent/ConnectionFromClient.cpp
@ -1043,10 +1043,10 @@ void ConnectionFromClient::file_picker_closed(u64 page_id, Vector<Web::HTML::Sel
        page->page().file_picker_closed(const_cast<Vector<Web::HTML::SelectedFile>&>(selected_files));
 }

-void ConnectionFromClient::select_dropdown_closed(u64 page_id, Optional<String> const& value)
+void ConnectionFromClient::select_dropdown_closed(u64 page_id, Optional<u32> const& selected_item_id)
 {
    if (auto page = this->page(page_id); page.has_value())
-        page->page().select_dropdown_closed(value);
+        page->page().select_dropdown_closed(selected_item_id);
 }

 void ConnectionFromClient::toggle_media_play_state(u64 page_id)
--- a/Userland/Services/WebContent/ConnectionFromClient.h
+++ b/Userland/Services/WebContent/ConnectionFromClient.h
@ -104,7 +104,7 @@ private:
    virtual void prompt_closed(u64 page_id, Optional<String> const& response) override;
    virtual void color_picker_update(u64 page_id, Optional<Color> const& picked_color, Web::HTML::ColorPickerUpdateState const& state) override;
    virtual void file_picker_closed(u64 page_id, Vector<Web::HTML::SelectedFile> const& selected_files) override;
-    virtual void select_dropdown_closed(u64 page_id, Optional<String> const& value) override;
+    virtual void select_dropdown_closed(u64 page_id, Optional<u32> const& selected_item_id) override;

    virtual void toggle_media_play_state(u64 page_id) override;
    virtual void toggle_media_mute_state(u64 page_id) override;
--- a/Userland/Services/WebContent/PageClient.cpp
+++ b/Userland/Services/WebContent/PageClient.cpp
@ -434,9 +434,9 @@ void PageClient::color_picker_update(Optional<Color> picked_color, Web::HTML::Co
    page().color_picker_update(picked_color, state);
 }

-void PageClient::select_dropdown_closed(Optional<String> value)
+void PageClient::select_dropdown_closed(Optional<u32> const& selected_item_id)
 {
-    page().select_dropdown_closed(value);
+    page().select_dropdown_closed(selected_item_id);
 }

 Web::WebIDL::ExceptionOr<void> PageClient::toggle_media_play_state()
--- a/Userland/Services/WebContent/PageClient.h
+++ b/Userland/Services/WebContent/PageClient.h
@ -62,7 +62,7 @@ public:
    void confirm_closed(bool accepted);
    void prompt_closed(Optional<String> response);
    void color_picker_update(Optional<Color> picked_color, Web::HTML::ColorPickerUpdateState state);
-    void select_dropdown_closed(Optional<String> value);
+    void select_dropdown_closed(Optional<u32> const& selected_item_id);

    void set_user_style(String source);

--- a/Userland/Services/WebContent/WebContentServer.ipc
+++ b/Userland/Services/WebContent/WebContentServer.ipc
@ -91,7 +91,7 @@ endpoint WebContentServer
    prompt_closed(u64 page_id, Optional<String> response) =|
    color_picker_update(u64 page_id, Optional<Color> picked_color, Web::HTML::ColorPickerUpdateState state) =|
    file_picker_closed(u64 page_id, Vector<Web::HTML::SelectedFile> selected_files) =|
-    select_dropdown_closed(u64 page_id, Optional<String> value) =|
+    select_dropdown_closed(u64 page_id, Optional<u32> selected_item_id) =|

    toggle_media_play_state(u64 page_id) =|
    toggle_media_mute_state(u64 page_id) =|