From c9c1ddd0bba65b5cfc4938b8ea621611ae27bdc1 Mon Sep 17 00:00:00 2001 From: Luke Wilde Date: Sun, 18 Jun 2023 15:08:15 +0100 Subject: [PATCH] LibWeb: Fix up constructing the form entry list In particular: - Don't include none submitter buttons. - Use type_state() instead type() to avoid direct string comparisons - Support the hidden _charset_ input - Get form associated element's value directly instead of via the value attribute - Split line break normalization into a separate function so that it can also be used by form submission. --- Userland/Libraries/LibWeb/Forward.h | 1 + .../LibWeb/HTML/FormAssociatedElement.h | 9 +++ .../LibWeb/HTML/FormControlInfrastructure.cpp | 66 +++++++++++-------- .../LibWeb/HTML/FormControlInfrastructure.h | 3 +- .../LibWeb/HTML/HTMLButtonElement.cpp | 16 +++++ .../Libraries/LibWeb/HTML/HTMLButtonElement.h | 8 +++ .../LibWeb/HTML/HTMLInputElement.cpp | 20 ++++++ .../Libraries/LibWeb/HTML/HTMLInputElement.h | 8 ++- 8 files changed, 101 insertions(+), 30 deletions(-) diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 00d9bec100..edf55b947c 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -628,4 +628,5 @@ class ProgressEvent; class XMLHttpRequest; class XMLHttpRequestEventTarget; class XMLHttpRequestUpload; +struct FormDataEntry; } diff --git a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h index 2dffbc7550..4e6c9113b5 100644 --- a/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h +++ b/Userland/Libraries/LibWeb/HTML/FormAssociatedElement.h @@ -6,6 +6,7 @@ #pragma once +#include #include #include @@ -63,6 +64,14 @@ public: // https://html.spec.whatwg.org/multipage/forms.html#category-autocapitalize virtual bool is_auto_capitalize_inheriting() const { return false; } + // https://html.spec.whatwg.org/multipage/forms.html#concept-button + virtual bool is_button() const { return false; } + + // https://html.spec.whatwg.org/multipage/forms.html#concept-submit-button + virtual bool is_submit_button() const { return false; } + + virtual DeprecatedString value() const { return DeprecatedString::empty(); } + virtual HTMLElement& form_associated_element_to_html_element() = 0; // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#concept-form-reset-control diff --git a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp index 7d8ee131f0..265d0a3a01 100644 --- a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp +++ b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp @@ -49,8 +49,7 @@ WebIDL::ExceptionOr create_entry(JS::Realm& realm, String co } // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#constructing-the-form-data-set -// FIXME: Add missing parameters optional submitter, and optional encoding -WebIDL::ExceptionOr>> construct_entry_list(JS::Realm& realm, HTMLFormElement& form) +WebIDL::ExceptionOr>> construct_entry_list(JS::Realm& realm, HTMLFormElement& form, JS::GCPtr submitter, Optional encoding) { auto& vm = realm.vm(); @@ -69,6 +68,9 @@ WebIDL::ExceptionOr>> construct_entry_list(J // 5. For each element field in controls, in tree order: for (auto const& control : controls) { + auto const* control_as_form_associated_element = dynamic_cast(control.ptr()); + VERIFY(control_as_form_associated_element); + // 1. If any of the following is true, then continue: // - The field element has a datalist element ancestor. if (control->first_ancestor_of_type()) @@ -76,16 +78,18 @@ WebIDL::ExceptionOr>> construct_entry_list(J // - The field element is disabled. if (control->is_actually_disabled()) continue; - // FIXME: - The field element is a button but it is not submitter. + // - The field element is a button but it is not submitter. + if (control_as_form_associated_element->is_button() && control.ptr() != submitter.ptr()) + continue; // - The field element is an input element whose type attribute is in the Checkbox state and whose checkedness is false. // - The field element is an input element whose type attribute is in the Radio Button state and whose checkedness is false. if (auto* input_element = dynamic_cast(control.ptr())) { - if ((input_element->type() == "checkbox" || input_element->type() == "radio") && !input_element->checked()) + if ((input_element->type_state() == HTMLInputElement::TypeAttributeState::Checkbox || input_element->type_state() == HTMLInputElement::TypeAttributeState::RadioButton) && !input_element->checked()) continue; } // 2. If the field element is an input element whose type attribute is in the Image Button state, then: - if (auto* input_element = dynamic_cast(control.ptr()); input_element && input_element->type() == "image") { + if (auto* input_element = dynamic_cast(control.ptr()); input_element && input_element->type_state() == HTMLInputElement::TypeAttributeState::ImageButton) { // FIXME: 1. If the field element has a name attribute specified and its value is not the empty string, let name be that value followed by a single U+002E FULL STOP character (.). Otherwise, let name be the empty string. // FIXME: 2. Let namex be the string consisting of the concatenation of name and a single U0078 LATIN SMALL LETTER X character (x). // FIXME: 3. Let namey be the string consisting of the concatenation of name and a single U+0079 LATIN SMALL LETTER Y character (y). @@ -116,7 +120,7 @@ WebIDL::ExceptionOr>> construct_entry_list(J } } // 7. Otherwise, if the field element is an input element whose type attribute is in the Checkbox state or the Radio Button state, then: - else if (auto* checkbox_or_radio_element = dynamic_cast(control.ptr()); checkbox_or_radio_element && (checkbox_or_radio_element->type() == "checkbox" || checkbox_or_radio_element->type() == "radio") && checkbox_or_radio_element->checked()) { + else if (auto* checkbox_or_radio_element = dynamic_cast(control.ptr()); checkbox_or_radio_element && (checkbox_or_radio_element->type_state() == HTMLInputElement::TypeAttributeState::Checkbox || checkbox_or_radio_element->type_state() == HTMLInputElement::TypeAttributeState::RadioButton) && checkbox_or_radio_element->checked()) { // 1. If the field element has a value attribute specified, then let value be the value of that attribute; otherwise, let value be the string "on". auto value = checkbox_or_radio_element->value(); if (value.is_empty()) @@ -128,7 +132,7 @@ WebIDL::ExceptionOr>> construct_entry_list(J TRY_OR_THROW_OOM(vm, entry_list.try_append(XHR::FormDataEntry { .name = move(checkbox_or_radio_element_name), .value = move(checkbox_or_radio_element_value) })); } // 8. Otherwise, if the field element is an input element whose type attribute is in the File Upload state, then: - else if (auto* file_element = dynamic_cast(control.ptr()); file_element && file_element->type() == "file") { + else if (auto* file_element = dynamic_cast(control.ptr()); file_element && file_element->type_state() == HTMLInputElement::TypeAttributeState::FileUpload) { // 1. If there are no selected files, then create an entry with name and a new File object with an empty name, application/octet-stream as type, and an empty body, and append it to entry list. if (file_element->files()->length() == 0) { FileAPI::FilePropertyBag options {}; @@ -144,14 +148,17 @@ WebIDL::ExceptionOr>> construct_entry_list(J } } } - // FIXME: 9. Otherwise, if the field element is an input element whose type attribute is in the Hidden state and name is an ASCII case-insensitive match for "_charset_": - // FIXME: 1. Let charset be the name of encoding if encoding is given, and "UTF-8" otherwise. - // FIXME: 2. Create an entry with name and charset, and append it to entry list. + // 9. Otherwise, if the field element is an input element whose type attribute is in the Hidden state and name is an ASCII case-insensitive match for "_charset_": + else if (auto* hidden_input = dynamic_cast(control.ptr()); hidden_input && hidden_input->type_state() == HTMLInputElement::TypeAttributeState::Hidden && Infra::is_ascii_case_insensitive_match(name, "_charset_"sv)) { + // 1. Let charset be the name of encoding if encoding is given, and "UTF-8" otherwise. + auto charset = encoding.has_value() ? encoding.value() : TRY_OR_THROW_OOM(vm, "UTF-8"_string); + + // 2. Create an entry with name and charset, and append it to entry list. + TRY_OR_THROW_OOM(vm, entry_list.try_append(XHR::FormDataEntry { .name = move(name), .value = move(charset) })); + } // 10. Otherwise, create an entry with name and the value of the field element, and append it to entry list. else { - auto* element = dynamic_cast(control.ptr()); - VERIFY(element); - auto value_attribute = TRY_OR_THROW_OOM(vm, String::from_deprecated_string(element->attribute("value"sv))); + auto value_attribute = TRY_OR_THROW_OOM(vm, String::from_deprecated_string(control_as_form_associated_element->value())); TRY_OR_THROW_OOM(vm, entry_list.try_append(XHR::FormDataEntry { .name = move(name), .value = move(value_attribute) })); } @@ -177,24 +184,27 @@ WebIDL::ExceptionOr>> construct_entry_list(J return entry_list; } +ErrorOr normalize_line_breaks(StringView value) +{ + // Replace every occurrence of U+000D (CR) not followed by U+000A (LF), and every occurrence of U+000A (LF) not + // preceded by U+000D (CR) by a string consisting of a U+000D (CR) and U+000A (LF). + StringBuilder builder; + GenericLexer lexer { value }; + while (!lexer.is_eof()) { + TRY(builder.try_append(lexer.consume_until(is_any_of("\r\n"sv)))); + if ((lexer.peek() == '\r' && lexer.peek(1) != '\n') || lexer.peek() == '\n') { + TRY(builder.try_append("\r\n"sv)); + lexer.ignore(1); + } else { + lexer.ignore(2); + } + } + return builder.to_string(); +} + // https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#multipart/form-data-encoding-algorithm ErrorOr serialize_to_multipart_form_data(Vector const& entry_list) { - auto normalize_line_breaks = [](StringView value) -> ErrorOr { - StringBuilder builder; - GenericLexer lexer { value }; - while (!lexer.is_eof()) { - TRY(builder.try_append(lexer.consume_until(is_any_of("\r\n"sv)))); - if ((lexer.peek() == '\r' && lexer.peek(1) != '\n') || lexer.peek() == '\n') { - TRY(builder.try_append("\r\n"sv)); - lexer.ignore(1); - } else { - lexer.ignore(2); - } - } - return builder.to_string(); - }; - auto escape_line_feed_carriage_return_double_quote = [](StringView value) -> ErrorOr { StringBuilder builder; GenericLexer lexer { value }; diff --git a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.h b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.h index c0d75a4bfc..b35262f188 100644 --- a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.h +++ b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.h @@ -16,7 +16,8 @@ struct SerializedFormData { }; WebIDL::ExceptionOr create_entry(JS::Realm& realm, String const& name, Variant, String> const& value, Optional const& filename = {}); -WebIDL::ExceptionOr>> construct_entry_list(JS::Realm&, HTMLFormElement&); +WebIDL::ExceptionOr>> construct_entry_list(JS::Realm&, HTMLFormElement&, JS::GCPtr submitter = nullptr, Optional encoding = Optional {}); +ErrorOr normalize_line_breaks(StringView value); ErrorOr serialize_to_multipart_form_data(Vector const& entry_list); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.cpp index ffe3e84b91..0055f2f93b 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.cpp @@ -99,4 +99,20 @@ i32 HTMLButtonElement::default_tab_index_value() const return 0; } +// https://html.spec.whatwg.org/multipage/forms.html#concept-submit-button +// https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:concept-submit-button +bool HTMLButtonElement::is_submit_button() const +{ + // If the type attribute is in the Submit Button state, the element is specifically a submit button. + return type_state() == TypeAttributeState::Submit; +} + +// https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:concept-fe-value +DeprecatedString HTMLButtonElement::value() const +{ + if (!has_attribute(AttributeNames::value)) + return DeprecatedString::empty(); + return attribute(AttributeNames::value); +} + } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.h b/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.h index d73f25c710..72b80c5ce2 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLButtonElement.h @@ -52,6 +52,12 @@ public: // https://html.spec.whatwg.org/multipage/forms.html#category-autocapitalize virtual bool is_auto_capitalize_inheriting() const override { return true; } + // https://html.spec.whatwg.org/multipage/forms.html#concept-button + // https://html.spec.whatwg.org/multipage/form-elements.html#the-button-element:concept-button + virtual bool is_button() const override { return true; } + + virtual bool is_submit_button() const override; + // ^HTMLElement // https://html.spec.whatwg.org/multipage/forms.html#category-label virtual bool is_labelable() const override { return true; } @@ -59,6 +65,8 @@ public: // https://www.w3.org/TR/html-aria/#el-button virtual Optional default_role() const override { return ARIA::Role::button; } + virtual DeprecatedString value() const override; + private: HTMLButtonElement(DOM::Document&, DOM::QualifiedName); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index c9a1322b8d..636e58631f 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -1085,4 +1085,24 @@ Optional HTMLInputElement::default_role() const return {}; } +bool HTMLInputElement::is_button() const +{ + // https://html.spec.whatwg.org/multipage/input.html#submit-button-state-(type=submit):concept-button + // https://html.spec.whatwg.org/multipage/input.html#image-button-state-(type=image):concept-button + // https://html.spec.whatwg.org/multipage/input.html#reset-button-state-(type=reset):concept-button + // https://html.spec.whatwg.org/multipage/input.html#button-state-(type=button):concept-button + return type_state() == TypeAttributeState::SubmitButton + || type_state() == TypeAttributeState::ImageButton + || type_state() == TypeAttributeState::ResetButton + || type_state() == TypeAttributeState::Button; +} + +bool HTMLInputElement::is_submit_button() const +{ + // https://html.spec.whatwg.org/multipage/input.html#submit-button-state-(type=submit):concept-submit-button + // https://html.spec.whatwg.org/multipage/input.html#image-button-state-(type=image):concept-submit-button + return type_state() == TypeAttributeState::SubmitButton + || type_state() == TypeAttributeState::ImageButton; +} + } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h index 0a020c9764..94dc226a45 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h @@ -63,7 +63,7 @@ public: DeprecatedString default_value() const { return attribute(HTML::AttributeNames::value); } DeprecatedString name() const { return attribute(HTML::AttributeNames::name); } - DeprecatedString value() const; + virtual DeprecatedString value() const override; WebIDL::ExceptionOr set_value(DeprecatedString); Optional placeholder_value() const; @@ -120,6 +120,12 @@ public: // https://html.spec.whatwg.org/multipage/forms.html#category-autocapitalize virtual bool is_auto_capitalize_inheriting() const override { return true; } + // https://html.spec.whatwg.org/multipage/forms.html#concept-button + virtual bool is_button() const override; + + // https://html.spec.whatwg.org/multipage/forms.html#concept-submit-button + virtual bool is_submit_button() const override; + virtual void reset_algorithm() override; virtual void form_associated_element_was_inserted() override;