LibWeb: Make input element placeholders look better

We now create a flex container inside the input element's UA shadow tree and add the placeholder and non-placeholder text as flex items (wrapped in elements whose style we can manipulate). This fixes the visual glitch where the placeholder would appear below the bounding box of the input element. It also allows us to align the text vertically inside the input element (like we're supposed to). In order to achieve this, I had to make two small architectural changes to layout tree building: - Elements can now report that they represent a given pseudo element. This allows us to instantiate the ::placeholder pseudo element as an actual DOM element inside the input element's UA shadow tree. - We no longer create a separate layout node for the shadow root itself. Instead, children of the shadow root are treated as if they were children of the DOM element itself for the purpose of layout tree building.
author: Andreas Kling <kling@serenityos.org> 2023-05-25 12:34:54 +0200
committer: Andreas Kling <kling@serenityos.org> 2023-05-25 14:42:24 +0200
commit: 7d24c13d8b6b0762b68d44f289083369e45879ae (patch)
tree: d26dc03fefc4a3ac2e6a7cc4d1d092efda9a1975
parent: 6fb661e781095d844a5f7d8f4d72a968b65960ee (diff)
download: serenity-7d24c13d8b6b0762b68d44f289083369e45879ae.zip
5 files changed, 109 insertions, 47 deletions
diff --git a/Tests/LibWeb/Layout/expected/input-element-with-display-inline.txt b/Tests/LibWeb/Layout/expected/input-element-with-display-inline.txt
index 4274ade933..44ad32caf5 100644
--- a/Tests/LibWeb/Layout/expected/input-element-with-display-inline.txt
+++ b/Tests/LibWeb/Layout/expected/input-element-with-display-inline.txt
@@ -1,9 +1,9 @@
 Viewport <#document> at (0,0) content-size 800x600 children: not-inline
   BlockContainer <html> at (1,1) content-size 798x45.835937 [BFC] children: not-inline
     BlockContainer <body> at (10,10) content-size 780x27.835937 children: inline
-      line 0 width: 202, height: 27.835937, bottom: 27.835937, baseline: 25.835937
+      line 0 width: 202, height: 27.835937, bottom: 27.835937, baseline: 23.835937
         frag 0 from BlockContainer start: 0, length: 0, rect: [11,11 200x25.835937]
       BlockContainer <input> at (11,11) content-size 200x25.835937 inline-block [BFC] children: not-inline
-        BlockContainer <#shadow-root> at (11,11) content-size 0x0 children: not-inline
-        BlockContainer <div> at (14,13) content-size 194x21.835937 children: inline
-          TextNode <#text>
+        Box <div> at (13,12) content-size 196x23.835937 flex-container(row) [FFC] children: not-inline
+          BlockContainer <div> at (14,13) content-size 0x21.835937 flex-item [BFC] children: inline
+            TextNode <#text>
diff --git a/Userland/Libraries/LibWeb/DOM/Element.h b/Userland/Libraries/LibWeb/DOM/Element.h
index 667e2365ac..4703ece58c 100644
--- a/Userland/Libraries/LibWeb/DOM/Element.h
+++ b/Userland/Libraries/LibWeb/DOM/Element.h
@@ -143,6 +143,8 @@ public:
 
     RequiredInvalidationAfterStyleChange recompute_style();
 
+    virtual Optional<CSS::Selector::PseudoElement> pseudo_element() const { return {}; }
+
     Layout::NodeWithStyle* layout_node();
     Layout::NodeWithStyle const* layout_node() const;
 
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp
index 7cc0d5ba35..2ec5e4c762 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp
+++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2022, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2018-2023, Andreas Kling <kling@serenityos.org>
  * Copyright (c) 2022, Adam Hodgen <ant1441@gmail.com>
  * Copyright (c) 2022, Andrew Kaster <akaster@serenityos.org>
  *
@@ -15,6 +15,7 @@
 #include <LibWeb/DOM/Text.h>
 #include <LibWeb/HTML/BrowsingContext.h>
 #include <LibWeb/HTML/EventNames.h>
+#include <LibWeb/HTML/HTMLDivElement.h>
 #include <LibWeb/HTML/HTMLFormElement.h>
 #include <LibWeb/HTML/HTMLInputElement.h>
 #include <LibWeb/HTML/Scripting/Environments.h>
@@ -56,7 +57,10 @@ JS::ThrowCompletionOr<void> HTMLInputElement::initialize(JS::Realm& realm)
 void HTMLInputElement::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
-    visitor.visit(m_text_node.ptr());
+    visitor.visit(m_inner_text_element);
+    visitor.visit(m_text_node);
+    visitor.visit(m_placeholder_element);
+    visitor.visit(m_placeholder_text_node);
     visitor.visit(m_legacy_pre_activation_behavior_checked_element_in_group.ptr());
     visitor.visit(m_selected_files);
 }
@@ -284,6 +288,8 @@ void HTMLInputElement::did_edit_text_node(Badge<BrowsingContext>)
     m_value = value_sanitization_algorithm(m_text_node->data());
     m_dirty_value = true;
 
+    update_placeholder_visibility();
+
     // NOTE: This is a bit ad-hoc, but basically implements part of "4.10.5.5 Common event behaviors"
     //       https://html.spec.whatwg.org/multipage/input.html#common-input-element-events
     queue_an_element_task(HTML::Task::Source::UserInteraction, [this] {
@@ -342,12 +348,26 @@ WebIDL::ExceptionOr<void> HTMLInputElement::set_value(DeprecatedString value)
     // 5. If the element's value (after applying the value sanitization algorithm) is different from oldValue,
     //    and the element has a text entry cursor position, move the text entry cursor position to the end of the
     //    text control, unselecting any selected text and resetting the selection direction to "none".
-    if (m_text_node && (m_value != old_value))
+    if (m_text_node && (m_value != old_value)) {
         m_text_node->set_data(m_value);
+        update_placeholder_visibility();
+    }
 
     return {};
 }
 
+void HTMLInputElement::update_placeholder_visibility()
+{
+    if (!m_placeholder_element)
+        return;
+    auto placeholder_text = this->placeholder_value();
+    if (placeholder_text.has_value()) {
+        MUST(m_placeholder_element->style_for_bindings()->set_property(CSS::PropertyID::Display, "block"sv));
+    } else {
+        MUST(m_placeholder_element->style_for_bindings()->set_property(CSS::PropertyID::Display, "none"sv));
+    }
+}
+
 // https://html.spec.whatwg.org/multipage/input.html#the-input-element:attr-input-placeholder-3
 static bool is_allowed_to_have_placeholder(HTML::HTMLInputElement::TypeAttributeState state)
 {
@@ -389,6 +409,17 @@ Optional<DeprecatedString> HTMLInputElement::placeholder_value() const
     return placeholder;
 }
 
+class PlaceholderElement final : public HTMLDivElement {
+    JS_CELL(PlaceholderElement, HTMLDivElement);
+
+public:
+    PlaceholderElement(DOM::Document& document)
+        : HTMLDivElement(document, DOM::QualifiedName { HTML::TagNames::div, ""sv, Namespace::HTML })
+    {
+    }
+    virtual Optional<CSS::Selector::PseudoElement> pseudo_element() const override { return CSS::Selector::PseudoElement::Placeholder; }
+};
+
 void HTMLInputElement::create_shadow_tree_if_needed()
 {
     if (shadow_root_internal())
@@ -412,7 +443,27 @@ void HTMLInputElement::create_shadow_tree_if_needed()
     if (initial_value.is_null())
         initial_value = DeprecatedString::empty();
     auto element = DOM::create_element(document(), HTML::TagNames::div, Namespace::HTML).release_value_but_fixme_should_propagate_errors();
-    MUST(element->set_attribute(HTML::AttributeNames::style, "white-space: pre; padding-top: 1px; padding-bottom: 1px; padding-left: 2px; padding-right: 2px; height: 1lh;"));
+    MUST(element->set_attribute(HTML::AttributeNames::style, R"~~~(
+        display: flex;
+        height: 100%;
+        align-items: center;
+        white-space: pre;
+        border: none;
+        padding: 1px 2px;
+)~~~"));
+
+    m_placeholder_element = heap().allocate<PlaceholderElement>(realm(), document()).release_allocated_value_but_fixme_should_propagate_errors();
+    MUST(m_placeholder_element->style_for_bindings()->set_property(CSS::PropertyID::Height, "1lh"sv));
+
+    m_placeholder_text_node = heap().allocate<DOM::Text>(realm(), document(), initial_value).release_allocated_value_but_fixme_should_propagate_errors();
+    m_placeholder_text_node->set_data(attribute(HTML::AttributeNames::placeholder));
+    m_placeholder_text_node->set_owner_input_element({}, *this);
+    MUST(m_placeholder_element->append_child(*m_placeholder_text_node));
+    MUST(element->append_child(*m_placeholder_element));
+
+    m_inner_text_element = DOM::create_element(document(), HTML::TagNames::div, Namespace::HTML).release_value_but_fixme_should_propagate_errors();
+    MUST(m_inner_text_element->style_for_bindings()->set_property(CSS::PropertyID::Height, "1lh"sv));
+
     m_text_node = heap().allocate<DOM::Text>(realm(), document(), initial_value).release_allocated_value_but_fixme_should_propagate_errors();
     m_text_node->set_always_editable(m_type != TypeAttributeState::FileUpload);
     m_text_node->set_owner_input_element({}, *this);
@@ -420,7 +471,8 @@ void HTMLInputElement::create_shadow_tree_if_needed()
     if (m_type == TypeAttributeState::Password)
         m_text_node->set_is_password_input({}, true);
 
-    MUST(element->append_child(*m_text_node));
+    MUST(m_inner_text_element->append_child(*m_text_node));
+    MUST(element->append_child(*m_inner_text_element));
     MUST(shadow_root->append_child(element));
     set_shadow_root(shadow_root);
 }
@@ -446,8 +498,13 @@ void HTMLInputElement::parse_attribute(DeprecatedFlyString const& name, Deprecat
     } else if (name == HTML::AttributeNames::type) {
         m_type = parse_type_attribute(value);
     } else if (name == HTML::AttributeNames::value) {
-        if (!m_dirty_value)
+        if (!m_dirty_value) {
             m_value = value_sanitization_algorithm(value);
+            update_placeholder_visibility();
+        }
+    } else if (name == HTML::AttributeNames::placeholder) {
+        if (m_placeholder_text_node)
+            m_placeholder_text_node->set_data(value);
     }
 }
 
@@ -474,8 +531,13 @@ void HTMLInputElement::did_remove_attribute(DeprecatedFlyString const& name)
         if (!m_dirty_checkedness)
             set_checked(false, ChangeSource::Programmatic);
     } else if (name == HTML::AttributeNames::value) {
-        if (!m_dirty_value)
+        if (!m_dirty_value) {
             m_value = DeprecatedString::empty();
+            update_placeholder_visibility();
+        }
+    } else if (name == HTML::AttributeNames::placeholder) {
+        if (m_placeholder_text_node)
+            m_placeholder_text_node->set_data({});
     }
 }
 
@@ -785,8 +847,10 @@ void HTMLInputElement::reset_algorithm()
 
     // and then invoke the value sanitization algorithm, if the type attribute's current state defines one.
     m_value = value_sanitization_algorithm(m_value);
-    if (m_text_node)
+    if (m_text_node) {
         m_text_node->set_data(m_value);
+        update_placeholder_visibility();
+    }
 }
 
 void HTMLInputElement::form_associated_element_was_inserted()
diff --git a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h
index 4cbfaabe06..d89a59f878 100644
--- a/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h
+++ b/Userland/Libraries/LibWeb/HTML/HTMLInputElement.h
@@ -130,6 +130,9 @@ public:
 
     virtual Optional<ARIA::Role> default_role() const override;
 
+    JS::GCPtr<Element> placeholder_element() { return m_placeholder_element; }
+    JS::GCPtr<Element const> placeholder_element() const { return m_placeholder_element; }
+
 private:
     HTMLInputElement(DOM::Document&, DOM::QualifiedName);
 
@@ -156,6 +159,11 @@ private:
     // https://html.spec.whatwg.org/multipage/input.html#value-sanitization-algorithm
     DeprecatedString value_sanitization_algorithm(DeprecatedString) const;
 
+    void update_placeholder_visibility();
+    JS::GCPtr<DOM::Element> m_placeholder_element;
+    JS::GCPtr<DOM::Text> m_placeholder_text_node;
+
+    JS::GCPtr<DOM::Element> m_inner_text_element;
     JS::GCPtr<DOM::Text> m_text_node;
     bool m_checked { false };
 
diff --git a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp
index 7cb1fcda5b..9b3d88bc7d 100644
--- a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp
+++ b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp
@@ -208,10 +208,6 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element
 
 ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context)
 {
-    // If the parent doesn't have a layout node, we don't need one either.
-    if (dom_node.parent_or_shadow_host() && !dom_node.parent_or_shadow_host()->layout_node())
-        return {};
-
     Optional<TemporaryChange<bool>> has_svg_root_change;
 
     if (dom_node.is_svg_container()) {
@@ -228,10 +224,24 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
 
     if (is<DOM::Element>(dom_node)) {
         auto& element = static_cast<DOM::Element&>(dom_node);
-        element.clear_pseudo_element_nodes({});
-        VERIFY(!element.needs_style_update());
-        style = element.computed_css_values();
-        display = style->display();
+
+        // Special path for ::placeholder, which corresponds to a synthetic DOM element inside the <input> UA shadow root.
+        // FIXME: This is very hackish. Find a better way to architect this.
+        if (element.pseudo_element() == CSS::Selector::PseudoElement::Placeholder) {
+            auto& input_element = verify_cast<HTML::HTMLInputElement>(*element.root().parent_or_shadow_host());
+            style = TRY(style_computer.compute_style(input_element, CSS::Selector::PseudoElement::Placeholder));
+            if (input_element.placeholder_value().has_value())
+                display = style->display();
+            else
+                display = CSS::Display::from_short(CSS::Display::Short::None);
+        }
+        // Common path: this is a regular DOM element. Style should be present already, thanks to Document::update_style().
+        else {
+            element.clear_pseudo_element_nodes({});
+            VERIFY(!element.needs_style_update());
+            style = element.computed_css_values();
+            display = style->display();
+        }
         if (display.is_none())
             return {};
         layout_node = element.create_layout_node(*style);
@@ -242,9 +252,6 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
     } else if (is<DOM::Text>(dom_node)) {
         layout_node = document.heap().allocate_without_realm<Layout::TextNode>(document, static_cast<DOM::Text&>(dom_node));
         display = CSS::Display(CSS::Display::Outside::Inline, CSS::Display::Inside::Flow);
-    } else if (is<DOM::ShadowRoot>(dom_node)) {
-        layout_node = document.heap().allocate_without_realm<Layout::BlockContainer>(document, &static_cast<DOM::ShadowRoot&>(dom_node), CSS::ComputedValues {});
-        display = CSS::Display(CSS::Display::Outside::Block, CSS::Display::Inside::FlowRoot);
     }
 
     if (!layout_node)
@@ -262,8 +269,11 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
 
     if ((dom_node.has_children() || shadow_root) && layout_node->can_have_children()) {
         push_parent(verify_cast<NodeWithStyle>(*layout_node));
-        if (shadow_root)
-            TRY(create_layout_tree(*shadow_root, context));
+        if (shadow_root) {
+            for (auto* node = shadow_root->first_child(); node; node = node->next_sibling()) {
+                TRY(create_layout_tree(*node, context));
+            }
+        }
 
         // This is the same as verify_cast<DOM::ParentNode>(dom_node).for_each_child
         for (auto* node = verify_cast<DOM::ParentNode>(dom_node).first_child(); node; node = node->next_sibling())
@@ -314,28 +324,6 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
         }
     }
 
-    if (is<HTML::HTMLInputElement>(dom_node)) {
-        auto& input_element = static_cast<HTML::HTMLInputElement&>(dom_node);
-
-        if (auto placeholder_value = input_element.placeholder_value(); placeholder_value.has_value()) {
-            auto placeholder_style = TRY(style_computer.compute_style(input_element, CSS::Selector::PseudoElement::Placeholder));
-            auto placeholder = DOM::Element::create_layout_node_for_display_type(document, placeholder_style->display(), placeholder_style, nullptr);
-
-            auto text = document.heap().allocate<DOM::Text>(document.realm(), document, *placeholder_value).release_allocated_value_but_fixme_should_propagate_errors();
-            auto text_node = document.heap().allocate_without_realm<Layout::TextNode>(document, *text);
-            text_node->set_generated(true);
-
-            push_parent(verify_cast<NodeWithStyle>(*layout_node));
-            push_parent(verify_cast<NodeWithStyle>(*placeholder));
-            insert_node_into_inline_or_block_ancestor(*text_node, text_node->display(), AppendOrPrepend::Append);
-            pop_parent();
-            insert_node_into_inline_or_block_ancestor(*placeholder, placeholder->display(), AppendOrPrepend::Append);
-            pop_parent();
-
-            input_element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Placeholder, placeholder);
-        }
-    }
-
     return {};
 }
author	Andreas Kling <kling@serenityos.org>	2023-05-25 12:34:54 +0200
committer	Andreas Kling <kling@serenityos.org>	2023-05-25 14:42:24 +0200
commit	7d24c13d8b6b0762b68d44f289083369e45879ae (patch)
tree	d26dc03fefc4a3ac2e6a7cc4d1d092efda9a1975
parent	6fb661e781095d844a5f7d8f4d72a968b65960ee (diff)
download	serenity-7d24c13d8b6b0762b68d44f289083369e45879ae.zip