From 3ecc843cffa59b360c9e153fab394b5fa87d3031 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 15 Nov 2024 11:38:51 +0100 Subject: [PATCH] LibWeb: Handle undefined arguments correctly in the Option constructor The hand-rolled factory function wasn't handling undefined values entirely correctly. --- Libraries/LibWeb/Bindings/OptionConstructor.cpp | 17 ++++++++++------- .../option-element-constructor.txt | 5 ++--- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Libraries/LibWeb/Bindings/OptionConstructor.cpp b/Libraries/LibWeb/Bindings/OptionConstructor.cpp index 601f1f4475f..ecf1de0710d 100644 --- a/Libraries/LibWeb/Bindings/OptionConstructor.cpp +++ b/Libraries/LibWeb/Bindings/OptionConstructor.cpp @@ -45,6 +45,11 @@ JS::ThrowCompletionOr> OptionConstructor::construct auto& vm = this->vm(); auto& realm = *vm.current_realm(); + // NOTE: This implements the default value for the `text` parameter (the empty string ""). + auto text_value = vm.argument(0); + if (text_value.is_undefined()) + text_value = &vm.empty_string(); + // 1. Let document be the current principal global object's associated Document. auto& window = verify_cast(HTML::current_principal_global_object()); auto& document = window.associated_document(); @@ -54,16 +59,14 @@ JS::ThrowCompletionOr> OptionConstructor::construct JS::NonnullGCPtr option_element = verify_cast(*element); // 3. If text is not the empty string, then append to option a new Text node whose data is text. - if (vm.argument_count() > 0) { - auto text = TRY(vm.argument(0).to_string(vm)); - if (!text.is_empty()) { - auto new_text_node = realm.create(document, text); - MUST(option_element->append_child(*new_text_node)); - } + auto text = TRY(text_value.to_string(vm)); + if (!text.is_empty()) { + auto new_text_node = realm.create(document, text); + MUST(option_element->append_child(*new_text_node)); } // 4. If value is given, then set an attribute value for option using "value" and value. - if (vm.argument_count() > 1) { + if (!vm.argument(1).is_undefined()) { auto value = TRY(vm.argument(1).to_string(vm)); MUST(option_element->set_attribute(HTML::AttributeNames::value, value)); } diff --git a/Tests/LibWeb/Text/expected/wpt-import/html/semantics/forms/the-option-element/option-element-constructor.txt b/Tests/LibWeb/Text/expected/wpt-import/html/semantics/forms/the-option-element/option-element-constructor.txt index 1e604e4a102..1c094d55638 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/html/semantics/forms/the-option-element/option-element-constructor.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/html/semantics/forms/the-option-element/option-element-constructor.txt @@ -6,15 +6,14 @@ Rerun Found 11 tests -10 Pass -1 Fail +11 Pass Details Result Test Name MessagePass Option constructor with no arguments Pass Option constructor with falsy arguments Pass Option constructor creates HTMLOptionElement with specified text and value Pass Option constructor handles selectedness correctly when specified with defaultSelected only Pass Option constructor handles selectedness correctly, even when incongruous with defaultSelected -Fail Option constructor treats undefined text and value correctly +Pass Option constructor treats undefined text and value correctly Pass Option constructor treats empty text and value correctly Pass Option constructor treats falsy selected and defaultSelected correctly Pass Option constructor treats truthy selected and defaultSelected correctly