From 29583104d220e4ea2d626781ef5c525fdf11974e Mon Sep 17 00:00:00 2001 From: sin-ack Date: Mon, 14 Mar 2022 23:05:55 +0000 Subject: [PATCH] LibWeb: Refactor all LabelableNode subclasses + input event handling :^) This commit is messy due to the Paintable and Layout classes being tangled together. The RadioButton, CheckBox and ButtonBox classes are now subclasses of FormAssociatedLabelableNode. This subclass separates these layout nodes from LabelableNode, which is also the superclass of non-form associated labelable nodes (Progress). ButtonPaintable, CheckBoxPaintable and RadioButtonPaintable no longer call events on DOM nodes directly from their mouse event handlers; instead, all the functionality is now directly in EventHandler, which dispatches the related events. handle_mousedown and related methods return a bool indicating whether the event handling should proceed. Paintable classes can now return an alternative DOM::Node which should be the target of the mouse event. Labels use this to indicate that the labeled control should be the target of the mouse events. HTMLInputElement put its activation behavior on run_activation_behavior, which wasn't actually called anywhere and had to be manually called by other places. We now use activation_behavior which is used by EventDispatcher. This commit also brings HTMLInputElement closer to spec by removing the did_foo functions that did ad-hoc event dispatching and unifies the behavior under run_input_activation_behavior. --- .../Libraries/LibWeb/DOM/EventDispatcher.cpp | 1 + Userland/Libraries/LibWeb/DOM/EventTarget.h | 4 +- .../LibWeb/HTML/HTMLInputElement.cpp | 151 +++++++++++++----- .../Libraries/LibWeb/HTML/HTMLInputElement.h | 18 +-- .../Libraries/LibWeb/Layout/ButtonBox.cpp | 13 +- Userland/Libraries/LibWeb/Layout/ButtonBox.h | 7 +- Userland/Libraries/LibWeb/Layout/CheckBox.cpp | 2 +- Userland/Libraries/LibWeb/Layout/CheckBox.h | 7 +- .../Layout/FormAssociatedLabelableNode.h | 29 ++++ Userland/Libraries/LibWeb/Layout/Label.h | 3 +- .../Libraries/LibWeb/Layout/RadioButton.cpp | 4 +- .../Libraries/LibWeb/Layout/RadioButton.h | 7 +- .../Libraries/LibWeb/Page/EventHandler.cpp | 129 ++++++++------- .../LibWeb/Painting/ButtonPaintable.cpp | 82 +--------- .../LibWeb/Painting/ButtonPaintable.h | 12 -- .../LibWeb/Painting/CheckBoxPaintable.cpp | 84 +--------- .../LibWeb/Painting/CheckBoxPaintable.h | 12 -- .../LibWeb/Painting/LabelablePaintable.cpp | 93 +++++++++-- .../LibWeb/Painting/LabelablePaintable.h | 29 +++- .../Libraries/LibWeb/Painting/Paintable.cpp | 9 +- .../Libraries/LibWeb/Painting/Paintable.h | 16 +- .../LibWeb/Painting/ProgressPaintable.h | 3 + .../LibWeb/Painting/RadioButtonPaintable.cpp | 102 +----------- .../LibWeb/Painting/RadioButtonPaintable.h | 17 -- .../LibWeb/Painting/TextPaintable.cpp | 34 ++-- .../Libraries/LibWeb/Painting/TextPaintable.h | 7 +- 26 files changed, 409 insertions(+), 466 deletions(-) create mode 100644 Userland/Libraries/LibWeb/Layout/FormAssociatedLabelableNode.h diff --git a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp index 208920cdeec..9584813beda 100644 --- a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp @@ -328,6 +328,7 @@ bool EventDispatcher::dispatch(NonnullRefPtr target, NonnullRefPtr< if (!event->cancelled()) { // NOTE: Since activation_target is set, it will have activation behavior. activation_target->activation_behavior(event); + activation_target->legacy_cancelled_activation_behavior_was_not_called(); } else { activation_target->legacy_cancelled_activation_behavior(); } diff --git a/Userland/Libraries/LibWeb/DOM/EventTarget.h b/Userland/Libraries/LibWeb/DOM/EventTarget.h index 8d5fb9b47cf..75586c3d367 100644 --- a/Userland/Libraries/LibWeb/DOM/EventTarget.h +++ b/Userland/Libraries/LibWeb/DOM/EventTarget.h @@ -55,13 +55,11 @@ public: // NOTE: These only exist for checkbox and radio input elements. virtual void legacy_pre_activation_behavior() { } virtual void legacy_cancelled_activation_behavior() { } + virtual void legacy_cancelled_activation_behavior_was_not_called() { } Bindings::CallbackType* event_handler_attribute(FlyString const& name); void set_event_handler_attribute(FlyString const& name, Optional); - // https://dom.spec.whatwg.org/#eventtarget-activation-behavior - virtual void run_activation_behavior() { } - protected: EventTarget(); diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp index 333cde11d6d..834d05a8bee 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp @@ -23,33 +23,20 @@ namespace Web::HTML { HTMLInputElement::HTMLInputElement(DOM::Document& document, DOM::QualifiedName qualified_name) : FormAssociatedElement(document, move(qualified_name)) { + activation_behavior = [this](auto&) { + // The activation behavior for input elements are these steps: + + // FIXME: 1. If this element is not mutable and is not in the Checkbox state and is not in the Radio state, then return. + + // 2. Run this element's input activation behavior, if any, and do nothing otherwise. + run_input_activation_behavior(); + }; } HTMLInputElement::~HTMLInputElement() { } -void HTMLInputElement::did_click_button(Badge) -{ - // FIXME: This should be a PointerEvent. - dispatch_event(DOM::Event::create(EventNames::click)); - - if (type() == "submit") { - if (auto* form = first_ancestor_of_type()) { - form->submit_form(this); - } - return; - } -} - -void HTMLInputElement::did_click_checkbox(Badge) -{ - // FIXME: This should be a PointerEvent. - auto click_event = DOM::Event::create(EventNames::click); - click_event->set_bubbles(true); - dispatch_event(move(click_event)); -} - RefPtr HTMLInputElement::create_layout_node(NonnullRefPtr style) { if (type() == "hidden") @@ -69,7 +56,7 @@ RefPtr HTMLInputElement::create_layout_node(NonnullRefPtrset_bubbles(true); dispatch_event(move(change_event)); + } else if (type() == "submit") { + RefPtr form; + // 1. If the element does not have a form owner, then return. + if (!(form = this->form())) + return; + + // 2. If the element's node document is not fully active, then return. + if (!document().is_fully_active()) + return; + + // 3. Submit the form owner from the element. + form->submit_form(this); } else { dispatch_event(DOM::Event::create(EventNames::change)); } @@ -208,7 +194,7 @@ void HTMLInputElement::parse_attribute(FlyString const& name, String const& valu // When the checked content attribute is added, if the control does not have dirty checkedness, // the user agent must set the checkedness of the element to true if (!m_dirty_checkedness) - set_checked(true, ChangeSource::Programmatic, ShouldRunActivationBehavior::No); + set_checked(true, ChangeSource::Programmatic); } } @@ -219,7 +205,7 @@ void HTMLInputElement::did_remove_attribute(FlyString const& name) // When the checked content attribute is removed, if the control does not have dirty checkedness, // the user agent must set the checkedness of the element to false. if (!m_dirty_checkedness) - set_checked(false, ChangeSource::Programmatic, ShouldRunActivationBehavior::No); + set_checked(false, ChangeSource::Programmatic); } } @@ -300,4 +286,93 @@ void HTMLInputElement::inserted() create_shadow_tree_if_needed(); } +void HTMLInputElement::set_checked_within_group() +{ + if (checked()) + return; + + set_checked(true, ChangeSource::User); + String name = this->name(); + + document().for_each_in_inclusive_subtree_of_type([&](auto& element) { + if (element.checked() && &element != this && element.name() == name) + element.set_checked(false, ChangeSource::User); + return IterationDecision::Continue; + }); +} + +// https://html.spec.whatwg.org/multipage/input.html#the-input-element:legacy-pre-activation-behavior +void HTMLInputElement::legacy_pre_activation_behavior() +{ + m_before_legacy_pre_activation_behavior_checked = checked(); + + // 1. If this element's type attribute is in the Checkbox state, then set + // this element's checkedness to its opposite value (i.e. true if it is + // false, false if it is true) and set this element's indeterminate IDL + // attribute to false. + // FIXME: Set indeterminate to false when that exists. + if (type_state() == TypeAttributeState::Checkbox) { + set_checked(!checked(), ChangeSource::User); + } + + // 2. If this element's type attribute is in the Radio Button state, then + // get a reference to the element in this element's radio button group that + // has its checkedness set to true, if any, and then set this element's + // checkedness to true. + if (type_state() == TypeAttributeState::RadioButton) { + String name = this->name(); + + document().for_each_in_inclusive_subtree_of_type([&](auto& element) { + if (element.checked() && element.name() == name) { + m_legacy_pre_activation_behavior_checked_element_in_group = element; + return IterationDecision::Break; + } + return IterationDecision::Continue; + }); + + set_checked_within_group(); + } +} + +// https://html.spec.whatwg.org/multipage/input.html#the-input-element:legacy-canceled-activation-behavior +void HTMLInputElement::legacy_cancelled_activation_behavior() +{ + // 1. If the element's type attribute is in the Checkbox state, then set the + // element's checkedness and the element's indeterminate IDL attribute back + // to the values they had before the legacy-pre-activation behavior was run. + if (type_state() == TypeAttributeState::Checkbox) { + set_checked(m_before_legacy_pre_activation_behavior_checked, ChangeSource::Programmatic); + } + + // 2. If this element 's type attribute is in the Radio Button state, then + // if the element to which a reference was obtained in the + // legacy-pre-activation behavior, if any, is still in what is now this + // element' s radio button group, if it still has one, and if so, setting + // that element 's checkedness to true; or else, if there was no such + // element, or that element is no longer in this element' s radio button + // group, or if this element no longer has a radio button group, setting + // this element's checkedness to false. + if (type_state() == TypeAttributeState::RadioButton) { + String name = this->name(); + bool did_reselect_previous_element = false; + if (m_legacy_pre_activation_behavior_checked_element_in_group) { + auto& element_in_group = *m_legacy_pre_activation_behavior_checked_element_in_group; + if (name == element_in_group.name()) { + element_in_group.set_checked_within_group(); + did_reselect_previous_element = true; + } + + m_legacy_pre_activation_behavior_checked_element_in_group = nullptr; + } + + if (!did_reselect_previous_element) + set_checked(false, ChangeSource::User); + } +} + +void HTMLInputElement::legacy_cancelled_activation_behavior_was_not_called() +{ + m_legacy_pre_activation_behavior_checked_element_in_group = nullptr; +} + } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h index 1bad7acd934..e0d22f363d8 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h @@ -67,14 +67,7 @@ public: Programmatic, User, }; - enum class ShouldRunActivationBehavior { - No, - Yes, - }; - void set_checked(bool, ChangeSource = ChangeSource::Programmatic, ShouldRunActivationBehavior = ShouldRunActivationBehavior::Yes); - - void did_click_button(Badge); - void did_click_checkbox(Badge); + void set_checked(bool, ChangeSource = ChangeSource::Programmatic); void did_edit_text_node(Badge); @@ -105,10 +98,13 @@ public: private: // ^DOM::EventTarget virtual void did_receive_focus() override; - virtual void run_activation_behavior() override; + virtual void legacy_pre_activation_behavior() override; + virtual void legacy_cancelled_activation_behavior() override; + virtual void legacy_cancelled_activation_behavior_was_not_called() override; void create_shadow_tree_if_needed(); void run_input_activation_behavior(); + void set_checked_within_group(); // https://html.spec.whatwg.org/multipage/input.html#value-sanitization-algorithm String value_sanitization_algorithm(String) const; @@ -118,6 +114,10 @@ private: // https://html.spec.whatwg.org/multipage/input.html#concept-input-checked-dirty-flag bool m_dirty_checkedness { false }; + + // https://html.spec.whatwg.org/multipage/input.html#the-input-element:legacy-pre-activation-behavior + bool m_before_legacy_pre_activation_behavior_checked { false }; + RefPtr m_legacy_pre_activation_behavior_checked_element_in_group; }; } diff --git a/Userland/Libraries/LibWeb/Layout/ButtonBox.cpp b/Userland/Libraries/LibWeb/Layout/ButtonBox.cpp index 17b25bb08cd..d297433a895 100644 --- a/Userland/Libraries/LibWeb/Layout/ButtonBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/ButtonBox.cpp @@ -7,13 +7,12 @@ #include #include #include -#include #include namespace Web::Layout { ButtonBox::ButtonBox(DOM::Document& document, HTML::HTMLInputElement& element, NonnullRefPtr style) - : LabelableNode(document, element, move(style)) + : FormAssociatedLabelableNode(document, element, move(style)) { } @@ -23,8 +22,14 @@ ButtonBox::~ButtonBox() void ButtonBox::prepare_for_replaced_layout() { - set_intrinsic_width(font().width(dom_node().value())); - set_intrinsic_height(font().glyph_height()); + // For and , the contents of + // the button does not appear as the contents of the element but as the + // value attribute. This is not the case with