LibWeb: Make select element use option's label, not text content

Psychpsyo · pbrw · commit e30f2792b0e9 · 2025-01-14T20:22:20.000+01:00
diff --git a/Libraries/LibWeb/HTML/HTMLOptionElement.cpp b/Libraries/LibWeb/HTML/HTMLOptionElement.cpp
@@ -114,6 +114,13 @@ String HTMLOptionElement::label() const
 void HTMLOptionElement::set_label(String const& label)
 {
     MUST(set_attribute(HTML::AttributeNames::label, label));
+
+    // NOTE: This option's select element may need to show different contents now.
+    if (selected()) {
+        if (auto select_element = first_ancestor_of_type<HTMLSelectElement>()) {
+            select_element->update_inner_text_element({});
+        }
+    }
 }
 
 // https://html.spec.whatwg.org/multipage/form-elements.html#dom-option-text
diff --git a/Libraries/LibWeb/HTML/HTMLSelectElement.cpp b/Libraries/LibWeb/HTML/HTMLSelectElement.cpp
@@ -413,15 +413,15 @@ void HTMLSelectElement::show_the_picker_if_applicable()
             for (auto const& child : opt_group_element.children_as_vector()) {
                 if (is<HTMLOptionElement>(*child)) {
                     auto& option_element = verify_cast<HTMLOptionElement>(*child);
-                    option_group_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.text_content()), option_element.value() });
+                    option_group_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.label()), option_element.value() });
                 }
             }
             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);
-            m_select_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.text_content()), option_element.value() });
+            m_select_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.label()), option_element.value() });
         }
 
         if (is<HTMLHRElement>(*child))
@@ -567,15 +567,21 @@ void HTMLSelectElement::create_shadow_tree_if_needed()
     update_inner_text_element();
 }
 
+void HTMLSelectElement::update_inner_text_element(Badge<HTMLOptionElement>)
+{
+    update_inner_text_element();
+}
+
+// FIXME: This needs to be called any time the selected option's children are modified.
 void HTMLSelectElement::update_inner_text_element()
 {
     if (!m_inner_text_element)
         return;
 
-    // Update inner text element to text content of selected option
+    // Update inner text element to the label of the selected option
     for (auto const& option_element : list_of_options()) {
         if (option_element->selected()) {
-            m_inner_text_element->set_text_content(strip_newlines(option_element->text_content()));
+            m_inner_text_element->set_text_content(strip_newlines(option_element->label()));
             return;
         }
     }
diff --git a/Libraries/LibWeb/HTML/HTMLSelectElement.h b/Libraries/LibWeb/HTML/HTMLSelectElement.h
@@ -96,6 +96,8 @@ class HTMLSelectElement final
 
     void update_selectedness();
 
+    void update_inner_text_element(Badge<HTMLOptionElement>);
+
 private:
     HTMLSelectElement(DOM::Document&, DOM::QualifiedName);
 
diff --git a/Tests/LibWeb/Ref/expected/select-option-use-label.html b/Tests/LibWeb/Ref/expected/select-option-use-label.html
@@ -0,0 +1,4 @@
+<!doctype html>
+<select>
+  <option>label</option>
+</select>
diff --git a/Tests/LibWeb/Ref/input/select-option-use-label.html b/Tests/LibWeb/Ref/input/select-option-use-label.html
@@ -0,0 +1,5 @@
+<!doctype html>
+<link rel="match" href="../expected/select-option-use-label.html" />
+<select>
+  <option label="label"></option>
+</select>

Original file line number	Diff line number	Diff line change
`@@ -114,6 +114,13 @@ String HTMLOptionElement::label() const`
`114`	`114`	`void HTMLOptionElement::set_label(String const& label)`
`115`	`115`	`{`
`116`	`116`	`MUST(set_attribute(HTML::AttributeNames::label, label));`
	`117`	`+`
	`118`	`+ // NOTE: This option's select element may need to show different contents now.`
	`119`	`+ if (selected()) {`
	`120`	`+ if (auto select_element = first_ancestor_of_type<HTMLSelectElement>()) {`
	`121`	`+ select_element->update_inner_text_element({});`
	`122`	`+ }`
	`123`	`+ }`
`117`	`124`	`}`
`118`	`125`
`119`	`126`	`// https://html.spec.whatwg.org/multipage/form-elements.html#dom-option-text`
Original file line number	Diff line number	Diff line change
`@@ -413,15 +413,15 @@ void HTMLSelectElement::show_the_picker_if_applicable()`
`413`	`413`	`for (auto const& child : opt_group_element.children_as_vector()) {`
`414`	`414`	`if (is<HTMLOptionElement>(*child)) {`
`415`	`415`	`auto& option_element = verify_cast<HTMLOptionElement>(*child);`
`416`		`- option_group_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.text_content()), option_element.value() });`
	`416`	`+ option_group_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.label()), option_element.value() });`
`417`	`417`	`}`
`418`	`418`	`}`
`419`	`419`	`m_select_items.append(SelectItemOptionGroup { opt_group_element.get_attribute(AttributeNames::label).value_or(String {}), option_group_items });`
`420`	`420`	`}`
`421`	`421`
`422`	`422`	`if (is<HTMLOptionElement>(*child)) {`
`423`	`423`	`auto& option_element = verify_cast<HTMLOptionElement>(*child);`
`424`		`- m_select_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.text_content()), option_element.value() });`
	`424`	`+ m_select_items.append(SelectItemOption { id_counter++, option_element.selected(), option_element.disabled(), option_element, strip_newlines(option_element.label()), option_element.value() });`
`425`	`425`	`}`
`426`	`426`
`427`	`427`	`if (is<HTMLHRElement>(*child))`
`@@ -567,15 +567,21 @@ void HTMLSelectElement::create_shadow_tree_if_needed()`
`567`	`567`	`update_inner_text_element();`
`568`	`568`	`}`
`569`	`569`
	`570`	`+void HTMLSelectElement::update_inner_text_element(Badge<HTMLOptionElement>)`
	`571`	`+{`
	`572`	`+ update_inner_text_element();`
	`573`	`+}`
	`574`	`+`
	`575`	`+// FIXME: This needs to be called any time the selected option's children are modified.`
`570`	`576`	`void HTMLSelectElement::update_inner_text_element()`
`571`	`577`	`{`
`572`	`578`	`if (!m_inner_text_element)`
`573`	`579`	`return;`
`574`	`580`
`575`		`- // Update inner text element to text content of selected option`
	`581`	`+ // Update inner text element to the label of the selected option`
`576`	`582`	`for (auto const& option_element : list_of_options()) {`
`577`	`583`	`if (option_element->selected()) {`
`578`		`- m_inner_text_element->set_text_content(strip_newlines(option_element->text_content()));`
	`584`	`+ m_inner_text_element->set_text_content(strip_newlines(option_element->label()));`
`579`	`585`	`return;`
`580`	`586`	`}`
`581`	`587`	`}`