LibWeb: Remove progress element custom paintable use shadow dom instead

Meta/gn/secondary/Userland/Libraries/LibWeb/Layout/BUILD.gn

@@ -32,7 +32,6 @@ source_set("Layout") {
     "ListItemBox.cpp",
     "ListItemMarkerBox.cpp",
     "Node.cpp",
-    "Progress.cpp",
     "RadioButton.cpp",
     "ReplacedBox.cpp",
     "SVGBox.cpp",

Meta/gn/secondary/Userland/Libraries/LibWeb/Painting/BUILD.gn

@@ -26,7 +26,6 @@ source_set("Painting") {
     "Paintable.cpp",
     "PaintableBox.cpp",
     "PaintingCommandExecutorCPU.cpp",
-    "ProgressPaintable.cpp",
     "RadioButtonPaintable.cpp",
     "RecordingPainter.cpp",
     "SVGGraphicsPaintable.cpp",

Userland/Libraries/LibWeb/CMakeLists.txt

@@ -448,7 +448,6 @@ set(SOURCES
     Layout/ListItemBox.cpp
     Layout/ListItemMarkerBox.cpp
     Layout/Node.cpp
-    Layout/Progress.cpp
     Layout/RadioButton.cpp
     Layout/ReplacedBox.cpp
     Layout/SVGBox.cpp
@@ -500,7 +499,6 @@ set(SOURCES
     Painting/Paintable.cpp
     Painting/PaintableBox.cpp
     Painting/PaintingCommandExecutorCPU.cpp
-    Painting/ProgressPaintable.cpp
     Painting/RadioButtonPaintable.cpp
     Painting/RecordingPainter.cpp
     Painting/SVGPathPaintable.cpp

Userland/Libraries/LibWeb/CSS/Default.css

@@ -90,6 +90,24 @@ meter::-webkit-meter-even-less-good-value {
     background-color: hsl(348, 100%, 61%);
 }
 
+/* Custom <progress> styles */
+progress {
+    display: inline-block;
+    width: 300px;
+    height: 12px;
+}
+progress::-webkit-progress-bar, progress::-webkit-progress-value {
+    display: block;
+    height: 100%;
+}
+progress::-webkit-progress-bar {
+    background-color: hsl(0, 0%, 96%);
+    border: 1px solid rgba(0, 0, 0, 0.5);
+}
+progress::-webkit-progress-value {
+    background-color: hsl(204, 86%, 53%);
+}
+
 /* 15.3.1 Hidden elements
  * https://html.spec.whatwg.org/multipage/rendering.html#hidden-elements
  */
@@ -796,24 +814,3 @@ progress {
         filter: invert(100%);
     }
 }
-
-/* This is the same as default intrinsic size of a <progress> element */
-progress {
-    width: 300px;
-    height: 12px;
-}
-
-/* The default progress-value/bar CSS below is the same as Blink/WebKit.
- * Note: Setting any more than the backgrond-color may have unintended consequences, as sites don't expect to unset more than that.
- */
-
-progress::-webkit-progress-bar {
-    width: inherit;
-    height: inherit;
-    background-color: hsl(0, 0%, 96%);
-}
-
-progress::-webkit-progress-value {
-    height: inherit;
-    background-color: hsl(204, 86%, 53%);
-}

Userland/Libraries/LibWeb/HTML/HTMLProgressElement.cpp

@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2020-2022, the SerenityOS developers.
  * Copyright (c) 2022, MacDue <macdue@dueutil.tech>
+ * Copyright (c) 2023, Bastiaan van der Plaat <bastiaan.v.d.plaat@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -9,9 +10,6 @@
 #include <LibWeb/DOM/ShadowRoot.h>
 #include <LibWeb/HTML/HTMLProgressElement.h>
 #include <LibWeb/HTML/Numbers.h>
-#include <LibWeb/Layout/BlockContainer.h>
-#include <LibWeb/Layout/Node.h>
-#include <LibWeb/Layout/Progress.h>
 
 namespace Web::HTML {
 
@@ -30,26 +28,10 @@ void HTMLProgressElement::initialize(JS::Realm& realm)
     set_prototype(&Bindings::ensure_web_prototype<Bindings::HTMLProgressElementPrototype>(realm, "HTMLProgressElement"_fly_string));
 }
 
-JS::GCPtr<Layout::Node> HTMLProgressElement::create_layout_node(NonnullRefPtr<CSS::StyleProperties> style)
+void HTMLProgressElement::visit_edges(Cell::Visitor& visitor)
 {
-    if (style->appearance().value_or(CSS::Appearance::Auto) == CSS::Appearance::None)
-        return HTMLElement::create_layout_node(style);
-    return heap().allocate_without_realm<Layout::Progress>(document(), *this, move(style));
-}
-
-bool HTMLProgressElement::using_system_appearance() const
-{
-    if (layout_node())
-        return is<Layout::Progress>(*layout_node());
-    return false;
-}
-
-void HTMLProgressElement::progress_position_updated()
-{
-    if (using_system_appearance())
-        layout_node()->set_needs_display();
-    else
-        document().invalidate_layout();
+    Base::visit_edges(visitor);
+    visitor.visit(m_progress_value_element);
 }
 
 // https://html.spec.whatwg.org/multipage/form-elements.html#dom-progress-value
@@ -70,7 +52,8 @@ WebIDL::ExceptionOr<void> HTMLProgressElement::set_value(double value)
         return {};
 
     TRY(set_attribute(HTML::AttributeNames::value, MUST(String::number(value))));
-    progress_position_updated();
+    update_progress_value_element();
+    document().invalidate_layout();
     return {};
 }
 
@@ -92,7 +75,8 @@ WebIDL::ExceptionOr<void> HTMLProgressElement::set_max(double value)
         return {};
 
     TRY(set_attribute(HTML::AttributeNames::max, MUST(String::number(value))));
-    progress_position_updated();
+    update_progress_value_element();
+    document().invalidate_layout();
     return {};
 }
 
@@ -104,4 +88,35 @@ double HTMLProgressElement::position() const
     return value() / max();
 }
 
+void HTMLProgressElement::inserted()
+{
+    create_shadow_tree_if_needed();
+}
+
+void HTMLProgressElement::removed_from(DOM::Node*)
+{
+    set_shadow_root(nullptr);
+}
+
+void HTMLProgressElement::create_shadow_tree_if_needed()
+{
+    if (shadow_root_internal())
+        return;
+
+    auto shadow_root = heap().allocate<DOM::ShadowRoot>(realm(), document(), *this, Bindings::ShadowRootMode::Closed);
+    set_shadow_root(shadow_root);
+
+    auto progress_bar_element = heap().allocate<ProgressBarElement>(realm(), document());
+    MUST(shadow_root->append_child(*progress_bar_element));
+
+    m_progress_value_element = heap().allocate<ProgressValueElement>(realm(), document());
+    MUST(progress_bar_element->append_child(*m_progress_value_element));
+    update_progress_value_element();
+}
+
+void HTMLProgressElement::update_progress_value_element()
+{
+    MUST(m_progress_value_element->set_attribute(HTML::AttributeNames::style, MUST(String::formatted("width: {}%;", position() * 100))));
+}
+
 }

Userland/Libraries/LibWeb/HTML/HTMLProgressElement.h

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020-2022, the SerenityOS developers.
+ * Copyright (c) 2023, Bastiaan van der Plaat <bastiaan.v.d.plaat@gmail.com>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -7,10 +8,34 @@
 #pragma once
 
 #include <LibWeb/ARIA/Roles.h>
+#include <LibWeb/HTML/HTMLDivElement.h>
 #include <LibWeb/HTML/HTMLElement.h>
+#include <LibWeb/Namespace.h>
 
 namespace Web::HTML {
 
+class ProgressBarElement final : public HTMLDivElement {
+    JS_CELL(ProgressBarElement, HTMLDivElement);
+
+public:
+    ProgressBarElement(DOM::Document& document)
+        : HTMLDivElement(document, DOM::QualifiedName { HTML::TagNames::div, ""_fly_string, Namespace::HTML })
+    {
+    }
+    virtual Optional<CSS::Selector::PseudoElement> pseudo_element() const override { return CSS::Selector::PseudoElement::ProgressBar; }
+};
+
+class ProgressValueElement final : public HTMLDivElement {
+    JS_CELL(ProgressValueElement, HTMLDivElement);
+
+public:
+    ProgressValueElement(DOM::Document& document)
+        : HTMLDivElement(document, DOM::QualifiedName { HTML::TagNames::div, ""_fly_string, Namespace::HTML })
+    {
+    }
+    virtual Optional<CSS::Selector::PseudoElement> pseudo_element() const override { return CSS::Selector::PseudoElement::ProgressValue; }
+};
+
 class HTMLProgressElement final : public HTMLElement {
     WEB_PLATFORM_OBJECT(HTMLProgressElement, HTMLElement);
     JS_DECLARE_ALLOCATOR(HTMLProgressElement);
@@ -18,8 +43,6 @@ class HTMLProgressElement final : public HTMLElement {
 public:
     virtual ~HTMLProgressElement() override;
 
-    virtual JS::GCPtr<Layout::Node> create_layout_node(NonnullRefPtr<CSS::StyleProperties>) override;
-
     double value() const;
     WebIDL::ExceptionOr<void> set_value(double);
 
@@ -29,11 +52,12 @@ public:
     double position() const;
 
     // ^HTMLElement
+    virtual void inserted() override;
+    virtual void removed_from(DOM::Node*) override;
+
     // https://html.spec.whatwg.org/multipage/forms.html#category-label
     virtual bool is_labelable() const override { return true; }
 
-    bool using_system_appearance() const;
-
     // https://www.w3.org/TR/html-aria/#el-progress
     virtual Optional<ARIA::Role> default_role() const override { return ARIA::Role::progressbar; }
 
@@ -44,10 +68,15 @@ private:
     virtual bool is_html_progress_element() const final { return true; }
 
     virtual void initialize(JS::Realm&) override;
+    virtual void visit_edges(Cell::Visitor&) override;
+
+    void create_shadow_tree_if_needed();
 
-    void progress_position_updated();
+    void update_progress_value_element();
 
     bool is_determinate() const { return has_attribute(HTML::AttributeNames::value); }
+
+    JS::GCPtr<ProgressValueElement> m_progress_value_element;
 };
 
 }

Userland/Libraries/LibWeb/Layout/Progress.cpp

@@ -1,25 +0,0 @@
-/*
- * Copyright (c) 2022, the SerenityOS developers.
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <LibWeb/Layout/Progress.h>
-#include <LibWeb/Painting/ProgressPaintable.h>
-
-namespace Web::Layout {
-
-Progress::Progress(DOM::Document& document, HTML::HTMLProgressElement& element, NonnullRefPtr<CSS::StyleProperties> style)
-    : LabelableNode(document, element, move(style))
-{
-    set_natural_height(12);
-}
-
-Progress::~Progress() = default;
-
-JS::GCPtr<Painting::Paintable> Progress::create_paintable() const
-{
-    return Painting::ProgressPaintable::create(*this);
-}
-
-}

Userland/Libraries/LibWeb/Layout/Progress.h

@@ -1,27 +0,0 @@
-/*
- * Copyright (c) 2022, the SerenityOS developers.
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <LibWeb/HTML/HTMLProgressElement.h>
-#include <LibWeb/Layout/LabelableNode.h>
-
-namespace Web::Layout {
-
-class Progress final : public LabelableNode {
-    JS_CELL(Progress, LabelableNode);
-
-public:
-    Progress(DOM::Document&, HTML::HTMLProgressElement&, NonnullRefPtr<CSS::StyleProperties>);
-    virtual ~Progress() override;
-
-    const HTML::HTMLProgressElement& dom_node() const { return static_cast<const HTML::HTMLProgressElement&>(LabelableNode::dom_node()); }
-    HTML::HTMLProgressElement& dom_node() { return static_cast<HTML::HTMLProgressElement&>(LabelableNode::dom_node()); }
-
-    virtual JS::GCPtr<Painting::Paintable> create_paintable() const override;
-};
-
-}

Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp

@@ -22,12 +22,10 @@
 #include <LibWeb/HTML/HTMLInputElement.h>
 #include <LibWeb/HTML/HTMLLIElement.h>
 #include <LibWeb/HTML/HTMLOListElement.h>
-#include <LibWeb/HTML/HTMLProgressElement.h>
 #include <LibWeb/HTML/HTMLSlotElement.h>
 #include <LibWeb/Layout/ListItemBox.h>
 #include <LibWeb/Layout/ListItemMarkerBox.h>
 #include <LibWeb/Layout/Node.h>
-#include <LibWeb/Layout/Progress.h>
 #include <LibWeb/Layout/TableGrid.h>
 #include <LibWeb/Layout/TableWrapper.h>
 #include <LibWeb/Layout/TextNode.h>
@@ -311,7 +309,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
 
         // Special path for elements that use pseudo selectors.
         // FIXME: This is very hackish. Find a better way to architect this.
-        if (element.pseudo_element() == CSS::Selector::PseudoElement::Placeholder || element.pseudo_element() == CSS::Selector::PseudoElement::MeterBar || element.pseudo_element() == CSS::Selector::PseudoElement::MeterOptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterSuboptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterEvenLessGoodValue) {
+        if (element.pseudo_element() == CSS::Selector::PseudoElement::Placeholder || element.pseudo_element() == CSS::Selector::PseudoElement::MeterBar || element.pseudo_element() == CSS::Selector::PseudoElement::MeterOptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterSuboptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterEvenLessGoodValue || element.pseudo_element() == CSS::Selector::PseudoElement::ProgressBar || element.pseudo_element() == CSS::Selector::PseudoElement::ProgressValue) {
             auto& parent_element = verify_cast<HTML::HTMLElement>(*element.root().parent_or_shadow_host());
             style = TRY(style_computer.compute_style(parent_element, element.pseudo_element()));
             display = style->display();
@@ -320,7 +318,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
                 if (!input_element.placeholder_value().has_value())
                     display = CSS::Display::from_short(CSS::Display::Short::None);
             }
-            if (element.pseudo_element() == CSS::Selector::PseudoElement::MeterOptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterSuboptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterEvenLessGoodValue) {
+            if (element.pseudo_element() == CSS::Selector::PseudoElement::MeterOptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterSuboptimumValue || element.pseudo_element() == CSS::Selector::PseudoElement::MeterEvenLessGoodValue || element.pseudo_element() == CSS::Selector::PseudoElement::ProgressValue) {
                 auto computed_style = element.computed_css_values();
                 style->set_property(CSS::PropertyID::Width, computed_style->property(CSS::PropertyID::Width));
             }
@@ -401,29 +399,6 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
         pop_parent();
     }
 
-    if (is<HTML::HTMLProgressElement>(dom_node)) {
-        auto& progress = static_cast<HTML::HTMLProgressElement&>(dom_node);
-        if (!progress.using_system_appearance()) {
-            auto bar_style = TRY(style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressBar));
-            bar_style->set_property(CSS::PropertyID::Display, CSS::DisplayStyleValue::create(CSS::Display::from_short(CSS::Display::Short::FlowRoot)));
-            auto value_style = TRY(style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressValue));
-            value_style->set_property(CSS::PropertyID::Display, CSS::DisplayStyleValue::create(CSS::Display::from_short(CSS::Display::Short::Block)));
-            value_style->set_property(CSS::PropertyID::Width, CSS::PercentageStyleValue::create(CSS::Percentage(progress.position() * 100)));
-            auto bar_display = bar_style->display();
-            auto value_display = value_style->display();
-            auto progress_bar = DOM::Element::create_layout_node_for_display_type(document, bar_display, bar_style, nullptr);
-            auto progress_value = DOM::Element::create_layout_node_for_display_type(document, value_display, value_style, nullptr);
-            push_parent(verify_cast<NodeWithStyle>(*layout_node));
-            push_parent(verify_cast<NodeWithStyle>(*progress_bar));
-            insert_node_into_inline_or_block_ancestor(*progress_value, value_display, AppendOrPrepend::Append);
-            pop_parent();
-            insert_node_into_inline_or_block_ancestor(*progress_bar, bar_display, AppendOrPrepend::Append);
-            pop_parent();
-            progress.set_pseudo_element_node({}, CSS::Selector::PseudoElement::ProgressBar, progress_bar);
-            progress.set_pseudo_element_node({}, CSS::Selector::PseudoElement::ProgressValue, progress_value);
-        }
-    }
-
     // https://html.spec.whatwg.org/multipage/rendering.html#button-layout
     // If the computed value of 'inline-size' is 'auto', then the used value is the fit-content inline size.
     if (dom_node.is_html_button_element() && dom_node.layout_node()->computed_values().width().is_auto()) {

Userland/Libraries/LibWeb/Painting/ProgressPaintable.cpp

@@ -1,48 +0,0 @@
-/*
- * Copyright (c) 2018-2022, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#include <LibGfx/StylePainter.h>
-#include <LibWeb/Painting/ProgressPaintable.h>
-
-namespace Web::Painting {
-
-JS::NonnullGCPtr<ProgressPaintable> ProgressPaintable::create(Layout::Progress const& layout_box)
-{
-    return layout_box.heap().allocate_without_realm<ProgressPaintable>(layout_box);
-}
-
-ProgressPaintable::ProgressPaintable(Layout::Progress const& layout_box)
-    : PaintableBox(layout_box)
-{
-}
-
-Layout::Progress const& ProgressPaintable::layout_box() const
-{
-    return static_cast<Layout::Progress const&>(layout_node());
-}
-
-void ProgressPaintable::paint(PaintContext& context, PaintPhase phase) const
-{
-    if (!is_visible())
-        return;
-
-    if (phase == PaintPhase::Foreground) {
-        auto progress_rect = context.rounded_device_rect(absolute_rect());
-        auto min_frame_thickness = context.rounded_device_pixels(3);
-        auto frame_thickness = min(min(progress_rect.width(), progress_rect.height()) / 6, min_frame_thickness);
-
-        context.recording_painter().paint_progressbar(
-            progress_rect.to_type<int>(),
-            progress_rect.shrunken(frame_thickness, frame_thickness).to_type<int>(),
-            context.palette(),
-            0,
-            round_to<int>(layout_box().dom_node().max()),
-            round_to<int>(layout_box().dom_node().value()),
-            ""sv);
-    }
-}
-
-}

Userland/Libraries/LibWeb/Painting/ProgressPaintable.h

@@ -1,31 +0,0 @@
-/*
- * Copyright (c) 2022, Andreas Kling <kling@serenityos.org>
- *
- * SPDX-License-Identifier: BSD-2-Clause
- */
-
-#pragma once
-
-#include <LibWeb/Layout/Progress.h>
-#include <LibWeb/Painting/PaintableBox.h>
-
-namespace Web::Painting {
-
-// FIXME: ProgressPaintable should inherit from LabelablePaintable, as it is a LabelableNode.
-//        LabelablePaintable should be split into FormAssociatedLabelablePaintable once this
-//        happens.
-class ProgressPaintable final : public PaintableBox {
-    JS_CELL(ProgressPaintable, PaintableBox);
-
-public:
-    static JS::NonnullGCPtr<ProgressPaintable> create(Layout::Progress const&);
-
-    virtual void paint(PaintContext&, PaintPhase) const override;
-
-    Layout::Progress const& layout_box() const;
-
-private:
-    ProgressPaintable(Layout::Progress const&);
-};
-
-}