From 7d24c13d8b6b0762b68d44f289083369e45879ae Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 25 May 2023 12:34:54 +0200 Subject: [PATCH] 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. --- .../input-element-with-display-inline.txt | 8 +- Userland/Libraries/LibWeb/DOM/Element.h | 2 + .../LibWeb/HTML/HTMLInputElement.cpp | 80 +++++++++++++++++-- .../Libraries/LibWeb/HTML/HTMLInputElement.h | 8 ++ .../Libraries/LibWeb/Layout/TreeBuilder.cpp | 58 ++++++-------- 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 at (1,1) content-size 798x45.835937 [BFC] children: not-inline BlockContainer 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 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
at (14,13) content-size 194x21.835937 children: inline - TextNode <#text> + Box
at (13,12) content-size 196x23.835937 flex-container(row) [FFC] children: not-inline + BlockContainer
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 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 + * Copyright (c) 2018-2023, Andreas Kling * Copyright (c) 2022, Adam Hodgen * Copyright (c) 2022, Andrew Kaster * @@ -15,6 +15,7 @@ #include #include #include +#include #include #include #include @@ -56,7 +57,10 @@ JS::ThrowCompletionOr 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) 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 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 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 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(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(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(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 default_role() const override; + JS::GCPtr placeholder_element() { return m_placeholder_element; } + JS::GCPtr 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 m_placeholder_element; + JS::GCPtr m_placeholder_text_node; + + JS::GCPtr m_inner_text_element; JS::GCPtr 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 TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element ErrorOr 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> has_svg_root_change; if (dom_node.is_svg_container()) { @@ -228,10 +224,24 @@ ErrorOr TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder:: if (is(dom_node)) { auto& element = static_cast(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 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(*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 TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder:: } else if (is(dom_node)) { layout_node = document.heap().allocate_without_realm(document, static_cast(dom_node)); display = CSS::Display(CSS::Display::Outside::Inline, CSS::Display::Inside::Flow); - } else if (is(dom_node)) { - layout_node = document.heap().allocate_without_realm(document, &static_cast(dom_node), CSS::ComputedValues {}); - display = CSS::Display(CSS::Display::Outside::Block, CSS::Display::Inside::FlowRoot); } if (!layout_node) @@ -262,8 +269,11 @@ ErrorOr 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(*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_node).for_each_child for (auto* node = verify_cast(dom_node).first_child(); node; node = node->next_sibling()) @@ -314,28 +324,6 @@ ErrorOr TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder:: } } - if (is(dom_node)) { - auto& input_element = static_cast(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(document.realm(), document, *placeholder_value).release_allocated_value_but_fixme_should_propagate_errors(); - auto text_node = document.heap().allocate_without_realm(document, *text); - text_node->set_generated(true); - - push_parent(verify_cast(*layout_node)); - push_parent(verify_cast(*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 {}; }