From ddabc44d4273f18207c2593a1a3a6c0a7da01685 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 7 Sep 2024 11:07:04 +0200 Subject: [PATCH 01/43] LibWeb: Keep custom properties from all cascade layers Before this change, we were cascading custom properties for each layer, and then replacing any previously cascaded properties for the element with only the set from this latest layer. The patch fixes the issue by making each pass of the custom property cascade add to the same set, and then finally assigning that set of properties to the element. (cherry picked from commit 95bd0602ba079e0bd7608e39bcbc3b0403ed89d1) --- .../css/custom-properties-from-all-layers.txt | 4 ++++ .../custom-properties-from-all-layers.html | 23 +++++++++++++++++++ .../Libraries/LibWeb/CSS/StyleComputer.cpp | 12 +++++----- 3 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt create mode 100644 Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html diff --git a/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt b/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt new file mode 100644 index 00000000000000..8b6f94330d6490 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/custom-properties-from-all-layers.txt @@ -0,0 +1,4 @@ + 1px +2px +3px +4px diff --git a/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html b/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html new file mode 100644 index 00000000000000..c8251a21e0cbcc --- /dev/null +++ b/Tests/LibWeb/Text/input/css/custom-properties-from-all-layers.html @@ -0,0 +1,23 @@ +
+ + diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index c7c91442596b49..ef04f045f6a833 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -892,7 +892,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e } } -static void cascade_custom_properties(DOM::Element& element, Optional pseudo_element, Vector const& matching_rules) +static void cascade_custom_properties(DOM::Element& element, Optional pseudo_element, Vector const& matching_rules, HashMap& custom_properties) { size_t needed_capacity = 0; for (auto const& matching_rule : matching_rules) @@ -903,8 +903,7 @@ static void cascade_custom_properties(DOM::Element& element, Optionalcustom_properties().size(); } - HashMap custom_properties; - custom_properties.ensure_capacity(needed_capacity); + custom_properties.ensure_capacity(custom_properties.size() + needed_capacity); for (auto const& matching_rule : matching_rules) { for (auto const& it : matching_rule.rule->declaration().custom_properties()) { @@ -921,8 +920,6 @@ static void cascade_custom_properties(DOM::Element& element, Optional pseudo_element, JS::NonnullGCPtr effect, StyleProperties& style_properties, AnimationRefresh refresh) const @@ -1417,9 +1414,12 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element // Then we resolve all the CSS custom properties ("variables") for this element: // FIXME: Also resolve !important custom properties, in a second cascade. + + HashMap custom_properties; for (auto& layer : matching_rule_set.author_rules) { - cascade_custom_properties(element, pseudo_element, layer.rules); + cascade_custom_properties(element, pseudo_element, layer.rules, custom_properties); } + element.set_custom_properties(pseudo_element, move(custom_properties)); // Then we apply the declarations from the matched rules in cascade order: From d66778f9da4388bf1d84fbd59e42f0a1c78fffb7 Mon Sep 17 00:00:00 2001 From: ronak69 Date: Wed, 18 Sep 2024 12:23:12 +0000 Subject: [PATCH 02/43] LibWebView: Do floating-point-error-free zoom calculation using rounding The current min/max zoom levels are supposed to be: 30% and 500%. Before, due to floating point error accumulation in incremental addition of zoom-step into zoom-level, one extra zoom step would get allowed, enabling user to zoom 20%-to-510%. Now, using rounding, the intermediate zoom-level values should be as close to the theoretical value as FP32 can represent. E.g. zoom-level of 70% (theoretical multiplier 0.7) is 0.69... . (cherry picked from commit 96335f31d5a8c6180a4cc0254585bc01ba6742fa) --- Userland/Libraries/LibWebView/ViewImplementation.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWebView/ViewImplementation.cpp b/Userland/Libraries/LibWebView/ViewImplementation.cpp index b926d52371261b..1b6d6b02996b3e 100644 --- a/Userland/Libraries/LibWebView/ViewImplementation.cpp +++ b/Userland/Libraries/LibWebView/ViewImplementation.cpp @@ -104,7 +104,7 @@ void ViewImplementation::zoom_in() { if (m_zoom_level >= ZOOM_MAX_LEVEL) return; - m_zoom_level += ZOOM_STEP; + m_zoom_level = round_to((m_zoom_level + ZOOM_STEP) * 100) / 100.0f; update_zoom(); } @@ -112,7 +112,7 @@ void ViewImplementation::zoom_out() { if (m_zoom_level <= ZOOM_MIN_LEVEL) return; - m_zoom_level -= ZOOM_STEP; + m_zoom_level = round_to((m_zoom_level - ZOOM_STEP) * 100) / 100.0f; update_zoom(); } From d9f0de7165b974090b5e2029e298f2df8fa8545b Mon Sep 17 00:00:00 2001 From: Annya Date: Mon, 16 Sep 2024 19:38:14 +0800 Subject: [PATCH 03/43] LibWeb: Implement Range's extension method This patch implements `Range::getClientRects` and `Range::getBoundingClientRect`. Since the rects returned by invoking getClientRects can be accessed without adding them to the Selection, `ViewportPaintable::recompute_selection_states` has been updated to accept a Range as a parameter, rather than acquiring it through the Document's Selection. With this change, the following tests now pass: - wpt[css/cssom-view/range-bounding-client-rect-with-nested-text.html] - wpt[css/cssom-view/DOMRectList.html] Note: The test "css/cssom-view/range-bounding-client-rect-with-display-contents.html" still fails due to an issue with Element::getClientRects, which will be addressed in a future commit. (cherry picked from commit 75c7dbc5d2dd045733a4c319aeab6644b5b7b36d) --- ...-bounding-client-rect-with-nested-text.txt | 2 + .../expected/DOM/range-get-client-rects.txt | 2 + ...bounding-client-rect-with-nested-text.html | 47 ++++++++ .../input/DOM/range-get-client-rects.html | 19 ++++ Userland/Libraries/LibWeb/DOM/Document.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Range.cpp | 103 ++++++++++++++++-- Userland/Libraries/LibWeb/DOM/Range.h | 4 +- .../Libraries/LibWeb/Painting/Paintable.h | 3 + .../LibWeb/Painting/PaintableFragment.cpp | 43 +++++--- .../LibWeb/Painting/PaintableFragment.h | 1 + .../LibWeb/Painting/ViewportPaintable.cpp | 34 ++++-- .../LibWeb/Painting/ViewportPaintable.h | 3 +- 12 files changed, 225 insertions(+), 38 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/DOM/range-bounding-client-rect-with-nested-text.txt create mode 100644 Tests/LibWeb/Text/expected/DOM/range-get-client-rects.txt create mode 100644 Tests/LibWeb/Text/input/DOM/range-bounding-client-rect-with-nested-text.html create mode 100644 Tests/LibWeb/Text/input/DOM/range-get-client-rects.html diff --git a/Tests/LibWeb/Text/expected/DOM/range-bounding-client-rect-with-nested-text.txt b/Tests/LibWeb/Text/expected/DOM/range-bounding-client-rect-with-nested-text.txt new file mode 100644 index 00000000000000..eeaea842de5988 --- /dev/null +++ b/Tests/LibWeb/Text/expected/DOM/range-bounding-client-rect-with-nested-text.txt @@ -0,0 +1,2 @@ + Hello Ladybird Ladybird again World 6 +4 diff --git a/Tests/LibWeb/Text/expected/DOM/range-get-client-rects.txt b/Tests/LibWeb/Text/expected/DOM/range-get-client-rects.txt new file mode 100644 index 00000000000000..7c6c2d09a74e7e --- /dev/null +++ b/Tests/LibWeb/Text/expected/DOM/range-get-client-rects.txt @@ -0,0 +1,2 @@ + x [object DOMRectList] +[object DOMRect] \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/DOM/range-bounding-client-rect-with-nested-text.html b/Tests/LibWeb/Text/input/DOM/range-bounding-client-rect-with-nested-text.html new file mode 100644 index 00000000000000..ec3cae1eb889d6 --- /dev/null +++ b/Tests/LibWeb/Text/input/DOM/range-bounding-client-rect-with-nested-text.html @@ -0,0 +1,47 @@ + + + +All the rectangles for the text nodes must included in getClientRects + + +
+
+
Hello
+
+
+
Ladybird
+
+
+
Ladybird again
+
+
+
World
+
+
+ diff --git a/Tests/LibWeb/Text/input/DOM/range-get-client-rects.html b/Tests/LibWeb/Text/input/DOM/range-get-client-rects.html new file mode 100644 index 00000000000000..dcfd402af0bfd4 --- /dev/null +++ b/Tests/LibWeb/Text/input/DOM/range-get-client-rects.html @@ -0,0 +1,19 @@ + + +
x
+ + + + diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 83e8c16fb9d8a1..ab0b33336a37ea 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1165,7 +1165,7 @@ void Document::update_layout() page().client().page_did_layout(); } - paintable()->recompute_selection_states(); + paintable()->update_selection(); m_needs_layout = false; diff --git a/Userland/Libraries/LibWeb/DOM/Range.cpp b/Userland/Libraries/LibWeb/DOM/Range.cpp index 0a4ab926f1d4f8..19c9e741800c0a 100644 --- a/Userland/Libraries/LibWeb/DOM/Range.cpp +++ b/Userland/Libraries/LibWeb/DOM/Range.cpp @@ -24,7 +24,7 @@ #include #include #include -#include +#include #include namespace Web::DOM { @@ -97,7 +97,8 @@ void Range::set_associated_selection(Badge, JS::GCPtrdocument().paintable()) { - viewport->recompute_selection_states(); + viewport->recompute_selection_states(*this); + viewport->update_selection(); viewport->set_needs_display(); } @@ -1165,17 +1166,103 @@ WebIDL::ExceptionOr Range::delete_contents() } // https://drafts.csswg.org/cssom-view/#dom-element-getclientrects -JS::NonnullGCPtr Range::get_client_rects() const +// https://drafts.csswg.org/cssom-view/#extensions-to-the-range-interface +JS::NonnullGCPtr Range::get_client_rects() { - dbgln("(STUBBED) Range::get_client_rects()"); - return Geometry::DOMRectList::create(realm(), {}); + // 1. return an empty DOMRectList object if the range is not in the document + if (!start_container()->document().navigable()) + return Geometry::DOMRectList::create(realm(), {}); + + start_container()->document().update_layout(); + update_associated_selection(); + Vector> rects; + // FIXME: take Range collapsed into consideration + // 2. Iterate the node included in Range + auto start_node = start_container(); + auto end_node = end_container(); + if (!is(start_node)) { + start_node = start_node->child_at_index(m_start_offset); + } + if (!is(end_node)) { + // end offset shouldn't be 0 + if (m_end_offset == 0) + return Geometry::DOMRectList::create(realm(), {}); + end_node = end_node->child_at_index(m_end_offset - 1); + } + for (Node const* node = start_node; node && node != end_node->next_in_pre_order(); node = node->next_in_pre_order()) { + auto node_type = static_cast(node->node_type()); + if (node_type == NodeType::ELEMENT_NODE) { + // 1. For each element selected by the range, whose parent is not selected by the range, include the border + // areas returned by invoking getClientRects() on the element. + if (contains_node(*node) && !contains_node(*node->parent())) { + auto const& element = static_cast(*node); + JS::NonnullGCPtr const element_rects = element.get_client_rects(); + for (u32 i = 0; i < element_rects->length(); i++) { + auto rect = element_rects->item(i); + rects.append(Geometry::DOMRect::create(realm(), + Gfx::FloatRect(rect->x(), rect->y(), rect->width(), rect->height()))); + } + } + } else if (node_type == NodeType::TEXT_NODE) { + // 2. For each Text node selected or partially selected by the range (including when the boundary-points + // are identical), include scaled DOMRect object (for the part that is selected, not the whole line box). + auto const& text = static_cast(*node); + auto const* paintable = text.paintable(); + if (paintable) { + auto const* containing_block = paintable->containing_block(); + if (is(*containing_block)) { + auto const& paintable_lines = static_cast(*containing_block); + auto fragments = paintable_lines.fragments(); + auto const& font = paintable->layout_node().first_available_font(); + for (auto frag = fragments.begin(); frag != fragments.end(); frag++) { + auto rect = frag->range_rect(font, *this); + if (rect.is_empty()) + continue; + rects.append(Geometry::DOMRect::create(realm(), + Gfx::FloatRect(rect))); + } + } else { + dbgln("FIXME: Failed to get client rects for node {}", node->debug_description()); + } + } + } + } + return Geometry::DOMRectList::create(realm(), move(rects)); } // https://w3c.github.io/csswg-drafts/cssom-view/#dom-range-getboundingclientrect -JS::NonnullGCPtr Range::get_bounding_client_rect() const +JS::NonnullGCPtr Range::get_bounding_client_rect() { - dbgln("(STUBBED) Range::get_bounding_client_rect()"); - return Geometry::DOMRect::construct_impl(realm(), 0, 0, 0, 0).release_value_but_fixme_should_propagate_errors(); + // 1. Let list be the result of invoking getClientRects() on element. + auto list = get_client_rects(); + + // 2. If the list is empty return a DOMRect object whose x, y, width and height members are zero. + if (list->length() == 0) + return Geometry::DOMRect::construct_impl(realm(), 0, 0, 0, 0).release_value_but_fixme_should_propagate_errors(); + + // 3. If all rectangles in list have zero width or height, return the first rectangle in list. + auto all_rectangle_has_zero_width_or_height = true; + for (auto i = 0u; i < list->length(); ++i) { + auto const& rect = list->item(i); + if (rect->width() != 0 && rect->height() != 0) { + all_rectangle_has_zero_width_or_height = false; + break; + } + } + if (all_rectangle_has_zero_width_or_height) + return JS::NonnullGCPtr { *const_cast(list->item(0)) }; + + // 4. Otherwise, return a DOMRect object describing the smallest rectangle that includes all of the rectangles in + // list of which the height or width is not zero. + auto const* first_rect = list->item(0); + auto bounding_rect = Gfx::Rect { first_rect->x(), first_rect->y(), first_rect->width(), first_rect->height() }; + for (auto i = 1u; i < list->length(); ++i) { + auto const& rect = list->item(i); + if (rect->width() == 0 || rect->height() == 0) + continue; + bounding_rect = bounding_rect.united({ rect->x(), rect->y(), rect->width(), rect->height() }); + } + return Geometry::DOMRect::create(realm(), bounding_rect.to_type()); } // https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#dom-range-createcontextualfragment diff --git a/Userland/Libraries/LibWeb/DOM/Range.h b/Userland/Libraries/LibWeb/DOM/Range.h index 748318ac54380c..8c6031b08d89a6 100644 --- a/Userland/Libraries/LibWeb/DOM/Range.h +++ b/Userland/Libraries/LibWeb/DOM/Range.h @@ -88,8 +88,8 @@ class Range final : public AbstractRange { static HashTable& live_ranges(); - JS::NonnullGCPtr get_client_rects() const; - JS::NonnullGCPtr get_bounding_client_rect() const; + JS::NonnullGCPtr get_client_rects(); + JS::NonnullGCPtr get_bounding_client_rect(); bool contains_node(Node const&) const; diff --git a/Userland/Libraries/LibWeb/Painting/Paintable.h b/Userland/Libraries/LibWeb/Painting/Paintable.h index 1e8d1dd434d158..4dec0b7ead4ecf 100644 --- a/Userland/Libraries/LibWeb/Painting/Paintable.h +++ b/Userland/Libraries/LibWeb/Painting/Paintable.h @@ -57,6 +57,7 @@ class Paintable [[nodiscard]] bool is_absolutely_positioned() const { return m_absolutely_positioned; } [[nodiscard]] bool is_floating() const { return m_floating; } [[nodiscard]] bool is_inline() const { return m_inline; } + [[nodiscard]] bool is_selected() const { return m_selected; } [[nodiscard]] CSS::Display display() const { return layout_node().display(); } template @@ -212,6 +213,7 @@ class Paintable SelectionState selection_state() const { return m_selection_state; } void set_selection_state(SelectionState state) { m_selection_state = state; } + void set_selected(bool selected) { m_selected = selected; } Gfx::AffineTransform compute_combined_css_transform() const; @@ -237,6 +239,7 @@ class Paintable bool m_absolutely_positioned : 1 { false }; bool m_floating : 1 { false }; bool m_inline : 1 { false }; + bool m_selected : 1 { false }; }; inline DOM::Node* HitTestResult::dom_node() diff --git a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp index 388679031b941f..3fc0b5cf4796b0 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp +++ b/Userland/Libraries/LibWeb/Painting/PaintableFragment.cpp @@ -59,8 +59,7 @@ int PaintableFragment::text_index_at(CSSPixels x) const return m_start + m_length; } - -CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const +CSSPixelRect PaintableFragment::range_rect(Gfx::Font const& font, DOM::Range const& range) const { if (paintable().selection_state() == Paintable::SelectionState::None) return {}; @@ -68,13 +67,6 @@ CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const if (paintable().selection_state() == Paintable::SelectionState::Full) return absolute_rect(); - auto selection = paintable().document().get_selection(); - if (!selection) - return {}; - auto range = selection->range(); - if (!range) - return {}; - // FIXME: m_start and m_length should be unsigned and then we won't need these casts. auto const start_index = static_cast(m_start); auto const end_index = static_cast(m_start) + static_cast(m_length); @@ -83,16 +75,16 @@ CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const if (paintable().selection_state() == Paintable::SelectionState::StartAndEnd) { // we are in the start/end node (both the same) - if (start_index > range->end_offset()) + if (start_index > range.end_offset()) return {}; - if (end_index < range->start_offset()) + if (end_index < range.start_offset()) return {}; - if (range->start_offset() == range->end_offset()) + if (range.start_offset() == range.end_offset()) return {}; - auto selection_start_in_this_fragment = max(0, range->start_offset() - m_start); - auto selection_end_in_this_fragment = min(m_length, range->end_offset() - m_start); + auto selection_start_in_this_fragment = max(0, range.start_offset() - m_start); + auto selection_end_in_this_fragment = min(m_length, range.end_offset() - m_start); auto pixel_distance_to_first_selected_character = CSSPixels::nearest_value_for(font.width(text.substring_view(0, selection_start_in_this_fragment))); auto pixel_width_of_selection = CSSPixels::nearest_value_for(font.width(text.substring_view(selection_start_in_this_fragment, selection_end_in_this_fragment - selection_start_in_this_fragment))) + 1; @@ -104,10 +96,10 @@ CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const } if (paintable().selection_state() == Paintable::SelectionState::Start) { // we are in the start node - if (end_index < range->start_offset()) + if (end_index < range.start_offset()) return {}; - auto selection_start_in_this_fragment = max(0, range->start_offset() - m_start); + auto selection_start_in_this_fragment = max(0, range.start_offset() - m_start); auto selection_end_in_this_fragment = m_length; auto pixel_distance_to_first_selected_character = CSSPixels::nearest_value_for(font.width(text.substring_view(0, selection_start_in_this_fragment))); auto pixel_width_of_selection = CSSPixels::nearest_value_for(font.width(text.substring_view(selection_start_in_this_fragment, selection_end_in_this_fragment - selection_start_in_this_fragment))) + 1; @@ -120,11 +112,11 @@ CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const } if (paintable().selection_state() == Paintable::SelectionState::End) { // we are in the end node - if (start_index > range->end_offset()) + if (start_index > range.end_offset()) return {}; auto selection_start_in_this_fragment = 0; - auto selection_end_in_this_fragment = min(range->end_offset() - m_start, m_length); + auto selection_end_in_this_fragment = min(range.end_offset() - m_start, m_length); auto pixel_distance_to_first_selected_character = CSSPixels::nearest_value_for(font.width(text.substring_view(0, selection_start_in_this_fragment))); auto pixel_width_of_selection = CSSPixels::nearest_value_for(font.width(text.substring_view(selection_start_in_this_fragment, selection_end_in_this_fragment - selection_start_in_this_fragment))) + 1; @@ -137,6 +129,21 @@ CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const return {}; } +CSSPixelRect PaintableFragment::selection_rect(Gfx::Font const& font) const +{ + if (!paintable().is_selected()) + return {}; + + auto selection = paintable().document().get_selection(); + if (!selection) + return {}; + auto range = selection->range(); + if (!range) + return {}; + + return range_rect(font, *range); +} + StringView PaintableFragment::string_view() const { if (!is(paintable())) diff --git a/Userland/Libraries/LibWeb/Painting/PaintableFragment.h b/Userland/Libraries/LibWeb/Painting/PaintableFragment.h index f044d336ee374d..a255b15fd636a0 100644 --- a/Userland/Libraries/LibWeb/Painting/PaintableFragment.h +++ b/Userland/Libraries/LibWeb/Painting/PaintableFragment.h @@ -42,6 +42,7 @@ class PaintableFragment { RefPtr glyph_run() const { return m_glyph_run; } CSSPixelRect selection_rect(Gfx::Font const&) const; + CSSPixelRect range_rect(Gfx::Font const&, DOM::Range const&) const; CSSPixels width() const { return m_size.width(); } CSSPixels height() const { return m_size.height(); } diff --git a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp index dc5aaba47fee9d..a3423ec034fb3f 100644 --- a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.cpp @@ -203,11 +203,11 @@ JS::GCPtr ViewportPaintable::selection() const return const_cast(document()).get_selection(); } -void ViewportPaintable::recompute_selection_states() +void ViewportPaintable::update_selection() { - // 1. Start by resetting the selection state of all layout nodes to None. + // 1. Start by setting all layout nodes to unselected. for_each_in_inclusive_subtree([&](auto& layout_node) { - layout_node.set_selection_state(SelectionState::None); + layout_node.set_selected(false); return TraversalDecision::Continue; }); @@ -222,10 +222,28 @@ void ViewportPaintable::recompute_selection_states() auto* start_container = range->start_container(); auto* end_container = range->end_container(); - // 3. If the selection starts and ends in the same node: + // 3. Mark the nodes included in range selected. + for (auto* node = start_container; node && node != end_container->next_in_pre_order(); node = node->next_in_pre_order()) { + if (auto* paintable = node->paintable()) + paintable->set_selected(true); + } +} + +void ViewportPaintable::recompute_selection_states(DOM::Range& range) +{ + // 1. Start by resetting the selection state of all layout nodes to None. + for_each_in_inclusive_subtree([&](auto& layout_node) { + layout_node.set_selection_state(SelectionState::None); + return TraversalDecision::Continue; + }); + + auto* start_container = range.start_container(); + auto* end_container = range.end_container(); + + // 2. If the selection starts and ends in the same node: if (start_container == end_container) { // 1. If the selection starts and ends at the same offset, return. - if (range->start_offset() == range->end_offset()) { + if (range.start_offset() == range.end_offset()) { // NOTE: A zero-length selection should not be visible. return; } @@ -238,7 +256,7 @@ void ViewportPaintable::recompute_selection_states() } } - // 4. Mark the selection start node as Start (if text) or Full (if anything else). + // 3. Mark the selection start node as Start (if text) or Full (if anything else). if (auto* paintable = start_container->paintable()) { if (is(*start_container)) paintable->set_selection_state(SelectionState::Start); @@ -246,7 +264,7 @@ void ViewportPaintable::recompute_selection_states() paintable->set_selection_state(SelectionState::Full); } - // 5. Mark the selection end node as End (if text) or Full (if anything else). + // 4. Mark the selection end node as End (if text) or Full (if anything else). if (auto* paintable = end_container->paintable()) { if (is(*end_container)) paintable->set_selection_state(SelectionState::End); @@ -254,7 +272,7 @@ void ViewportPaintable::recompute_selection_states() paintable->set_selection_state(SelectionState::Full); } - // 6. Mark the nodes between start node and end node (in tree order) as Full. + // 5. Mark the nodes between start node and end node (in tree order) as Full. for (auto* node = start_container->next_in_pre_order(); node && node != end_container; node = node->next_in_pre_order()) { if (auto* paintable = node->paintable()) paintable->set_selection_state(SelectionState::Full); diff --git a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.h b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.h index deba2b5fbd5e1b..7fa68fe74dc4cf 100644 --- a/Userland/Libraries/LibWeb/Painting/ViewportPaintable.h +++ b/Userland/Libraries/LibWeb/Painting/ViewportPaintable.h @@ -32,7 +32,8 @@ class ViewportPaintable final : public PaintableWithLines { void resolve_paint_only_properties(); JS::GCPtr selection() const; - void recompute_selection_states(); + void recompute_selection_states(DOM::Range&); + void update_selection(); bool handle_mousewheel(Badge, CSSPixelPoint, unsigned, unsigned, int wheel_delta_x, int wheel_delta_y) override; From 4c386e03e1bfc895d6223ee7079a663e1e85825b Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 21 Sep 2024 07:47:37 +0200 Subject: [PATCH 04/43] LibWeb: Add StyleElementUtils::visit_edges() Let's do this instead of making embedders visit every field of this helper class. (cherry picked from commit 2064be708f7318f72553fb210f918fe167e08257) --- Userland/Libraries/LibWeb/DOM/StyleElementUtils.h | 5 +++++ Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp | 2 +- Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h index f0879ff36c1420..f592dab7d8d701 100644 --- a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h +++ b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h @@ -19,6 +19,11 @@ class StyleElementUtils { CSS::CSSStyleSheet* sheet() { return m_associated_css_style_sheet; } CSS::CSSStyleSheet const* sheet() const { return m_associated_css_style_sheet; } + void visit_edges(JS::Cell::Visitor& visitor) + { + visitor.visit(m_associated_css_style_sheet); + } + private: // https://www.w3.org/TR/cssom/#associated-css-style-sheet JS::GCPtr m_associated_css_style_sheet; diff --git a/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp index 33126fb0bb048b..693c7f7a468e27 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp @@ -29,7 +29,7 @@ void HTMLStyleElement::initialize(JS::Realm& realm) void HTMLStyleElement::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_style_element_utils.sheet()); + m_style_element_utils.visit_edges(visitor); } void HTMLStyleElement::children_changed() diff --git a/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp index 86ee2fd8b4c867..6904e56658cc15 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp @@ -27,7 +27,7 @@ void SVGStyleElement::initialize(JS::Realm& realm) void SVGStyleElement::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); - visitor.visit(m_style_element_utils.sheet()); + m_style_element_utils.visit_edges(visitor); } void SVGStyleElement::children_changed() From 01a9d5ae954cf0c0bc4a4d7321b6a2d445435847 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 21 Sep 2024 08:11:49 +0200 Subject: [PATCH 05/43] LibWeb: Let style elements remember which StyleSheetList they live in Instead of trying to locate the relevant StyleSheetList on style element removal from the DOM, we now simply keep a pointer to the list instead. This fixes an issue where using attachShadow() on an element that had a declarative shadow DOM would cause any style elements present to use the wrong StyleSheetList when removing themselves from the tree. (cherry picked from commit 8543b8ad6a4d96bd0c247052572b8c2abf3929a7) --- ...clarative-shadow-root-with-style-sheet.txt | 1 + ...larative-shadow-root-with-style-sheet.html | 9 ++++++ .../LibWeb/DOM/StyleElementUtils.cpp | 29 +++++++------------ .../Libraries/LibWeb/DOM/StyleElementUtils.h | 12 ++++---- .../LibWeb/HTML/HTMLStyleElement.cpp | 2 +- .../Libraries/LibWeb/SVG/SVGStyleElement.cpp | 2 +- 6 files changed, 30 insertions(+), 25 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.txt create mode 100644 Tests/LibWeb/Text/input/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.html diff --git a/Tests/LibWeb/Text/expected/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.txt b/Tests/LibWeb/Text/expected/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.txt new file mode 100644 index 00000000000000..39701378c55653 --- /dev/null +++ b/Tests/LibWeb/Text/expected/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.txt @@ -0,0 +1 @@ + PASS (didn't crash) diff --git a/Tests/LibWeb/Text/input/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.html b/Tests/LibWeb/Text/input/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.html new file mode 100644 index 00000000000000..e9e64053ab5aef --- /dev/null +++ b/Tests/LibWeb/Text/input/ShadowDOM/replace-declarative-shadow-root-with-style-sheet.html @@ -0,0 +1,9 @@ +
+ + diff --git a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp index fea12d0583b54d..ba251894bd8c34 100644 --- a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp +++ b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.cpp @@ -13,15 +13,6 @@ namespace Web::DOM { -static CSS::StyleSheetList& relevant_style_sheet_list_for_node(DOM::Node& node) -{ - auto& root_node = node.root(); - if (is(root_node)) - return static_cast(root_node).style_sheets(); - - return node.document().style_sheets(); -} - // The user agent must run the "update a style block" algorithm whenever one of the following conditions occur: // FIXME: The element is popped off the stack of open elements of an HTML parser or XML parser. // @@ -32,7 +23,7 @@ static CSS::StyleSheetList& relevant_style_sheet_list_for_node(DOM::Node& node) // The element is not on the stack of open elements of an HTML parser or XML parser, and it becomes connected or disconnected. // // https://html.spec.whatwg.org/multipage/semantics.html#update-a-style-block -void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GCPtr old_parent_if_removed_from) +void StyleElementUtils::update_a_style_block(DOM::Element& style_element) { // OPTIMIZATION: Skip parsing CSS if we're in the middle of parsing a HTML fragment. // The style block will be parsed upon insertion into a proper document. @@ -43,13 +34,8 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC // 2. If element has an associated CSS style sheet, remove the CSS style sheet in question. if (m_associated_css_style_sheet) { - // NOTE: If we're here in response to a node being removed from the tree, we need to remove the stylesheet from the style scope - // of the old parent, not the style scope of the node itself, since it's too late to find it that way! - if (old_parent_if_removed_from) { - relevant_style_sheet_list_for_node(*old_parent_if_removed_from).remove_a_css_style_sheet(*m_associated_css_style_sheet); - } else { - style_element.document_or_shadow_root_style_sheets().remove_a_css_style_sheet(*m_associated_css_style_sheet); - } + m_style_sheet_list->remove_a_css_style_sheet(*m_associated_css_style_sheet); + m_style_sheet_list = nullptr; // FIXME: This should probably be handled by StyleSheet::set_owner_node(). m_associated_css_style_sheet = nullptr; @@ -76,7 +62,8 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC m_associated_css_style_sheet = sheet; // 6. Create a CSS style sheet with the following properties... - style_element.document_or_shadow_root_style_sheets().create_a_css_style_sheet( + m_style_sheet_list = style_element.document_or_shadow_root_style_sheets(); + m_style_sheet_list->create_a_css_style_sheet( "text/css"_string, &style_element, style_element.attribute(HTML::AttributeNames::media).value_or({}), @@ -91,4 +78,10 @@ void StyleElementUtils::update_a_style_block(DOM::Element& style_element, JS::GC *sheet); } +void StyleElementUtils::visit_edges(JS::Cell::Visitor& visitor) +{ + visitor.visit(m_associated_css_style_sheet); + visitor.visit(m_style_sheet_list); +} + } diff --git a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h index f592dab7d8d701..d7e2fa1acd2555 100644 --- a/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h +++ b/Userland/Libraries/LibWeb/DOM/StyleElementUtils.h @@ -14,19 +14,21 @@ namespace Web::DOM { class StyleElementUtils { public: - void update_a_style_block(DOM::Element& style_element, JS::GCPtr old_parent_if_removed_from = nullptr); + void update_a_style_block(DOM::Element& style_element); CSS::CSSStyleSheet* sheet() { return m_associated_css_style_sheet; } CSS::CSSStyleSheet const* sheet() const { return m_associated_css_style_sheet; } - void visit_edges(JS::Cell::Visitor& visitor) - { - visitor.visit(m_associated_css_style_sheet); - } + [[nodiscard]] JS::GCPtr style_sheet_list() { return m_style_sheet_list; } + [[nodiscard]] JS::GCPtr style_sheet_list() const { return m_style_sheet_list; } + + void visit_edges(JS::Cell::Visitor&); private: // https://www.w3.org/TR/cssom/#associated-css-style-sheet JS::GCPtr m_associated_css_style_sheet; + + JS::GCPtr m_style_sheet_list; }; } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp index 693c7f7a468e27..84a1273159156f 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLStyleElement.cpp @@ -46,7 +46,7 @@ void HTMLStyleElement::inserted() void HTMLStyleElement::removed_from(Node* old_parent) { - m_style_element_utils.update_a_style_block(*this, old_parent); + m_style_element_utils.update_a_style_block(*this); Base::removed_from(old_parent); } diff --git a/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp index 6904e56658cc15..7e41c25b3b12dc 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGStyleElement.cpp @@ -44,7 +44,7 @@ void SVGStyleElement::inserted() void SVGStyleElement::removed_from(Node* old_parent) { - m_style_element_utils.update_a_style_block(*this, old_parent); + m_style_element_utils.update_a_style_block(*this); Base::removed_from(old_parent); } From a2315d8b84a2a9209cdf251487794c3355a30192 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Sat, 21 Sep 2024 06:25:26 +0100 Subject: [PATCH 06/43] LibWeb: Return a WindowProxy from `document.defaultView` This aligns our implementation with the most recent specification steps. (cherry picked from commit 089139f09dbc78455c5c09916cbc97e33f823ca0) --- .../Text/expected/DOM/Document-defaultView.txt | 1 + .../Text/input/DOM/Document-defaultView.html | 7 +++++++ Userland/Libraries/LibWeb/DOM/Document.cpp | 16 ++++++++++++++++ Userland/Libraries/LibWeb/DOM/Document.h | 4 ++-- Userland/Libraries/LibWeb/DOM/Document.idl | 2 +- Userland/Libraries/LibWeb/DOM/Element.cpp | 16 +++++++++++----- 6 files changed, 38 insertions(+), 8 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/DOM/Document-defaultView.txt create mode 100644 Tests/LibWeb/Text/input/DOM/Document-defaultView.html diff --git a/Tests/LibWeb/Text/expected/DOM/Document-defaultView.txt b/Tests/LibWeb/Text/expected/DOM/Document-defaultView.txt new file mode 100644 index 00000000000000..aa721174eec93e --- /dev/null +++ b/Tests/LibWeb/Text/expected/DOM/Document-defaultView.txt @@ -0,0 +1 @@ +document.defaultView === window: true diff --git a/Tests/LibWeb/Text/input/DOM/Document-defaultView.html b/Tests/LibWeb/Text/input/DOM/Document-defaultView.html new file mode 100644 index 00000000000000..5bf173fd6af6d3 --- /dev/null +++ b/Tests/LibWeb/Text/input/DOM/Document-defaultView.html @@ -0,0 +1,7 @@ + + + diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index ab0b33336a37ea..9d7be26122dfa9 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -694,6 +694,22 @@ WebIDL::ExceptionOr Document::close() return {}; } +// https://html.spec.whatwg.org/multipage/nav-history-apis.html#dom-document-defaultview +JS::GCPtr Document::default_view() +{ + // If this's browsing context is null, then return null. + if (!browsing_context()) + return {}; + + // 2. Return this's browsing context's WindowProxy object. + return browsing_context()->window_proxy(); +} + +JS::GCPtr Document::default_view() const +{ + return const_cast(this)->default_view(); +} + HTML::Origin Document::origin() const { return m_origin; diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index c38f96a3ab3374..cb67ccdcecd85a 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -367,8 +367,8 @@ class Document WebIDL::ExceptionOr> open(StringView url, StringView name, StringView features); WebIDL::ExceptionOr close(); - HTML::Window* default_view() { return m_window.ptr(); } - HTML::Window const* default_view() const { return m_window.ptr(); } + JS::GCPtr default_view() const; + JS::GCPtr default_view(); String const& content_type() const { return m_content_type; } void set_content_type(String content_type) { m_content_type = move(content_type); } diff --git a/Userland/Libraries/LibWeb/DOM/Document.idl b/Userland/Libraries/LibWeb/DOM/Document.idl index 3ee2e8aa4ea9d3..859e91329e23ef 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.idl +++ b/Userland/Libraries/LibWeb/DOM/Document.idl @@ -48,7 +48,7 @@ interface Document : Node { readonly attribute DOMString inputEncoding; readonly attribute DOMString contentType; - readonly attribute Window? defaultView; + readonly attribute WindowProxy? defaultView; [CEReactions] Document open(optional DOMString unused1, optional DOMString unused2); WindowProxy? open(USVString url, DOMString name, DOMString features); diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 98e6d7d9d55cee..d6488d6895f699 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -1240,7 +1240,8 @@ double Element::scroll_top() const return 0.0; // 3. Let window be the value of document’s defaultView attribute. - auto* window = document.default_view(); + // FIXME: The specification expects defaultView to be a Window object, but defaultView actually returns a WindowProxy object. + auto window = document.window(); // 4. If window is null, return zero and terminate these steps. if (!window) @@ -1270,6 +1271,7 @@ double Element::scroll_top() const return paintable_box()->scroll_offset().y().to_double(); } +// https://drafts.csswg.org/cssom-view/#dom-element-scrollleft double Element::scroll_left() const { // 1. Let document be the element’s node document. @@ -1280,7 +1282,8 @@ double Element::scroll_left() const return 0.0; // 3. Let window be the value of document’s defaultView attribute. - auto* window = document.default_view(); + // FIXME: The specification expects defaultView to be a Window object, but defaultView actually returns a WindowProxy object. + auto window = document.window(); // 4. If window is null, return zero and terminate these steps. if (!window) @@ -1326,7 +1329,8 @@ void Element::set_scroll_left(double x) return; // 5. Let window be the value of document’s defaultView attribute. - auto* window = document.default_view(); + // FIXME: The specification expects defaultView to be a Window object, but defaultView actually returns a WindowProxy object. + auto window = document.window(); // 6. If window is null, terminate these steps. if (!window) @@ -1382,7 +1386,8 @@ void Element::set_scroll_top(double y) return; // 5. Let window be the value of document’s defaultView attribute. - auto* window = document.default_view(); + // FIXME: The specification expects defaultView to be a Window object, but defaultView actually returns a WindowProxy object. + auto window = document.window(); // 6. If window is null, terminate these steps. if (!window) @@ -2317,7 +2322,8 @@ void Element::scroll(double x, double y) return; // 5. Let window be the value of document’s defaultView attribute. - auto* window = document.default_view(); + // FIXME: The specification expects defaultView to be a Window object, but defaultView actually returns a WindowProxy object. + auto window = document.window(); // 6. If window is null, terminate these steps. if (!window) From 77f0c4ec9b8176756e1b90b73ac223a5cc78e257 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 21 Sep 2024 09:24:09 +0200 Subject: [PATCH 07/43] LibWeb: Always flush character insertions before exiting HTML parser This fixes an issue where document.write() with only text input would leave all the character data as unflushed text in the parser. This fixes many of the WPT tests for document.write(). (cherry picked from commit a0ed12e839f14b3ac80caca0e18a09ca256fd99d) --- .../HTML/document-write-flush-character-insertions.txt | 1 + .../HTML/document-write-flush-character-insertions.html | 6 ++++++ Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp | 2 +- 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 Tests/LibWeb/Text/expected/HTML/document-write-flush-character-insertions.txt create mode 100644 Tests/LibWeb/Text/input/HTML/document-write-flush-character-insertions.html diff --git a/Tests/LibWeb/Text/expected/HTML/document-write-flush-character-insertions.txt b/Tests/LibWeb/Text/expected/HTML/document-write-flush-character-insertions.txt new file mode 100644 index 00000000000000..465701e3460aff --- /dev/null +++ b/Tests/LibWeb/Text/expected/HTML/document-write-flush-character-insertions.txt @@ -0,0 +1 @@ +PASS \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/HTML/document-write-flush-character-insertions.html b/Tests/LibWeb/Text/input/HTML/document-write-flush-character-insertions.html new file mode 100644 index 00000000000000..2098b00c670f04 --- /dev/null +++ b/Tests/LibWeb/Text/input/HTML/document-write-flush-character-insertions.html @@ -0,0 +1,6 @@ + + diff --git a/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp b/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp index 2df5288cea7eed..f6b689237fae33 100644 --- a/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp +++ b/Userland/Libraries/LibWeb/HTML/Parser/HTMLParser.cpp @@ -177,7 +177,7 @@ void HTMLParser::run(HTMLTokenizer::StopAtInsertionPoint stop_at_insertion_point for (;;) { // FIXME: Find a better way to say that we come from Document::close() and want to process EOF. if (!m_tokenizer.is_eof_inserted() && m_tokenizer.is_insertion_point_reached()) - return; + break; auto optional_token = m_tokenizer.next_token(stop_at_insertion_point); if (!optional_token.has_value()) From 063123b043dd02d025eb801ef675ba178581e94e Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 21 Sep 2024 20:11:03 +0200 Subject: [PATCH 08/43] LibWeb: Allow setting shorthand CSS properties via CSSStyleDeclaration We now expand shorthands into their respective longhand values when assigning to a shorthand named property on a CSSStyleDeclaration. We also make sure that shorthands can be round-tripped by correctly routing named property access through the getPropertyValue() AO, and expanding it to handle shorthands as well. A lot of WPT tests for CSS parsing rely on these mechanisms and should now start working. :^) Note that multi-level recursive shorthands like `border` don't work 100% correctly yet. We're going to need a bunch more logic to properly serialize e.g `border-width` or `border` itself. (cherry picked from commit e40ad73ae79023f64e250f854d0730c21e0f54fc) --- .../css/set-style-declaration-shorthand.txt | 46 +++++++++++++ .../css/set-style-declaration-shorthand.html | 38 +++++++++++ .../LibWeb/CSS/CSSStyleDeclaration.cpp | 67 +++++++++++++++---- 3 files changed, 138 insertions(+), 13 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/set-style-declaration-shorthand.txt create mode 100644 Tests/LibWeb/Text/input/css/set-style-declaration-shorthand.html diff --git a/Tests/LibWeb/Text/expected/css/set-style-declaration-shorthand.txt b/Tests/LibWeb/Text/expected/css/set-style-declaration-shorthand.txt new file mode 100644 index 00000000000000..b6c856ce1709d6 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/set-style-declaration-shorthand.txt @@ -0,0 +1,46 @@ +Setting flex: 'none'; becomes... + flex-basis: 'auto' + flex-grow: '0' + flex-shrink: '0' + flex: '0 0 auto' + e.style.length: 3 + > [0] flex-grow + > [1] flex-shrink + > [2] flex-basis + +Setting flex: 'auto 1 2'; becomes... + flex-basis: 'auto' + flex-grow: '1' + flex-shrink: '2' + flex: '1 2 auto' + e.style.length: 3 + > [0] flex-grow + > [1] flex-shrink + > [2] flex-basis + +Setting flex: ''; becomes... + flex-basis: '' + flex-grow: '' + flex-shrink: '' + flex: '' + e.style.length: 0 + +Setting border: '1px solid red'; becomes... + border-width: '1px 1px 1px 1px' + border-style: 'solid solid solid solid' + border-color: 'rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0) rgb(255, 0, 0)' + border: '' + e.style.length: 12 + > [0] border-top-width + > [1] border-right-width + > [2] border-bottom-width + > [3] border-left-width + > [4] border-top-style + > [5] border-right-style + > [6] border-bottom-style + > [7] border-left-style + > [8] border-top-color + > [9] border-right-color + > [10] border-bottom-color + > [11] border-left-color + diff --git a/Tests/LibWeb/Text/input/css/set-style-declaration-shorthand.html b/Tests/LibWeb/Text/input/css/set-style-declaration-shorthand.html new file mode 100644 index 00000000000000..540d3c7a7fc127 --- /dev/null +++ b/Tests/LibWeb/Text/input/css/set-style-declaration-shorthand.html @@ -0,0 +1,38 @@ + + diff --git a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp index 6b314e3651b6c5..cd81b5c0d5466b 100644 --- a/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp +++ b/Userland/Libraries/LibWeb/CSS/CSSStyleDeclaration.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2023, Andreas Kling + * Copyright (c) 2018-2024, Andreas Kling * * SPDX-License-Identifier: BSD-2-Clause */ @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -130,15 +131,23 @@ WebIDL::ExceptionOr PropertyOwningCSSStyleDeclaration::set_property(Proper // 7. Let updated be false. bool updated = false; - // FIXME: 8. If property is a shorthand property, then for each longhand property longhand that property maps to, in canonical order, follow these substeps: - // FIXME: 1. Let longhand result be the result of set the CSS declaration longhand with the appropriate value(s) from component value list, - // with the important flag set if priority is not the empty string, and unset otherwise, and with the list of declarations being the declarations. - // FIXME: 2. If longhand result is true, let updated be true. - - // 9. Otherwise, let updated be the result of set the CSS declaration property with value component value list, - // with the important flag set if priority is not the empty string, and unset otherwise, - // and with the list of declarations being the declarations. - updated = set_a_css_declaration(property_id, component_value_list.release_nonnull(), !priority.is_empty() ? Important::Yes : Important::No); + // 8. If property is a shorthand property, + if (property_is_shorthand(property_id)) { + // then for each longhand property longhand that property maps to, in canonical order, follow these substeps: + StyleComputer::for_each_property_expanding_shorthands(property_id, *component_value_list, StyleComputer::AllowUnresolved::Yes, [this, &updated, priority](PropertyID longhand_property_id, CSSStyleValue const& longhand_value) { + // 1. Let longhand result be the result of set the CSS declaration longhand with the appropriate value(s) from component value list, + // with the important flag set if priority is not the empty string, and unset otherwise, and with the list of declarations being the declarations. + // 2. If longhand result is true, let updated be true. + updated |= set_a_css_declaration(longhand_property_id, longhand_value, !priority.is_empty() ? Important::Yes : Important::No); + }); + } + // 9. Otherwise, + else { + // let updated be the result of set the CSS declaration property with value component value list, + // with the important flag set if priority is not the empty string, and unset otherwise, + // and with the list of declarations being the declarations. + updated = set_a_css_declaration(property_id, *component_value_list, !priority.is_empty() ? Important::Yes : Important::No); + } // 10. If updated is true, update style attribute for the CSS declaration block. if (updated) @@ -222,11 +231,45 @@ bool PropertyOwningCSSStyleDeclaration::set_a_css_declaration(PropertyID propert return true; } +// https://drafts.csswg.org/cssom/#dom-cssstyledeclaration-getpropertyvalue String CSSStyleDeclaration::get_property_value(StringView property_name) const { auto property_id = property_id_from_string(property_name); if (!property_id.has_value()) return {}; + + // 2. If property is a shorthand property, then follow these substeps: + if (property_is_shorthand(property_id.value())) { + // 1. Let list be a new empty array. + StringBuilder list; + Optional last_important_flag; + + // 2. For each longhand property longhand that property maps to, in canonical order, follow these substeps: + for (auto longhand_property_id : longhands_for_shorthand(property_id.value())) { + // 1. If longhand is a case-sensitive match for a property name of a CSS declaration in the declarations, let declaration be that CSS declaration, or null otherwise. + auto declaration = property(longhand_property_id); + + // 2. If declaration is null, then return the empty string. + if (!declaration.has_value()) + return {}; + + // 3. Append the declaration to list. + if (!list.is_empty()) + list.append(' '); + list.append(declaration->value->to_string()); + + if (last_important_flag.has_value() && declaration->important != *last_important_flag) + return {}; + last_important_flag = declaration->important; + } + + // 3. If important flags of all declarations in list are same, then return the serialization of list. + return list.to_string_without_validation(); + + // 4. Return the empty string. + // NOTE: This is handled by the loop. + } + auto maybe_property = property(property_id.value()); if (!maybe_property.has_value()) return {}; @@ -424,9 +467,7 @@ JS::ThrowCompletionOr CSSStyleDeclaration::internal_get(JS::PropertyK auto property_id = property_id_from_name(name.to_string()); if (property_id == CSS::PropertyID::Invalid) return Base::internal_get(name, receiver, cacheable_metadata, phase); - if (auto maybe_property = property(property_id); maybe_property.has_value()) - return { JS::PrimitiveString::create(vm(), maybe_property->value->to_string()) }; - return { JS::PrimitiveString::create(vm(), String {}) }; + return JS::PrimitiveString::create(vm(), get_property_value(string_from_property_id(property_id))); } JS::ThrowCompletionOr CSSStyleDeclaration::internal_set(JS::PropertyKey const& name, JS::Value value, JS::Value receiver, JS::CacheablePropertyMetadata* cacheable_metadata) From ccb85facc3317db921ddc9f461cf8003c24637e2 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 19 Sep 2024 13:18:34 +0200 Subject: [PATCH 09/43] LibWeb: Include siblings+descendants when invalidating style When an element is invalidated, it's possible for any subsequent sibling or any of their descendants to also need invalidation. (Due to the CSS sibling combinators, `+` and `~`) For DOM node insertion/removal, we must also invalidate preceding siblings, since they could be affected by :first-child, :last-child or :nth-child() selectors. This increases the amount of invalidation we do, but it's more correct. In the future, we will implement optimizations that drastically reduce the number of elements invalidated. (cherry picked from commit df048e10f5a84d7fd90b1115c6bb90f45acd75ec) --- Userland/Libraries/LibWeb/DOM/Node.cpp | 46 ++++++++++++++++++++------ 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index c031a7c0eddf94..5e2cc9cdb34a80 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -405,18 +405,42 @@ void Node::invalidate_style(StyleInvalidationReason reason) return; } - for_each_in_inclusive_subtree([&](Node& node) { - node.m_needs_style_update = true; - if (node.has_children()) - node.m_child_needs_style_update = true; - if (auto shadow_root = node.is_element() ? static_cast(node).shadow_root() : nullptr) { - node.m_child_needs_style_update = true; - shadow_root->m_needs_style_update = true; - if (shadow_root->has_children()) - shadow_root->m_child_needs_style_update = true; + // When invalidating style for a node, we actually invalidate: + // - the node itself + // - all of its descendants + // - all of its preceding siblings and their descendants (only on DOM insert/remove) + // - all of its subsequent siblings and their descendants + // FIXME: This is a lot of invalidation and we should implement more sophisticated invalidation to do less work! + + auto invalidate_entire_subtree = [&](Node& subtree_root) { + subtree_root.for_each_in_inclusive_subtree([&](Node& node) { + node.m_needs_style_update = true; + if (node.has_children()) + node.m_child_needs_style_update = true; + if (auto shadow_root = node.is_element() ? static_cast(node).shadow_root() : nullptr) { + node.m_child_needs_style_update = true; + shadow_root->m_needs_style_update = true; + if (shadow_root->has_children()) + shadow_root->m_child_needs_style_update = true; + } + return TraversalDecision::Continue; + }); + }; + + invalidate_entire_subtree(*this); + + if (reason == StyleInvalidationReason::NodeInsertBefore || reason == StyleInvalidationReason::NodeRemove) { + for (auto* sibling = previous_sibling(); sibling; sibling = sibling->previous_sibling()) { + if (sibling->is_element()) + invalidate_entire_subtree(*sibling); } - return TraversalDecision::Continue; - }); + } + + for (auto* sibling = next_sibling(); sibling; sibling = sibling->next_sibling()) { + if (sibling->is_element()) + invalidate_entire_subtree(*sibling); + } + for (auto* ancestor = parent_or_shadow_host(); ancestor; ancestor = ancestor->parent_or_shadow_host()) ancestor->m_child_needs_style_update = true; document().schedule_style_update(); From a7bab0538bdb96e8df8fb87787b507a4317432b4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 19 Sep 2024 13:27:37 +0200 Subject: [PATCH 10/43] LibWeb: Cache whether there are any :has() selectors present As useful as they may be to web developers, :has() selectors complicate the style invalidation process quite a lot. Let's have StyleComputer keep track of whether they are present at all in the current set of active style sheets. This will allow us to implement fast-path optimizations when there are no :has() selectors. (cherry picked from commit 8beb7c77002a3359ad2fe73969fc6bb2dbc75413) --- Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 4 ++++ Userland/Libraries/LibWeb/CSS/StyleComputer.h | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index ef04f045f6a833..9c272b79f59a88 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -2372,6 +2372,8 @@ NonnullOwnPtr StyleComputer::make_rule_cache_for_casca Optional pseudo_element; for (auto const& simple_selector : selector.compound_selectors().last().simple_selectors) { + if (!rule_cache->has_has_selectors && simple_selector.type == CSS::Selector::SimpleSelector::Type::PseudoClass && simple_selector.pseudo_class().type == CSS::PseudoClass::Has) + rule_cache->has_has_selectors = true; if (!matching_rule.contains_pseudo_element) { if (simple_selector.type == CSS::Selector::SimpleSelector::Type::PseudoElement) { matching_rule.contains_pseudo_element = true; @@ -2619,6 +2621,8 @@ void StyleComputer::build_rule_cache() m_author_rule_cache = make_rule_cache_for_cascade_origin(CascadeOrigin::Author); m_user_rule_cache = make_rule_cache_for_cascade_origin(CascadeOrigin::User); m_user_agent_rule_cache = make_rule_cache_for_cascade_origin(CascadeOrigin::UserAgent); + + m_has_has_selectors = m_author_rule_cache->has_has_selectors || m_user_rule_cache->has_has_selectors || m_user_agent_rule_cache->has_has_selectors; } void StyleComputer::invalidate_rule_cache() diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index d41243df35a199..243936b6e23521 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -154,6 +154,8 @@ class StyleComputer { }; void collect_animation_into(DOM::Element&, Optional, JS::NonnullGCPtr animation, StyleProperties& style_properties, AnimationRefresh = AnimationRefresh::No) const; + [[nodiscard]] bool has_has_selectors() const { return m_has_has_selectors; } + private: enum class ComputeStyleMode { Normal, @@ -219,12 +221,15 @@ class StyleComputer { Vector other_rules; HashMap> rules_by_animation_keyframes; + + bool has_has_selectors { false }; }; NonnullOwnPtr make_rule_cache_for_cascade_origin(CascadeOrigin); RuleCache const& rule_cache_for_cascade_origin(CascadeOrigin) const; + bool m_has_has_selectors { false }; OwnPtr m_author_rule_cache; OwnPtr m_user_rule_cache; OwnPtr m_user_agent_rule_cache; From ffe28ebf3a1c5d9fe222dc952ca78d409d6ab8f4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 22 Sep 2024 13:25:21 +0200 Subject: [PATCH 11/43] LibWeb: Don't try to invalidate style for character data nodes Character data nodes like text and HTML comments don't have style, so let's just exit invalidation immediately for those. (cherry picked from commit f351f75a34619ad969f660312a2e5486f742f685) --- Userland/Libraries/LibWeb/DOM/Node.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 5e2cc9cdb34a80..4926b3612952cb 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -389,6 +389,9 @@ JS::GCPtr Node::navigable() const void Node::invalidate_style(StyleInvalidationReason reason) { + if (is_character_data()) + return; + if (!needs_style_update() && !document().needs_full_style_update()) { dbgln_if(STYLE_INVALIDATION_DEBUG, "Invalidate style ({}): {}", to_string(reason), debug_description()); } From 1cf02e809f7a83c7efefe68a4cf4cdf5a4ce6415 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 22 Sep 2024 13:27:02 +0200 Subject: [PATCH 12/43] LibWeb: Distinguish parent/child on style invalidation for DOM insertion (cherry picked from commit 7d644ecd50c57454dfb73b92e59d7112d54a1b1d) --- Userland/Libraries/LibWeb/DOM/Node.cpp | 2 +- Userland/Libraries/LibWeb/DOM/Node.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index 4926b3612952cb..bce087b7676502 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -682,7 +682,7 @@ void Node::insert_before(JS::NonnullGCPtr node, JS::GCPtr child, boo if (is_connected()) { // FIXME: This will need to become smarter when we implement the :has() selector. - invalidate_style(StyleInvalidationReason::NodeInsertBefore); + invalidate_style(StyleInvalidationReason::ParentOfInsertedNode); document().invalidate_layout_tree(); } diff --git a/Userland/Libraries/LibWeb/DOM/Node.h b/Userland/Libraries/LibWeb/DOM/Node.h index da60057e6be0cb..dabbb2c0a00ee3 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.h +++ b/Userland/Libraries/LibWeb/DOM/Node.h @@ -72,6 +72,7 @@ enum class FragmentSerializationMode { X(NodeRemove) \ X(NodeSetTextContent) \ X(Other) \ + X(ParentOfInsertedNode) \ X(SetSelectorText) \ X(SettingsChange) \ X(StyleSheetDeleteRule) \ From d40f857b04988e1e6a51314b6f40dda2944e50f7 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 22 Sep 2024 13:27:46 +0200 Subject: [PATCH 13/43] LibWeb: Invalidate less style on textContent change and node removal ...unless there are :has() selectors present. Then we have to invalidate everything for now. (cherry picked from commit b8ce34068f5bf8ab8cf049d56b864bff52ef3dff) --- Userland/Libraries/LibWeb/DOM/Node.cpp | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Node.cpp b/Userland/Libraries/LibWeb/DOM/Node.cpp index bce087b7676502..00ff00de9a616c 100644 --- a/Userland/Libraries/LibWeb/DOM/Node.cpp +++ b/Userland/Libraries/LibWeb/DOM/Node.cpp @@ -207,7 +207,13 @@ void Node::set_text_content(Optional const& maybe_content) // Otherwise, do nothing. if (is_connected()) { - document().invalidate_style(StyleInvalidationReason::NodeSetTextContent); + // FIXME: If there are any :has() selectors, we currently invalidate style for the whole document. + // We need to find a way to invalidate less! + if (document().style_computer().has_has_selectors()) { + document().invalidate_style(StyleInvalidationReason::NodeSetTextContent); + } else { + invalidate_style(StyleInvalidationReason::NodeSetTextContent); + } document().invalidate_layout_tree(); } @@ -882,7 +888,14 @@ void Node::remove(bool suppress_observers) if (was_connected) { // Since the tree structure has changed, we need to invalidate both style and layout. // In the future, we should find a way to only invalidate the parts that actually need it. - document().invalidate_style(StyleInvalidationReason::NodeRemove); + + // FIXME: If there are any :has() selectors, we currently invalidate style for the whole document. + // We need to find a way to invalidate less! + if (document().style_computer().has_has_selectors()) { + document().invalidate_style(StyleInvalidationReason::NodeRemove); + } else { + invalidate_style(StyleInvalidationReason::NodeRemove); + } // NOTE: If we didn't have a layout node before, rebuilding the layout tree isn't gonna give us one // after we've been removed from the DOM. From cbe6f445b6de6488a25e3cddc1cf26188aaafb63 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 22 Sep 2024 13:29:49 +0200 Subject: [PATCH 14/43] LibWeb: Abort ongoing fetch before starting a new link element fetch If we decide to fetch another linked resource, we don't care about the earlier fetch and can safely abort it. This fixes an issue on GitHub where we'd load the same style sheet multiple times and invalidate style for the entire document every time it finished fetching. By aborting the ongoing fetch, no excess invalidation happens. (cherry picked from commit 57e26ed6b9b2c0cf1e4f5ac3e85ca0c7f3647caa) --- Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp | 6 +++++- Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h | 2 ++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp index dba3d3100f22e2..754028b974347c 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp +++ b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -363,7 +364,9 @@ void HTMLLinkElement::default_fetch_and_process_linked_resource() process_linked_resource(success, response, body_bytes); }; - Fetch::Fetching::fetch(realm(), *request, Fetch::Infrastructure::FetchAlgorithms::create(vm(), move(fetch_algorithms_input))).release_value_but_fixme_should_propagate_errors(); + if (m_fetch_controller) + m_fetch_controller->abort(realm(), {}); + m_fetch_controller = MUST(Fetch::Fetching::fetch(realm(), *request, Fetch::Infrastructure::FetchAlgorithms::create(vm(), move(fetch_algorithms_input)))); } // https://html.spec.whatwg.org/multipage/links.html#link-type-stylesheet:process-the-linked-resource @@ -605,6 +608,7 @@ WebIDL::ExceptionOr HTMLLinkElement::load_fallback_favicon_if_needed(JS::N void HTMLLinkElement::visit_edges(Cell::Visitor& visitor) { Base::visit_edges(visitor); + visitor.visit(m_fetch_controller); visitor.visit(m_loaded_style_sheet); visitor.visit(m_rel_list); } diff --git a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h index 4f572d1226517d..caa6e3fdd0cf50 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLLinkElement.h @@ -137,6 +137,8 @@ class HTMLLinkElement final }; }; + JS::GCPtr m_fetch_controller; + JS::GCPtr m_loaded_style_sheet; Optional m_document_load_event_delayer; From c676eb6738f58946c5179cf39c9e4bb2c0796dee Mon Sep 17 00:00:00 2001 From: rmg-x Date: Sun, 22 Sep 2024 13:14:50 -0500 Subject: [PATCH 15/43] UI/Qt: Move "Open in New Tab" to the top of the link context menu (cherry picked from commit 7a2d837c8ab137d8d31b3789dfe7ef377b3b225e) --- Ladybird/Qt/Tab.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Ladybird/Qt/Tab.cpp b/Ladybird/Qt/Tab.cpp index f66fcfb33e9b63..be596e20bd875e 100644 --- a/Ladybird/Qt/Tab.cpp +++ b/Ladybird/Qt/Tab.cpp @@ -588,8 +588,8 @@ Tab::Tab(BrowserWindow* window, WebContentOptions const& web_content_options, St }); m_link_context_menu = new QMenu("Link context menu", this); - m_link_context_menu->addAction(open_link_action); m_link_context_menu->addAction(open_link_in_new_tab_action); + m_link_context_menu->addAction(open_link_action); m_link_context_menu->addSeparator(); m_link_context_menu->addAction(m_link_context_menu_copy_url_action); m_link_context_menu->addSeparator(); From aab2670b5f2402efc8137e336fbbcc42dd1aa123 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Fri, 20 Sep 2024 15:49:33 +0100 Subject: [PATCH 16/43] LibWeb: Update `close_top_level_traversable()` to match the spec (cherry picked from commit 583eef265fe9ba4f91a08e5ef1b7c7339f2371d7) --- .../LibWeb/HTML/TraversableNavigable.cpp | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp index 0f2477370ff669..4804ca6ec1be1b 100644 --- a/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/TraversableNavigable.cpp @@ -1016,18 +1016,25 @@ void TraversableNavigable::close_top_level_traversable() { VERIFY(is_top_level_traversable()); - // 1. Let toUnload be traversable's active document's inclusive descendant navigables. + // 1. If traversable's is closing is true, then return. + if (is_closing()) + return; + + // 2. Let toUnload be traversable's active document's inclusive descendant navigables. auto to_unload = active_document()->inclusive_descendant_navigables(); - // FIXME: 2. If the result of checking if unloading is user-canceled for toUnload is true, then return. + // FIXME: 3. If the result of checking if unloading is canceled for toUnload is true, then return. - // 3. Unload the active documents of each of toUnload. - for (auto navigable : to_unload) { - navigable->active_document()->unload(); - } + // 4. Append the following session history traversal steps to traversable: + append_session_history_traversal_steps(JS::create_heap_function(heap(), [this] { + // 1. Let afterAllUnloads be an algorithm step which destroys traversable. + auto after_all_unloads = JS::create_heap_function(heap(), [this] { + destroy_top_level_traversable(); + }); - // 4. Destroy traversable. - destroy_top_level_traversable(); + // 2. Unload a document and its descendants given traversable's active document, null, and afterAllUnloads. + active_document()->unload_a_document_and_its_descendants({}, after_all_unloads); + })); } // https://html.spec.whatwg.org/multipage/document-sequences.html#destroy-a-top-level-traversable From 1ce9f6f2bf715a28ff60d358a7e42e0fc61a21e4 Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Mon, 23 Sep 2024 10:24:32 +0100 Subject: [PATCH 17/43] LibWeb: Allow WebSockets to be established within workers Previously, we would crash when attempting to establish a web socket connection from inside a worker, as we were assuming that the ESO's global object was a `Window`. (cherry picked from commit 0c0595bb31db827fc14b5d16b8645bc6aa912d60) --- Userland/Libraries/LibWeb/WebSockets/WebSocket.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Userland/Libraries/LibWeb/WebSockets/WebSocket.cpp b/Userland/Libraries/LibWeb/WebSockets/WebSocket.cpp index 7f40aa072f21d6..55c67ae8f6b562 100644 --- a/Userland/Libraries/LibWeb/WebSockets/WebSocket.cpp +++ b/Userland/Libraries/LibWeb/WebSockets/WebSocket.cpp @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include #include @@ -123,8 +123,9 @@ ErrorOr WebSocket::establish_web_socket_connection(URL::URL& url_record, V { // FIXME: Integrate properly with FETCH as per https://fetch.spec.whatwg.org/#websocket-opening-handshake - auto& window = verify_cast(client.global_object()); - auto origin_string = window.associated_document().origin().serialize(); + auto* window_or_worker = dynamic_cast(&client.global_object()); + VERIFY(window_or_worker); + auto origin_string = MUST(window_or_worker->origin()).to_byte_string(); Vector protcol_byte_strings; for (auto const& protocol : protocols) From 3621b290b96cb4dbfd0ad0a1ee83d23faf532883 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Mon, 23 Sep 2024 11:13:14 -0400 Subject: [PATCH 18/43] LibWeb: Clean up HTMLInputElement-related includes This mainly uses forward declarations as appropriate for input element related files. This reduces the number of targets being built when we change HTMLInputElement.h from 430 to 44. (cherry picked from commit 57e4fb0caebb0074a23bef70d7558ce5e8a5f357) --- Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp | 2 ++ Userland/Libraries/LibWeb/HTML/HTMLFormElement.h | 1 - Userland/Libraries/LibWeb/Layout/CheckBox.cpp | 4 +--- Userland/Libraries/LibWeb/Layout/CheckBox.h | 2 +- Userland/Libraries/LibWeb/Layout/RadioButton.cpp | 2 ++ Userland/Libraries/LibWeb/Layout/RadioButton.h | 2 +- Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp | 1 + Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.h | 2 +- Userland/Libraries/LibWeb/Painting/RadioButtonPaintable.h | 2 +- Userland/Libraries/LibWeb/XHR/FormDataIterator.cpp | 1 + 10 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp index ad49e70fbc55a1..ed3b6f1289af7f 100644 --- a/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp +++ b/Userland/Libraries/LibWeb/HTML/FormControlInfrastructure.cpp @@ -6,10 +6,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include diff --git a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.h b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.h index cc1ef50d3f8420..58bbb65b62f8fe 100644 --- a/Userland/Libraries/LibWeb/HTML/HTMLFormElement.h +++ b/Userland/Libraries/LibWeb/HTML/HTMLFormElement.h @@ -11,7 +11,6 @@ #include #include #include -#include #include namespace Web::HTML { diff --git a/Userland/Libraries/LibWeb/Layout/CheckBox.cpp b/Userland/Libraries/LibWeb/Layout/CheckBox.cpp index f7c51b36396ee0..17c53b9ec9bc8f 100644 --- a/Userland/Libraries/LibWeb/Layout/CheckBox.cpp +++ b/Userland/Libraries/LibWeb/Layout/CheckBox.cpp @@ -4,11 +4,9 @@ * SPDX-License-Identifier: BSD-2-Clause */ -#include -#include +#include #include #include -#include #include namespace Web::Layout { diff --git a/Userland/Libraries/LibWeb/Layout/CheckBox.h b/Userland/Libraries/LibWeb/Layout/CheckBox.h index 9bc631e476e516..365ac019f3a110 100644 --- a/Userland/Libraries/LibWeb/Layout/CheckBox.h +++ b/Userland/Libraries/LibWeb/Layout/CheckBox.h @@ -6,7 +6,7 @@ #pragma once -#include +#include #include namespace Web::Layout { diff --git a/Userland/Libraries/LibWeb/Layout/RadioButton.cpp b/Userland/Libraries/LibWeb/Layout/RadioButton.cpp index ecf86bdd1d2532..5526e8aec6e73f 100644 --- a/Userland/Libraries/LibWeb/Layout/RadioButton.cpp +++ b/Userland/Libraries/LibWeb/Layout/RadioButton.cpp @@ -6,6 +6,8 @@ */ #include +#include +#include #include namespace Web::Layout { diff --git a/Userland/Libraries/LibWeb/Layout/RadioButton.h b/Userland/Libraries/LibWeb/Layout/RadioButton.h index 9fc315169cb141..45d456ef57874b 100644 --- a/Userland/Libraries/LibWeb/Layout/RadioButton.h +++ b/Userland/Libraries/LibWeb/Layout/RadioButton.h @@ -6,7 +6,7 @@ #pragma once -#include +#include #include namespace Web::Layout { diff --git a/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp b/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp index e3b95518f3128f..2f8d268391aa69 100644 --- a/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp +++ b/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include diff --git a/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.h b/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.h index 3b0cd1b055e548..3b188bb8a80ac8 100644 --- a/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.h +++ b/Userland/Libraries/LibWeb/Painting/CheckBoxPaintable.h @@ -6,7 +6,7 @@ #pragma once -#include +#include #include namespace Web::Painting { diff --git a/Userland/Libraries/LibWeb/Painting/RadioButtonPaintable.h b/Userland/Libraries/LibWeb/Painting/RadioButtonPaintable.h index 8a89579689f095..db63adc3181891 100644 --- a/Userland/Libraries/LibWeb/Painting/RadioButtonPaintable.h +++ b/Userland/Libraries/LibWeb/Painting/RadioButtonPaintable.h @@ -6,7 +6,7 @@ #pragma once -#include +#include #include namespace Web::Painting { diff --git a/Userland/Libraries/LibWeb/XHR/FormDataIterator.cpp b/Userland/Libraries/LibWeb/XHR/FormDataIterator.cpp index 606b617c26ff22..eb85b3cd4e19bf 100644 --- a/Userland/Libraries/LibWeb/XHR/FormDataIterator.cpp +++ b/Userland/Libraries/LibWeb/XHR/FormDataIterator.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include namespace Web::Bindings { From 60d6668f7cef39a473c12957bc61622375a81ea2 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 6 Aug 2024 10:30:43 +0200 Subject: [PATCH 19/43] LibWeb: Sync with spec in "destroy a document and its descendants" The only real change here is that we make the document unsalvageable. Everything else is fixing up spec comments. (cherry picked from commit faf097bb4168208a7c0250280ff07e638be8058a) --- Userland/Libraries/LibWeb/DOM/Document.cpp | 27 ++++++++++++++++++---- Userland/Libraries/LibWeb/DOM/Document.h | 2 ++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 9d7be26122dfa9..97d01bce830eb3 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -3267,16 +3267,35 @@ void Document::destroy() // FIXME: 9. For each workletGlobalScope in document's worklet global scopes, terminate workletGlobalScope. } +// https://html.spec.whatwg.org/multipage/browsing-the-web.html#make-document-unsalvageable +void Document::make_unsalvageable([[maybe_unused]] String reason) +{ + // FIXME: 1. Let details be a new not restored reason details whose reason is reason. + // FIXME: 2. Append details to document's bfcache blocking details. + + // 3. Set document's salvageable state to false. + set_salvageable(false); +} + // https://html.spec.whatwg.org/multipage/document-lifecycle.html#destroy-a-document-and-its-descendants void Document::destroy_a_document_and_its_descendants(JS::GCPtr> after_all_destruction) { - // 1. Let childNavigables be document's child navigables. + // 1. If document is not fully active, then: + if (!is_fully_active()) { + // 1. Make document unsalvageable given document and "masked". + make_unsalvageable("masked"_string); + + // FIXME: 2. If document's node navigable is a top-level traversable, + // build not restored reasons for a top-level traversable and its descendants given document's node navigable. + } + + // 2. Let childNavigables be document's child navigables. auto child_navigables = document_tree_child_navigables(); // 3. Let numberDestroyed be 0. IGNORE_USE_IN_ESCAPING_LAMBDA size_t number_destroyed = 0; - // 3. For each childNavigable of childNavigable's, queue a global task on the navigation and traversal task source + // 4. For each childNavigable of childNavigable's, queue a global task on the navigation and traversal task source // given childNavigable's active window to perform the following steps: for (auto& child_navigable : child_navigables) { HTML::queue_global_task(HTML::Task::Source::NavigationAndTraversal, *child_navigable->active_window(), JS::create_heap_function(heap(), [&heap = heap(), &number_destroyed, child_navigable = child_navigable.ptr()] { @@ -3288,12 +3307,12 @@ void Document::destroy_a_document_and_its_descendants(JS::GCPtr m_page; OwnPtr m_style_computer; JS::GCPtr m_style_sheets; From d22ec946ab96b0df897e490d36ad780bf1602e72 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Sun, 8 Sep 2024 11:31:00 -0400 Subject: [PATCH 20/43] LibWeb: Make `make_unsalvageable` a public field (cherry picked from commit 8b4dde0b0911f59512e1c40d19f33f49bdeeb10c) --- Userland/Libraries/LibWeb/DOM/Document.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.h b/Userland/Libraries/LibWeb/DOM/Document.h index f58be596234674..e9eee6cc51e593 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.h +++ b/Userland/Libraries/LibWeb/DOM/Document.h @@ -575,6 +575,8 @@ class Document void set_salvageable(bool value) { m_salvageable = value; } + void make_unsalvageable(String reason); + HTML::ListOfAvailableImages& list_of_available_images(); HTML::ListOfAvailableImages const& list_of_available_images() const; @@ -729,8 +731,6 @@ class Document void reset_cursor_blink_cycle(); - void make_unsalvageable(String reason); - JS::NonnullGCPtr m_page; OwnPtr m_style_computer; JS::GCPtr m_style_sheets; From 9487535035a824ce0dc0e0acaae8fd0f56d2b5b4 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Sun, 8 Sep 2024 00:28:22 -0400 Subject: [PATCH 21/43] LibWeb: Fix "attempt to update a history entry's document" This updates our implementation of [attempt-to-populate-the-history-entry's-document](https://html.spec.whatwg.org/multipage/browsing-the-web.html#attempt-to-populate-the-history-entry's-document) after fixes were made to the logic inside the queued task in https://github.com/whatwg/html/commit/cdd014ae84faf3165f8ce8441d132bb0e76513fb#diff-41cf6794ba4200b839c53531555f0f3998df4cbb01a4d5cb0b94e3ca5e23947dL99534. (cherry picked from commit ed04124cbffbfe8b850e8e3a1bc0788513ce6a4d) --- Userland/Libraries/LibWeb/HTML/Navigable.cpp | 118 ++++++++----------- 1 file changed, 51 insertions(+), 67 deletions(-) diff --git a/Userland/Libraries/LibWeb/HTML/Navigable.cpp b/Userland/Libraries/LibWeb/HTML/Navigable.cpp index b11ffa6dada5da..cd3c1ad26f55c5 100644 --- a/Userland/Libraries/LibWeb/HTML/Navigable.cpp +++ b/Userland/Libraries/LibWeb/HTML/Navigable.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -1064,9 +1065,10 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( if (document_resource.has()) { navigation_params = TRY(create_navigation_params_from_a_srcdoc_resource(entry, this, target_snapshot_params, navigation_id)); } - // 2. Otherwise, if both of the following are true: + // 2. Otherwise, if all of the following are true: // - entry's URL's scheme is a fetch scheme; and // - documentResource is null, or allowPOST is true and documentResource's request body is not failure (FIXME: check if request body is not failure) + // then set navigationParams to the result of creating navigation params by fetching given entry, navigable, sourceSnapshotParams, targetSnapshotParams, cspNavigationType, navigationId, and navTimingType. else if (Fetch::Infrastructure::is_fetch_scheme(entry->url().scheme()) && (document_resource.has() || allow_POST)) { navigation_params = TRY(create_navigation_params_by_fetching(entry, this, source_snapshot_params, target_snapshot_params, csp_navigation_type, navigation_id)); } @@ -1100,48 +1102,35 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( if (has_been_destroyed()) return; - // 1. If navigable's ongoing navigation no longer equals navigationId, then run completionSteps and return. + // 1. If navigable's ongoing navigation no longer equals navigationId, then run completionSteps and abort these steps. if (navigation_id.has_value() && (!ongoing_navigation().has() || ongoing_navigation().get() != *navigation_id)) { if (completion_steps) completion_steps->function()(); return; } - // 2. Let failure be false. - auto failure = false; + // 2. Let saveExtraDocumentState be true. + auto saveExtraDocumentState = true; - // 3. If navigationParams is a non-fetch scheme navigation params, then set entry's document state's document to the result of - // running attempt to create a non-fetch scheme document navigationParams + // 3. If navigationParams is a non-fetch scheme navigation params, then: if (navigation_params.has>()) { - // FIXME: https://github.com/whatwg/html/issues/9767 - // We probably are expected to skip to steps 13 and 14 and return after doing this + // 1. Set entry's document state's document to the result of running attempt to create a non-fetch scheme document given navigationParams. entry->document_state()->set_document(attempt_to_create_a_non_fetch_scheme_document(navigation_params.get>())); if (entry->document()) { entry->document_state()->set_ever_populated(true); } - if (completion_steps) - completion_steps->function()(); - return; - } - // 4. Otherwise, if navigationParams is null, then set failure to true. - if (navigation_params.has() || navigation_params.has()) { - failure = true; + // 2. Set saveExtraDocumentState to false. + saveExtraDocumentState = false; } - // FIXME: 5. Otherwise, if the result of should navigation response to navigation request of type in target be blocked by Content Security Policy? given navigationParams's request, - // navigationParams's response, navigationParams's policy container's CSP list, cspNavigationType, and navigable is "Blocked", then set failure to true. - - // FIXME: 6. Otherwise, if navigationParams's reserved environment is non-null and the result of checking a navigation response's adherence to its embedder policy given - // navigationParams's response, navigable, and navigationParams's policy container's embedder policy is false, then set failure to true. - - // FIXME: 7. Otherwise, if the result of checking a navigation response's adherence to `X-Frame-Options` given navigationParams's response, navigable, - // navigationParams's policy container's CSP list, and navigationParams's origin is false, then set failure to true. - - // 8. If failure is true, then: - if (failure) { - // 1. Set entry's document state's document to the result of creating a document for inline content that doesn't have a DOM, given navigable, null, and navTimingType. - // The inline content should indicate to the user the sort of error that occurred. + // 4. Otherwise, if any of the following are true: + // - navigationParams is null; + // - FIXME: the result of should navigation response to navigation request of type in target be blocked by Content Security Policy? given navigationParams's request, navigationParams's response, navigationParams's policy container's CSP list, cspNavigationType, and navigable is "Blocked"; + // - FIXME: navigationParams's reserved environment is non-null and the result of checking a navigation response's adherence to its embedder policy given navigationParams's response, navigable, and navigationParams's policy container's embedder policy is false; or + // - FIXME: the result of checking a navigation response's adherence to `X-Frame-Options` given navigationParams's response, navigable, navigationParams's policy container's CSP list, and navigationParams's origin is false, + if (navigation_params.has() || navigation_params.has()) { + // 1. Set entry's document state's document to the result of creating a document for inline content that doesn't have a DOM, given navigable, null, and navTimingType. The inline content should indicate to the user the sort of error that occurred. auto error_message = navigation_params.has() ? navigation_params.get() : "Unknown error"sv; auto error_html = load_error_page(entry->url(), error_message).release_value_but_fixme_should_propagate_errors(); @@ -1151,57 +1140,52 @@ WebIDL::ExceptionOr Navigable::populate_session_history_entry_document( parser->run(); })); - // 2. Set entry's document state's document's salvageable to false. - entry->document()->set_salvageable(false); + // 2. Make document unsalvageable given entry's document state's document and "navigation-failure". + entry->document()->make_unsalvageable("navigation-failure"_string); - // FIXME: 3. If navigationParams is not null, then: - if (!navigation_params.has()) { - // 1. FIXME: Run the environment discarding steps for navigationParams's reserved environment. - // 2. Invoke WebDriver BiDi navigation failed with currentBrowsingContext and a new WebDriver BiDi navigation status - // whose id is navigationId, status is "canceled", and url is navigationParams's response's URL. - } - } - // FIXME: 9. Otherwise, if navigationParams's response's status is 204 or 205, then: - else if (navigation_params.get>()->response->status() == 204 || navigation_params.get>()->response->status() == 205) { - // 1. Run completionSteps. - if (completion_steps) - completion_steps->function()(); + // 3. Set saveExtraDocumentState to false. + saveExtraDocumentState = false; - // 2. Return. - return; + // 4. If navigationParams is not null, then: + if (navigation_params.has()) { + // FIXME: 1. Run the environment discarding steps for navigationParams's reserved environment. + // FIXME: 2. Invoke WebDriver BiDi navigation failed with currentBrowsingContext and a new WebDriver BiDi navigation status whose id is navigationId, status is "canceled", and url is navigationParams's response's URL. + } } - // FIXME: 10. Otherwise, if navigationParams's response has a `Content-Disposition` + // FIXME: 5. Otherwise, if navigationParams's response has a `Content-Disposition` // header specifying the attachment disposition type, then: - // 11. Otherwise: - else { - // 1. Let document be the result of loading a document given navigationParams, sourceSnapshotParams, - // and entry's document state's initiator origin. + // 6. Otherwise, if navigationParams's response's status is not 204 and is not 205, then set entry's document state's document to the result of + // loading a document given navigationParams, sourceSnapshotParams, and entry's document state's initiator origin. + else if (navigation_params.get>()->response->status() != 204 && navigation_params.get>()->response->status() != 205) { auto document = load_document(navigation_params.get>()); + entry->document_state()->set_document(document); + } - // 2. If document is null, then run completionSteps and return. - if (!document) { - if (completion_steps) - completion_steps->function()(); - return; - } + // 7. If entry's document state's document is not null, then: + if (entry->document()) { + // 1. Set entry's document state's ever populated to true. + entry->document_state()->set_ever_populated(true); - // 3. Set entry's document state's document to document. - entry->document_state()->set_document(document.ptr()); + // 2. If saveExtraDocumentState is true: + if (saveExtraDocumentState) { + // 1. Let document be entry's document state's document. + auto document = entry->document(); - // 4. Set entry's document state's origin to document's origin. - entry->document_state()->set_origin(document->origin()); - } + // 2. Set entry's document state's origin to document's origin. + entry->document_state()->set_origin(document->origin()); - // FIXME: 12. If entry's document state's request referrer is "client", then set it to request's referrer. - // https://github.com/whatwg/html/issues/9767 - // What is "request" here? + // FIXME: 3. If document's URL requires storing the policy container in history, then: + } - // 13. If entry's document state's document is not null, then set entry's document state's ever populated to true. - if (entry->document()) { - entry->document_state()->set_ever_populated(true); + // 3. If entry's document state's request referrer is "client", and navigationParams is a navigation params (i.e., neither null nor a non-fetch scheme navigation params), then: + if (entry->document_state()->request_referrer() == Fetch::Infrastructure::Request::Referrer::Client + && (!navigation_params.has() && Fetch::Infrastructure::is_fetch_scheme(entry->url().scheme()))) { + // FIXME: 1. Assert: navigationParams's request is not null. + // FIXME: 2. Set entry's document state's request referrer to navigationParams's request's referrer. + } } - // 14. Run completionSteps. + // 8. Run completionSteps. if (completion_steps) completion_steps->function()(); })); From 2dd7b53a0bdc64d0ed9ab4f008a409416afb4299 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Tue, 24 Sep 2024 09:48:05 +0100 Subject: [PATCH 22/43] LibJS: Update wording from Console spec https://github.com/whatwg/console/pull/240 is an editorial change to use the term "implementation-defined" more consistently. This seems to be the only instance in the spec text which we quote verbatim. (cherry picked from commit 51f82c1d939dd28a3e719d7fa495cf9f30d0921c) --- Userland/Libraries/LibJS/Console.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Userland/Libraries/LibJS/Console.cpp b/Userland/Libraries/LibJS/Console.cpp index 2c191a2d444a8a..23b1701734b4e4 100644 --- a/Userland/Libraries/LibJS/Console.cpp +++ b/Userland/Libraries/LibJS/Console.cpp @@ -346,7 +346,7 @@ ThrowCompletionOr Console::trace() auto& vm = realm().vm(); - // 1. Let trace be some implementation-specific, potentially-interactive representation of the callstack from where this function was called. + // 1. Let trace be some implementation-defined, potentially-interactive representation of the callstack from where this function was called. Console::Trace trace; auto& execution_context_stack = vm.execution_context_stack(); // NOTE: -2 to skip the console.trace() execution context From b9baf79a6ce01dff17d33fe9cc483ddddc00099b Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Tue, 24 Sep 2024 18:05:29 -0400 Subject: [PATCH 23/43] WebDriver: Do not break WebDriver responses into multiple socket writes WPT uses Python's http.client.HTTPConnection to send/receive WebDriver messages. For some reason, on Linux, we see an ~0.04s delay between the WPT server receiving the WebDriver response headers and its body. There are tests which make north of 1100 of these requests, which adds up to ~44s. These connections are almost always going to be over localhost and able the be sent in a single write. So let's send the response all at once. On my Linux machine, this reduces the runtime of /cookies/name/name.html from 45-60s down to 3-4s. (cherry picked from commit e5877cda61eb53cd9c1eebbfaf3c35d084b2973c) --- Userland/Libraries/LibWeb/WebDriver/Client.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/Userland/Libraries/LibWeb/WebDriver/Client.cpp b/Userland/Libraries/LibWeb/WebDriver/Client.cpp index 7887f1f0d283d4..4868370b9084a9 100644 --- a/Userland/Libraries/LibWeb/WebDriver/Client.cpp +++ b/Userland/Libraries/LibWeb/WebDriver/Client.cpp @@ -303,14 +303,9 @@ ErrorOr Client::send_success_response(JsonValue resu builder.append("Content-Type: application/json; charset=utf-8\r\n"sv); builder.appendff("Content-Length: {}\r\n", content.length()); builder.append("\r\n"sv); + builder.append(content); - auto builder_contents = TRY(builder.to_byte_buffer()); - TRY(m_socket->write_until_depleted(builder_contents)); - - while (!content.is_empty()) { - auto bytes_sent = TRY(m_socket->write_some(content.bytes())); - content = content.substring_view(bytes_sent); - } + TRY(m_socket->write_until_depleted(builder.string_view())); if (!keep_alive) die(); From 01e85086bd55be30d7418b527f58d27369e88469 Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Thu, 19 Sep 2024 19:41:57 +0100 Subject: [PATCH 24/43] LibWeb/HTML: Implement TextTrackCue idl interface (cherry picked from commit 0b2449d8d264f58b39ed2a4a69b0afb8eb762011) --- .../Text/expected/all-window-properties.txt | 1 + Userland/Libraries/LibWeb/CMakeLists.txt | 1 + Userland/Libraries/LibWeb/Forward.h | 1 + Userland/Libraries/LibWeb/HTML/EventNames.h | 2 + .../Libraries/LibWeb/HTML/TextTrackCue.cpp | 89 +++++++++++++++++++ Userland/Libraries/LibWeb/HTML/TextTrackCue.h | 64 +++++++++++++ .../Libraries/LibWeb/HTML/TextTrackCue.idl | 15 ++++ Userland/Libraries/LibWeb/idl_files.cmake | 1 + 8 files changed, 174 insertions(+) create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCue.cpp create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCue.h create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCue.idl diff --git a/Tests/LibWeb/Text/expected/all-window-properties.txt b/Tests/LibWeb/Text/expected/all-window-properties.txt index 22eac7d605d2f0..124f1b53946fcc 100644 --- a/Tests/LibWeb/Text/expected/all-window-properties.txt +++ b/Tests/LibWeb/Text/expected/all-window-properties.txt @@ -343,6 +343,7 @@ TextDecoder TextEncoder TextMetrics TextTrack +TextTrackCue TextTrackList TimeRanges ToggleEvent diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index f034d838a6ee8b..a22a6b5dbd5445 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -459,6 +459,7 @@ set(SOURCES HTML/TagNames.cpp HTML/TextMetrics.cpp HTML/TextTrack.cpp + HTML/TextTrackCue.cpp HTML/TextTrackList.cpp HTML/Timer.cpp HTML/TimeRanges.cpp diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 0bb3c810bb67f5..ac15dffd1d5b69 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -493,6 +493,7 @@ class Storage; class SubmitEvent; class TextMetrics; class TextTrack; +class TextTrackCue; class TextTrackList; class Timer; class TimeRanges; diff --git a/Userland/Libraries/LibWeb/HTML/EventNames.h b/Userland/Libraries/LibWeb/HTML/EventNames.h index dda9ee49aa5adb..47e36297294a87 100644 --- a/Userland/Libraries/LibWeb/HTML/EventNames.h +++ b/Userland/Libraries/LibWeb/HTML/EventNames.h @@ -51,7 +51,9 @@ namespace Web::HTML::EventNames { __ENUMERATE_HTML_EVENT(durationchange) \ __ENUMERATE_HTML_EVENT(emptied) \ __ENUMERATE_HTML_EVENT(ended) \ + __ENUMERATE_HTML_EVENT(enter) \ __ENUMERATE_HTML_EVENT(error) \ + __ENUMERATE_HTML_EVENT(exit) \ __ENUMERATE_HTML_EVENT(finish) \ __ENUMERATE_HTML_EVENT(focus) \ __ENUMERATE_HTML_EVENT(focusin) \ diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCue.cpp b/Userland/Libraries/LibWeb/HTML/TextTrackCue.cpp new file mode 100644 index 00000000000000..5713c5e91f6baa --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCue.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include +#include + +namespace Web::HTML { + +JS_DEFINE_ALLOCATOR(TextTrackCue); + +TextTrackCue::TextTrackCue(JS::Realm& realm, JS::GCPtr track) + : DOM::EventTarget(realm) + , m_track(track) +{ +} + +TextTrackCue::~TextTrackCue() = default; + +void TextTrackCue::initialize(JS::Realm& realm) +{ + Base::initialize(realm); + WEB_SET_PROTOTYPE_FOR_INTERFACE(TextTrackCue); +} + +void TextTrackCue::visit_edges(JS::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_track); +} + +// https://html.spec.whatwg.org/multipage/media.html#dom-texttrackcue-starttime +void TextTrackCue::set_start_time(double start_time) +{ + // On setting, the text track cue start time must be set to the new value, interpreted in seconds; + m_start_time = start_time; + + // FIXME: then, if the TextTrackCue object's text track cue is in a text track's list of cues, and that text track is in a media + // element's list of text tracks, and the media element's show poster flag is not set, then run the time marches on steps + // for that media element. +} + +// https://html.spec.whatwg.org/multipage/media.html#dom-texttrackcue-endtime +WebIDL::ExceptionOr TextTrackCue::set_end_time(double end_time) +{ + // On setting, if the new value is negative Infinity or a Not-a-Number (NaN) value, then throw a TypeError exception. + if (end_time == -AK::Infinity || isnan(end_time)) + return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, "Value is negative infinity or NaN"_string }; + + // Otherwise, the text track cue end time must be set to the new value. + m_end_time = end_time; + + // FIXME: Then, if the TextTrackCue object's text track cue is in a text track's list of cues, and that text track is in a media + // element's list of text tracks, and the media element's show poster flag is not set, then run the time marches on steps + // for that media element. + return {}; +} + +// https://html.spec.whatwg.org/multipage/media.html#handler-texttrackcue-onenter +WebIDL::CallbackType* TextTrackCue::onenter() +{ + return event_handler_attribute(HTML::EventNames::enter); +} + +// https://html.spec.whatwg.org/multipage/media.html#handler-texttrackcue-onenter +void TextTrackCue::set_onenter(WebIDL::CallbackType* event_handler) +{ + set_event_handler_attribute(HTML::EventNames::enter, event_handler); +} + +// https://html.spec.whatwg.org/multipage/media.html#handler-texttrackcue-onexit +WebIDL::CallbackType* TextTrackCue::onexit() +{ + return event_handler_attribute(HTML::EventNames::exit); +} + +// https://html.spec.whatwg.org/multipage/media.html#handler-texttrackcue-onexit +void TextTrackCue::set_onexit(WebIDL::CallbackType* event_handler) +{ + set_event_handler_attribute(HTML::EventNames::exit, event_handler); +} + +} diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCue.h b/Userland/Libraries/LibWeb/HTML/TextTrackCue.h new file mode 100644 index 00000000000000..946e03f81f2462 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCue.h @@ -0,0 +1,64 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace Web::HTML { + +// https://html.spec.whatwg.org/multipage/media.html#texttrackcue +class TextTrackCue : public DOM::EventTarget { + WEB_PLATFORM_OBJECT(TextTrackCue, DOM::EventTarget); + JS_DECLARE_ALLOCATOR(TextTrackCue); + +public: + virtual ~TextTrackCue() override; + + JS::GCPtr track() { return m_track; } + + String const& id() const { return m_identifier; } + void set_id(String const& id) { m_identifier = id; } + + double start_time() const { return m_start_time; } + void set_start_time(double start_time); + + double end_time() const { return m_end_time; } + WebIDL::ExceptionOr set_end_time(double end_time); + + bool pause_on_exit() const { return m_pause_on_exit; } + void set_pause_on_exit(bool pause_on_exit) { m_pause_on_exit = pause_on_exit; } + + WebIDL::CallbackType* onenter(); + void set_onenter(WebIDL::CallbackType*); + + WebIDL::CallbackType* onexit(); + void set_onexit(WebIDL::CallbackType*); + +protected: + TextTrackCue(JS::Realm&, JS::GCPtr); + + virtual void initialize(JS::Realm&) override; + virtual void visit_edges(Visitor&) override; + + JS::GCPtr m_track; + + // https://html.spec.whatwg.org/multipage/media.html#text-track-cue-identifier + String m_identifier; + + // https://html.spec.whatwg.org/multipage/media.html#text-track-cue-start-time + double m_start_time; + + // https://html.spec.whatwg.org/multipage/media.html#text-track-cue-end-time + double m_end_time; + + // https://html.spec.whatwg.org/multipage/media.html#text-track-cue-pause-on-exit-flag + bool m_pause_on_exit; +}; + +} diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCue.idl b/Userland/Libraries/LibWeb/HTML/TextTrackCue.idl new file mode 100644 index 00000000000000..e46443705f66f4 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCue.idl @@ -0,0 +1,15 @@ +#import + +// https://html.spec.whatwg.org/multipage/media.html#texttrackcue +[Exposed=Window] +interface TextTrackCue : EventTarget { + readonly attribute TextTrack? track; + + attribute DOMString id; + attribute double startTime; + attribute unrestricted double endTime; + attribute boolean pauseOnExit; + + attribute EventHandler onenter; + attribute EventHandler onexit; +}; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index b4ac2c8f8e5bf0..cbb98a83784b55 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -221,6 +221,7 @@ libweb_js_bindings(HTML/Storage) libweb_js_bindings(HTML/SubmitEvent) libweb_js_bindings(HTML/TextMetrics) libweb_js_bindings(HTML/TextTrack) +libweb_js_bindings(HTML/TextTrackCue) libweb_js_bindings(HTML/TextTrackList) libweb_js_bindings(HTML/TimeRanges) libweb_js_bindings(HTML/ToggleEvent) From c395b31b5d6502f9e328bae5feadbe2b75f88265 Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Fri, 20 Sep 2024 23:06:51 +0100 Subject: [PATCH 25/43] LibWeb/HTML: Implement TextTrackCueList idl interface (cherry picked from commit cfec88feb312f3fbffe2270009a9ba07e2fd9b3f) --- .../Text/expected/all-window-properties.txt | 1 + Userland/Libraries/LibWeb/CMakeLists.txt | 1 + Userland/Libraries/LibWeb/Forward.h | 1 + .../LibWeb/HTML/TextTrackCueList.cpp | 79 +++++++++++++++++++ .../Libraries/LibWeb/HTML/TextTrackCueList.h | 38 +++++++++ .../LibWeb/HTML/TextTrackCueList.idl | 9 +++ Userland/Libraries/LibWeb/idl_files.cmake | 1 + 7 files changed, 130 insertions(+) create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCueList.cpp create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCueList.h create mode 100644 Userland/Libraries/LibWeb/HTML/TextTrackCueList.idl diff --git a/Tests/LibWeb/Text/expected/all-window-properties.txt b/Tests/LibWeb/Text/expected/all-window-properties.txt index 124f1b53946fcc..88298d49cc1102 100644 --- a/Tests/LibWeb/Text/expected/all-window-properties.txt +++ b/Tests/LibWeb/Text/expected/all-window-properties.txt @@ -344,6 +344,7 @@ TextEncoder TextMetrics TextTrack TextTrackCue +TextTrackCueList TextTrackList TimeRanges ToggleEvent diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index a22a6b5dbd5445..fb33421c6174b5 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -460,6 +460,7 @@ set(SOURCES HTML/TextMetrics.cpp HTML/TextTrack.cpp HTML/TextTrackCue.cpp + HTML/TextTrackCueList.cpp HTML/TextTrackList.cpp HTML/Timer.cpp HTML/TimeRanges.cpp diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index ac15dffd1d5b69..d9dace6f66981b 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -494,6 +494,7 @@ class SubmitEvent; class TextMetrics; class TextTrack; class TextTrackCue; +class TextTrackCueList; class TextTrackList; class Timer; class TimeRanges; diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCueList.cpp b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.cpp new file mode 100644 index 00000000000000..b39c63f2b5afd3 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.cpp @@ -0,0 +1,79 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include +#include +#include + +namespace Web::HTML { + +JS_DEFINE_ALLOCATOR(TextTrackCueList); + +TextTrackCueList::TextTrackCueList(JS::Realm& realm) + : DOM::EventTarget(realm, MayInterfereWithIndexedPropertyAccess::Yes) +{ +} + +TextTrackCueList::~TextTrackCueList() = default; + +void TextTrackCueList::initialize(JS::Realm& realm) +{ + Base::initialize(realm); + WEB_SET_PROTOTYPE_FOR_INTERFACE(TextTrackCueList); +} + +void TextTrackCueList::visit_edges(JS::Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_cues); +} + +// https://html.spec.whatwg.org/multipage/media.html#dom-texttrackcuelist-item +JS::ThrowCompletionOr> TextTrackCueList::internal_get_own_property(JS::PropertyKey const& property_name) const +{ + // To determine the value of an indexed property for a given index index, the user agent must return the indexth text track cue in the list + // represented by the TextTrackCueList object. + if (property_name.is_number()) { + if (auto index = property_name.as_number(); index < m_cues.size()) { + JS::PropertyDescriptor descriptor; + descriptor.value = m_cues.at(index); + + return descriptor; + } + } + + return Base::internal_get_own_property(property_name); +} + +// https://html.spec.whatwg.org/multipage/media.html#dom-texttrackcuelist-length +size_t TextTrackCueList::length() const +{ + // The length attribute must return the number of cues in the list represented by the TextTrackCueList object. + return m_cues.size(); +} + +// https://html.spec.whatwg.org/multipage/media.html#dom-texttrackcuelist-getcuebyid +JS::GCPtr TextTrackCueList::get_cue_by_id(StringView id) const +{ + // The getCueById(id) method, when called with an argument other than the empty string, must return the first text track cue in the list + // represented by the TextTrackCueList object whose text track cue identifier is id, if any, or null otherwise. If the argument is the + // empty string, then the method must return null. + if (id.is_empty()) + return nullptr; + + auto it = m_cues.find_if([&](auto const& cue) { + return cue->id() == id; + }); + + if (it == m_cues.end()) + return nullptr; + + return *it; +} + +} diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCueList.h b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.h new file mode 100644 index 00000000000000..5ee286c59a4d16 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.h @@ -0,0 +1,38 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace Web::HTML { + +// https://html.spec.whatwg.org/multipage/media.html#texttrackcuelist +class TextTrackCueList final : public DOM::EventTarget { + WEB_PLATFORM_OBJECT(TextTrackCueList, DOM::EventTarget); + JS_DECLARE_ALLOCATOR(TextTrackCueList); + +public: + virtual ~TextTrackCueList() override; + + size_t length() const; + + JS::GCPtr get_cue_by_id(StringView id) const; + +private: + TextTrackCueList(JS::Realm&); + + virtual void initialize(JS::Realm&) override; + virtual void visit_edges(Visitor&) override; + + virtual JS::ThrowCompletionOr> internal_get_own_property(JS::PropertyKey const& property_name) const override; + + Vector> m_cues; +}; + +} diff --git a/Userland/Libraries/LibWeb/HTML/TextTrackCueList.idl b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.idl new file mode 100644 index 00000000000000..ba08b5f62002f1 --- /dev/null +++ b/Userland/Libraries/LibWeb/HTML/TextTrackCueList.idl @@ -0,0 +1,9 @@ +#import + +// https://html.spec.whatwg.org/multipage/media.html#texttrackcuelist +[Exposed=Window] +interface TextTrackCueList { + readonly attribute unsigned long length; + getter TextTrackCue (unsigned long index); + TextTrackCue? getCueById(DOMString id); +}; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index cbb98a83784b55..875b74a25e96f0 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -222,6 +222,7 @@ libweb_js_bindings(HTML/SubmitEvent) libweb_js_bindings(HTML/TextMetrics) libweb_js_bindings(HTML/TextTrack) libweb_js_bindings(HTML/TextTrackCue) +libweb_js_bindings(HTML/TextTrackCueList) libweb_js_bindings(HTML/TextTrackList) libweb_js_bindings(HTML/TimeRanges) libweb_js_bindings(HTML/ToggleEvent) From 82259fc7f33e18c8d8ef7057df11cafe905960d6 Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Thu, 19 Sep 2024 19:18:00 +0100 Subject: [PATCH 26/43] LibWeb/WebVTT: Implement VTTRegion idl interface (cherry picked from commit 1a012f279a9791c685781b7189fba6cddb973f96) --- .../BindingsGenerator/IDLGenerators.cpp | 2 + .../Text/expected/all-window-properties.txt | 1 + Userland/Libraries/LibWeb/CMakeLists.txt | 1 + Userland/Libraries/LibWeb/Forward.h | 4 + .../Libraries/LibWeb/WebVTT/VTTRegion.cpp | 120 ++++++++++++++++++ Userland/Libraries/LibWeb/WebVTT/VTTRegion.h | 74 +++++++++++ .../Libraries/LibWeb/WebVTT/VTTRegion.idl | 16 +++ Userland/Libraries/LibWeb/idl_files.cmake | 1 + 8 files changed, 219 insertions(+) create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTRegion.cpp create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTRegion.h create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTRegion.idl diff --git a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp index 1c27d7465447e0..38f4014245ded1 100644 --- a/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp +++ b/Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp @@ -94,6 +94,7 @@ static bool is_platform_object(Type const& type) "TextMetrics"sv, "TextTrack"sv, "URLSearchParams"sv, + "VTTRegion"sv, "VideoTrack"sv, "VideoTrackList"sv, "WebGLRenderingContext"sv, @@ -4212,6 +4213,7 @@ static void generate_using_namespace_definitions(SourceGenerator& generator) using namespace Web::WebAudio; using namespace Web::WebGL; using namespace Web::WebIDL; + using namespace Web::WebVTT; using namespace Web::XHR; )~~~"sv); } diff --git a/Tests/LibWeb/Text/expected/all-window-properties.txt b/Tests/LibWeb/Text/expected/all-window-properties.txt index 88298d49cc1102..c210a33eecd095 100644 --- a/Tests/LibWeb/Text/expected/all-window-properties.txt +++ b/Tests/LibWeb/Text/expected/all-window-properties.txt @@ -362,6 +362,7 @@ Uint32Array Uint8Array Uint8ClampedArray UserActivation +VTTRegion ValidityState VideoTrack VideoTrackList diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index fb33421c6174b5..769262e857afbd 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -752,6 +752,7 @@ set(SOURCES WebIDL/Promise.cpp WebIDL/Tracing.cpp WebSockets/WebSocket.cpp + WebVTT/VTTRegion.cpp XHR/EventNames.cpp XHR/FormData.cpp XHR/FormDataIterator.cpp diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index d9dace6f66981b..7edbf150f30c05 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -787,6 +787,10 @@ namespace Web::WebSockets { class WebSocket; } +namespace Web::WebVTT { +class VTTRegion; +} + namespace Web::XHR { class FormData; class FormDataIterator; diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTRegion.cpp b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.cpp new file mode 100644 index 00000000000000..5c85b8ca7c99ab --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.cpp @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include + +namespace Web::WebVTT { + +JS_DEFINE_ALLOCATOR(VTTRegion); + +// https://w3c.github.io/webvtt/#dom-vttregion-vttregion +WebIDL::ExceptionOr> VTTRegion::construct_impl(JS::Realm& realm) +{ + // 1. Create a new WebVTT region. Let region be that WebVTT region. + auto region = realm.heap().allocate(realm, realm); + + // 2. Let region’s WebVTT region identifier be the empty string. + region->m_identifier = ""_string; + + // 3. Let region’s WebVTT region width be 100. + region->m_width = 100; + + // 4. Let region’s WebVTT region lines be 3. + region->m_lines = 3; + + // 5. Let region’s text track region regionAnchorX be 0. + region->m_anchor_x = 0; + + // 6. Let region’s text track region regionAnchorY be 100. + region->m_anchor_y = 100; + + // 7. Let region’s text track region viewportAnchorX be 0. + region->m_viewport_anchor_x = 0; + + // 8. Let region’s text track region viewportAnchorY be 100. + region->m_viewport_anchor_y = 100; + + // 9. Let region’s WebVTT region scroll be the empty string. + region->m_scroll_setting = Bindings::ScrollSetting::Empty; + + // 10. Return the VTTRegion object representing region. + return region; +} + +VTTRegion::VTTRegion(JS::Realm& realm) + : PlatformObject(realm) +{ +} + +void VTTRegion::initialize(JS::Realm& realm) +{ + Base::initialize(realm); + WEB_SET_PROTOTYPE_FOR_INTERFACE(VTTRegion); +} + +// https://w3c.github.io/webvtt/#dom-vttregion-width +WebIDL::ExceptionOr VTTRegion::set_width(double width) +{ + // On setting, if the new value is negative or greater than 100, then an IndexSizeError exception must be thrown. + if (width < 0 || width > 100) + return WebIDL::IndexSizeError::create(realm(), "Value is negative or greater than 100"_fly_string); + + // Otherwise, the WebVTT region width must be set to the new value. + m_width = width; + return {}; +} + +// https://w3c.github.io/webvtt/#dom-vttregion-regionanchorx +WebIDL::ExceptionOr VTTRegion::set_region_anchor_x(double region_anchor_x) +{ + // On setting, if the new value is negative or greater than 100, then an IndexSizeError exception must be thrown. + if (region_anchor_x < 0 || region_anchor_x > 100) + return WebIDL::IndexSizeError::create(realm(), "Value is negative or greater than 100"_fly_string); + + // Otherwise, the WebVTT region anchor X distance must be set to the new value. + m_anchor_x = region_anchor_x; + return {}; +} + +// https://w3c.github.io/webvtt/#dom-vttregion-regionanchory +WebIDL::ExceptionOr VTTRegion::set_region_anchor_y(double region_anchor_y) +{ + // On setting, if the new value is negative or greater than 100, then an IndexSizeError exception must be thrown. + if (region_anchor_y < 0 || region_anchor_y > 100) + return WebIDL::IndexSizeError::create(realm(), "Value is negative or greater than 100"_fly_string); + + // Otherwise, the WebVTT region anchor Y distance must be set to the new value. + m_anchor_y = region_anchor_y; + return {}; +} + +// https://w3c.github.io/webvtt/#dom-vttregion-viewportanchorx +WebIDL::ExceptionOr VTTRegion::set_viewport_anchor_x(double viewport_anchor_x) +{ + // On setting, if the new value is negative or greater than 100, then an IndexSizeError exception must be thrown. + if (viewport_anchor_x < 0 || viewport_anchor_x > 100) + return WebIDL::IndexSizeError::create(realm(), "Value is negative or greater than 100"_fly_string); + + // Otherwise, the WebVTT region viewport anchor X distance must be set to the new value. + m_viewport_anchor_x = viewport_anchor_x; + return {}; +} + +// https://w3c.github.io/webvtt/#dom-vttregion-viewportanchory +WebIDL::ExceptionOr VTTRegion::set_viewport_anchor_y(double viewport_anchor_y) +{ + // On setting, if the new value is negative or greater than 100, then an IndexSizeError exception must be thrown. + if (viewport_anchor_y < 0 || viewport_anchor_y > 100) + return WebIDL::IndexSizeError::create(realm(), "Value is negative or greater than 100"_fly_string); + + // Otherwise, the WebVTT region viewport anchor Y distance must be set to the new value. + m_viewport_anchor_y = viewport_anchor_y; + return {}; +} + +} diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTRegion.h b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.h new file mode 100644 index 00000000000000..52f8d232af3446 --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.h @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace Web::WebVTT { + +// https://w3c.github.io/webvtt/#vttregion +class VTTRegion final : public Bindings::PlatformObject { + WEB_PLATFORM_OBJECT(VTTRegion, Bindings::PlatformObject); + JS_DECLARE_ALLOCATOR(VTTRegion); + +public: + static WebIDL::ExceptionOr> construct_impl(JS::Realm&); + virtual ~VTTRegion() override = default; + + String const& id() const { return m_identifier; } + void set_id(String const& id) { m_identifier = id; } + + double width() const { return m_width; } + WebIDL::ExceptionOr set_width(double width); + + WebIDL::UnsignedLong lines() const { return m_lines; } + void set_lines(WebIDL::UnsignedLong lines) { m_lines = lines; } + + double region_anchor_x() const { return m_anchor_x; } + WebIDL::ExceptionOr set_region_anchor_x(double region_anchor_x); + + double region_anchor_y() const { return m_anchor_y; } + WebIDL::ExceptionOr set_region_anchor_y(double region_anchor_y); + + double viewport_anchor_x() const { return m_viewport_anchor_x; } + WebIDL::ExceptionOr set_viewport_anchor_x(double viewport_anchor_x); + + double viewport_anchor_y() const { return m_viewport_anchor_y; } + WebIDL::ExceptionOr set_viewport_anchor_y(double viewport_anchor_y); + + Bindings::ScrollSetting scroll() const { return m_scroll_setting; } + void set_scroll(Bindings::ScrollSetting scroll) { m_scroll_setting = scroll; } + +private: + VTTRegion(JS::Realm&); + + virtual void initialize(JS::Realm&) override; + + // https://w3c.github.io/webvtt/#webvtt-region-identifier + String m_identifier {}; + + // https://w3c.github.io/webvtt/#webvtt-region-width + double m_width { 100 }; + + // https://w3c.github.io/webvtt/#webvtt-region-lines + WebIDL::UnsignedLong m_lines { 3 }; + + // https://w3c.github.io/webvtt/#webvtt-region-anchor + double m_anchor_x { 0 }; + double m_anchor_y { 100 }; + + // https://w3c.github.io/webvtt/#webvtt-region-viewport-anchor + double m_viewport_anchor_x { 0 }; + double m_viewport_anchor_y { 100 }; + + // https://w3c.github.io/webvtt/#webvtt-region-scroll + Bindings::ScrollSetting m_scroll_setting { Bindings::ScrollSetting::Empty }; +}; + +} diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTRegion.idl b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.idl new file mode 100644 index 00000000000000..0561f094317034 --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTRegion.idl @@ -0,0 +1,16 @@ +// https://w3c.github.io/webvtt/#enumdef-scrollsetting +enum ScrollSetting { "", "up" }; + +// https://w3c.github.io/webvtt/#vttregion +[Exposed=Window] +interface VTTRegion { + constructor(); + attribute DOMString id; + attribute double width; + attribute unsigned long lines; + attribute double regionAnchorX; + attribute double regionAnchorY; + attribute double viewportAnchorX; + attribute double viewportAnchorY; + attribute ScrollSetting scroll; +}; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index 875b74a25e96f0..384cc0538c0f2d 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -344,6 +344,7 @@ libweb_js_bindings(WebGL/WebGLContextEvent) libweb_js_bindings(WebGL/WebGLRenderingContext) libweb_js_bindings(WebIDL/DOMException) libweb_js_bindings(WebSockets/WebSocket) +libweb_js_bindings(WebVTT/VTTRegion) libweb_js_bindings(XHR/FormData ITERABLE) libweb_js_bindings(XHR/ProgressEvent) libweb_js_bindings(XHR/XMLHttpRequest) From 93591230221378b750bdd9f296cdcc988193d6de Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Thu, 19 Sep 2024 21:49:54 +0100 Subject: [PATCH 27/43] LibWeb/WebVTT: Implement VTTCue idl interface (cherry picked from commit 973f774e56b519964f2f43d965035d9076658096) --- .../Text/expected/all-window-properties.txt | 1 + Userland/Libraries/LibWeb/CMakeLists.txt | 1 + Userland/Libraries/LibWeb/Forward.h | 1 + Userland/Libraries/LibWeb/WebVTT/VTTCue.cpp | 143 ++++++++++++++++++ Userland/Libraries/LibWeb/WebVTT/VTTCue.h | 94 ++++++++++++ Userland/Libraries/LibWeb/WebVTT/VTTCue.idl | 26 ++++ Userland/Libraries/LibWeb/idl_files.cmake | 1 + 7 files changed, 267 insertions(+) create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTCue.cpp create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTCue.h create mode 100644 Userland/Libraries/LibWeb/WebVTT/VTTCue.idl diff --git a/Tests/LibWeb/Text/expected/all-window-properties.txt b/Tests/LibWeb/Text/expected/all-window-properties.txt index c210a33eecd095..1584a9e182049d 100644 --- a/Tests/LibWeb/Text/expected/all-window-properties.txt +++ b/Tests/LibWeb/Text/expected/all-window-properties.txt @@ -362,6 +362,7 @@ Uint32Array Uint8Array Uint8ClampedArray UserActivation +VTTCue VTTRegion ValidityState VideoTrack diff --git a/Userland/Libraries/LibWeb/CMakeLists.txt b/Userland/Libraries/LibWeb/CMakeLists.txt index 769262e857afbd..bdad489aef03c3 100644 --- a/Userland/Libraries/LibWeb/CMakeLists.txt +++ b/Userland/Libraries/LibWeb/CMakeLists.txt @@ -752,6 +752,7 @@ set(SOURCES WebIDL/Promise.cpp WebIDL/Tracing.cpp WebSockets/WebSocket.cpp + WebVTT/VTTCue.cpp WebVTT/VTTRegion.cpp XHR/EventNames.cpp XHR/FormData.cpp diff --git a/Userland/Libraries/LibWeb/Forward.h b/Userland/Libraries/LibWeb/Forward.h index 7edbf150f30c05..ccc8bddc3b7300 100644 --- a/Userland/Libraries/LibWeb/Forward.h +++ b/Userland/Libraries/LibWeb/Forward.h @@ -788,6 +788,7 @@ class WebSocket; } namespace Web::WebVTT { +class VTTCue; class VTTRegion; } diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTCue.cpp b/Userland/Libraries/LibWeb/WebVTT/VTTCue.cpp new file mode 100644 index 00000000000000..ea371e54eb65a3 --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTCue.cpp @@ -0,0 +1,143 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include +#include + +namespace Web::WebVTT { + +JS_DEFINE_ALLOCATOR(VTTCue); + +// https://w3c.github.io/webvtt/#dom-vttcue-vttcue +WebIDL::ExceptionOr> VTTCue::construct_impl(JS::Realm& realm, double start_time, double end_time, String const& text) +{ + // 1. Create a new WebVTT cue. Let cue be that WebVTT cue. + auto cue = realm.heap().allocate(realm, realm, nullptr); + + // 2. Let cue’s text track cue start time be the value of the startTime argument. + cue->m_start_time = start_time; + + // 3. If the value of the endTime argument is negative Infinity or a Not-a-Number (NaN) value, then throw a TypeError exception. + // Otherwise, let cue’s text track cue end time be the value of the endTime argument. + if (end_time == -AK::Infinity || isnan(end_time)) + return WebIDL::SimpleException { WebIDL::SimpleExceptionType::TypeError, "End time is negative infinity or NaN"_string }; + cue->m_end_time = end_time; + + // 4. Let cue’s cue text be the value of the text argument, and let the rules for extracting the chapter title be the WebVTT rules + // for extracting the chapter title. + cue->m_text = text; + // FIXME: let the rules for extracting the chapter title be the WebVTT rules for extracting the chapter title. + + // 5. Let cue’s text track cue identifier be the empty string. + cue->m_identifier = ""_string; + + // 6. Let cue’s text track cue pause-on-exit flag be false. + cue->m_pause_on_exit = false; + + // 7. Let cue’s WebVTT cue region be null. + cue->m_region = nullptr; + + // 8. Let cue’s WebVTT cue writing direction be horizontal. + cue->m_writing_direction = WritingDirection::Horizontal; + + // 9. Let cue’s WebVTT cue snap-to-lines flag be true. + cue->m_snap_to_lines = true; + + // FIXME: 10. Let cue’s WebVTT cue line be auto. + + // 11. Let cue’s WebVTT cue line alignment be start alignment. + cue->m_line_alignment = Bindings::LineAlignSetting::Start; + + // FIXME: 12. Let cue’s WebVTT cue position be auto. + + // 13. Let cue’s WebVTT cue position alignment be auto. + cue->m_position_alignment = Bindings::PositionAlignSetting::Auto; + + // 14. Let cue’s WebVTT cue size be 100. + cue->m_size = 100; + + // 15. Let cue’s WebVTT cue text alignment be center alignment. + cue->m_text_alignment = Bindings::AlignSetting::Center; + + // 16. Return the VTTCue object representing cue. + return cue; +} + +VTTCue::VTTCue(JS::Realm& realm, JS::GCPtr track) + : HTML::TextTrackCue(realm, track) +{ +} + +void VTTCue::initialize(JS::Realm& realm) +{ + Base::initialize(realm); + WEB_SET_PROTOTYPE_FOR_INTERFACE(VTTCue); +} + +void VTTCue::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_region); +} + +// https://w3c.github.io/webvtt/#dom-vttcue-vertical +Bindings::DirectionSetting VTTCue::vertical() const +{ + switch (m_writing_direction) { + case WritingDirection::Horizontal: + return Bindings::DirectionSetting::Empty; + case WritingDirection::VerticalGrowingLeft: + return Bindings::DirectionSetting::Rl; + case WritingDirection::VerticalGrowingRight: + return Bindings::DirectionSetting::Lr; + } + VERIFY_NOT_REACHED(); +} + +// https://w3c.github.io/webvtt/#dom-vttcue-vertical +void VTTCue::set_vertical(Bindings::DirectionSetting vertical) +{ + switch (vertical) { + case Bindings::DirectionSetting::Empty: + m_writing_direction = WritingDirection::Horizontal; + break; + case Bindings::DirectionSetting::Rl: + m_writing_direction = WritingDirection::VerticalGrowingLeft; + break; + case Bindings::DirectionSetting::Lr: + m_writing_direction = WritingDirection::VerticalGrowingRight; + break; + } +} + +// https://w3c.github.io/webvtt/#cue-computed-position-alignment +Bindings::PositionAlignSetting VTTCue::computed_position_alignment() +{ + // 1. If the WebVTT cue position alignment is not auto, then return the value of the WebVTT cue position alignment and abort these + // steps. + if (m_position_alignment != Bindings::PositionAlignSetting::Auto) + return m_position_alignment; + + // 2. If the WebVTT cue text alignment is left, return line-left and abort these steps. + if (m_text_alignment == Bindings::AlignSetting::Left) + return Bindings::PositionAlignSetting::LineLeft; + + // 3. If the WebVTT cue text alignment is right, return line-right and abort these steps. + if (m_text_alignment == Bindings::AlignSetting::Right) + return Bindings::PositionAlignSetting::LineRight; + + // FIXME: 4. If the WebVTT cue text alignment is start, return line-left if the base direction of the cue text is left-to-right, line-right + // otherwise. + + // FIXME: 5. If the WebVTT cue text alignment is end, return line-right if the base direction of the cue text is left-to-right, line-left + // otherwise. + + // 6. Otherwise, return center. + return Bindings::PositionAlignSetting::Center; +} + +} diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTCue.h b/Userland/Libraries/LibWeb/WebVTT/VTTCue.h new file mode 100644 index 00000000000000..71aeb50eb03352 --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTCue.h @@ -0,0 +1,94 @@ +/* + * Copyright (c) 2024, Jamie Mansfield + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include +#include + +namespace Web::WebVTT { + +// https://w3c.github.io/webvtt/#vttcue +class VTTCue final : public HTML::TextTrackCue { + WEB_PLATFORM_OBJECT(VTTCue, HTML::TextTrackCue); + JS_DECLARE_ALLOCATOR(VTTCue); + +public: + enum class WritingDirection : u8 { + // https://w3c.github.io/webvtt/#webvtt-cue-horizontal-writing-direction + Horizontal, + + // https://w3c.github.io/webvtt/#webvtt-cue-vertical-growing-left-writing-direction + VerticalGrowingLeft, + + // https://w3c.github.io/webvtt/#webvtt-cue-vertical-growing-right-writing-direction + VerticalGrowingRight, + }; + + static WebIDL::ExceptionOr> construct_impl(JS::Realm&, double start_time, double end_time, String const& text); + virtual ~VTTCue() override = default; + + JS::GCPtr region() const { return m_region; } + void set_region(JS::GCPtr region) { m_region = region; } + + Bindings::DirectionSetting vertical() const; + void set_vertical(Bindings::DirectionSetting); + + bool snap_to_lines() const { return m_snap_to_lines; } + void set_snap_to_lines(bool snap_to_lines) { m_snap_to_lines = snap_to_lines; } + + Bindings::LineAlignSetting line_align() const { return m_line_alignment; } + void set_line_align(Bindings::LineAlignSetting line_align) { m_line_alignment = line_align; } + + Bindings::PositionAlignSetting position_align() const { return m_position_alignment; } + void set_position_align(Bindings::PositionAlignSetting position_align) { m_position_alignment = position_align; } + + double size() const { return m_size; } + void set_size(double size) { m_size = size; } + + Bindings::AlignSetting align() const { return m_text_alignment; } + void set_align(Bindings::AlignSetting align) { m_text_alignment = align; } + + String const& text() const { return m_text; } + void set_text(String const& text) { m_text = text; } + +protected: + Bindings::PositionAlignSetting computed_position_alignment(); + +private: + VTTCue(JS::Realm&, JS::GCPtr); + + virtual void initialize(JS::Realm&) override; + virtual void visit_edges(Visitor&) override; + + // https://w3c.github.io/webvtt/#cue-text + String m_text; + + // https://w3c.github.io/webvtt/#webvtt-cue-writing-direction + WritingDirection m_writing_direction { WritingDirection::Horizontal }; + + // https://w3c.github.io/webvtt/#webvtt-cue-snap-to-lines-flag + bool m_snap_to_lines { true }; + + // https://w3c.github.io/webvtt/#webvtt-cue-line-alignment + Bindings::LineAlignSetting m_line_alignment { Bindings::LineAlignSetting::Start }; + + // https://w3c.github.io/webvtt/#webvtt-cue-position-alignment + Bindings::PositionAlignSetting m_position_alignment { Bindings::PositionAlignSetting::Auto }; + + // https://w3c.github.io/webvtt/#webvtt-cue-size + double m_size { 100 }; + + // https://w3c.github.io/webvtt/#webvtt-cue-text-alignment + Bindings::AlignSetting m_text_alignment { Bindings::AlignSetting::Center }; + + // https://w3c.github.io/webvtt/#webvtt-cue-region + JS::GCPtr m_region; +}; + +} diff --git a/Userland/Libraries/LibWeb/WebVTT/VTTCue.idl b/Userland/Libraries/LibWeb/WebVTT/VTTCue.idl new file mode 100644 index 00000000000000..30f7e46e30aae2 --- /dev/null +++ b/Userland/Libraries/LibWeb/WebVTT/VTTCue.idl @@ -0,0 +1,26 @@ +#import +#import + +enum AutoKeyword { "auto" }; +typedef (double or AutoKeyword) LineAndPositionSetting; +enum DirectionSetting { "", "rl", "lr" }; +enum LineAlignSetting { "start", "center", "end" }; +enum PositionAlignSetting { "line-left", "center", "line-right", "auto" }; +enum AlignSetting { "start", "center", "end", "left", "right" }; + +// https://w3c.github.io/webvtt/#vttcue +[Exposed=Window] +interface VTTCue : TextTrackCue { + constructor(double startTime, unrestricted double endTime, DOMString text); + attribute VTTRegion? region; + attribute DirectionSetting vertical; + attribute boolean snapToLines; + [FIXME] attribute LineAndPositionSetting line; + attribute LineAlignSetting lineAlign; + [FIXME] attribute LineAndPositionSetting position; + attribute PositionAlignSetting positionAlign; + attribute double size; + attribute AlignSetting align; + attribute DOMString text; + [FIXME] DocumentFragment getCueAsHTML(); +}; diff --git a/Userland/Libraries/LibWeb/idl_files.cmake b/Userland/Libraries/LibWeb/idl_files.cmake index 384cc0538c0f2d..0752a85a01ca3e 100644 --- a/Userland/Libraries/LibWeb/idl_files.cmake +++ b/Userland/Libraries/LibWeb/idl_files.cmake @@ -344,6 +344,7 @@ libweb_js_bindings(WebGL/WebGLContextEvent) libweb_js_bindings(WebGL/WebGLRenderingContext) libweb_js_bindings(WebIDL/DOMException) libweb_js_bindings(WebSockets/WebSocket) +libweb_js_bindings(WebVTT/VTTCue) libweb_js_bindings(WebVTT/VTTRegion) libweb_js_bindings(XHR/FormData ITERABLE) libweb_js_bindings(XHR/ProgressEvent) From eb98ccf7c8672f0812c91b48069f36ed3838c5c8 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 25 Sep 2024 14:22:50 +0100 Subject: [PATCH 28/43] LibWeb/CSS: Correct behavior of `revert` inside a `@layer` `revert` is supposed to revert to the previous cascade origin, but we previously had it reverting to the previous layer. To support both, track them separately during the cascade. As part of this, we make `set_property_expanding_shorthands()` fall back to `initial` if it can't find a previous value to revert to. Previously we would just shrug and do nothing if that happened, which only works if the value you want to revert to is whatever is currently in `style`. That's no longer the case, because `revert` should skip over any layer styles that have been applied since the previous origin. (cherry picked from commit bea47a25545adfb96d83a16a3e4f4435bae05e39) --- .../expected/css/revert-ignores-layers.txt | 1 + .../Text/input/css/revert-ignores-layers.html | 35 +++++++++ .../Libraries/LibWeb/CSS/StyleComputer.cpp | 76 ++++++++++++------- Userland/Libraries/LibWeb/CSS/StyleComputer.h | 6 +- 4 files changed, 88 insertions(+), 30 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/css/revert-ignores-layers.txt create mode 100644 Tests/LibWeb/Text/input/css/revert-ignores-layers.html diff --git a/Tests/LibWeb/Text/expected/css/revert-ignores-layers.txt b/Tests/LibWeb/Text/expected/css/revert-ignores-layers.txt new file mode 100644 index 00000000000000..26a017828be187 --- /dev/null +++ b/Tests/LibWeb/Text/expected/css/revert-ignores-layers.txt @@ -0,0 +1 @@ + PASS, revert skipped the layers diff --git a/Tests/LibWeb/Text/input/css/revert-ignores-layers.html b/Tests/LibWeb/Text/input/css/revert-ignores-layers.html new file mode 100644 index 00000000000000..35ad4720355794 --- /dev/null +++ b/Tests/LibWeb/Text/input/css/revert-ignores-layers.html @@ -0,0 +1,35 @@ + + + +
+
+ diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 9c272b79f59a88..385249c6b6dcd7 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -788,18 +788,25 @@ void StyleComputer::for_each_property_expanding_shorthands(PropertyID property_i set_longhand_property(property_id, value); } -void StyleComputer::set_property_expanding_shorthands(StyleProperties& style, CSS::PropertyID property_id, CSSStyleValue const& value, CSS::CSSStyleDeclaration const* declaration, StyleProperties const& style_for_revert, Important important) +void StyleComputer::set_property_expanding_shorthands(StyleProperties& style, PropertyID property_id, CSSStyleValue const& value, CSSStyleDeclaration const* declaration, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer, Important important) { + auto revert_shorthand = [&](PropertyID shorthand_id, StyleProperties const& style_for_revert) { + auto previous_value = style_for_revert.m_data->m_property_values[to_underlying(shorthand_id)]; + if (!previous_value) + previous_value = CSSKeywordValue::create(Keyword::Initial); + + style.set_property(shorthand_id, *previous_value, StyleProperties::Inherited::No, important); + if (shorthand_id == CSS::PropertyID::AnimationName) + style.set_animation_name_source(style_for_revert.animation_name_source()); + if (shorthand_id == CSS::PropertyID::TransitionProperty) + style.set_transition_property_source(style_for_revert.transition_property_source()); + }; + for_each_property_expanding_shorthands(property_id, value, AllowUnresolved::No, [&](PropertyID shorthand_id, CSSStyleValue const& shorthand_value) { - if (shorthand_value.is_revert() || shorthand_value.is_revert_layer()) { - auto const& property_in_previous_cascade_origin = style_for_revert.m_data->m_property_values[to_underlying(shorthand_id)]; - if (property_in_previous_cascade_origin) { - style.set_property(shorthand_id, *property_in_previous_cascade_origin, StyleProperties::Inherited::No, important); - if (shorthand_id == CSS::PropertyID::AnimationName) - style.set_animation_name_source(style_for_revert.animation_name_source()); - if (shorthand_id == CSS::PropertyID::TransitionProperty) - style.set_transition_property_source(style_for_revert.transition_property_source()); - } + if (shorthand_value.is_revert()) { + revert_shorthand(shorthand_id, style_for_revert); + } else if (shorthand_value.is_revert_layer()) { + revert_shorthand(shorthand_id, style_for_revert_layer); } else { style.set_property(shorthand_id, shorthand_value, StyleProperties::Inherited::No, important); if (shorthand_id == CSS::PropertyID::AnimationName) @@ -810,16 +817,21 @@ void StyleComputer::set_property_expanding_shorthands(StyleProperties& style, CS }); } -void StyleComputer::set_all_properties(DOM::Element& element, Optional pseudo_element, StyleProperties& style, CSSStyleValue const& value, DOM::Document& document, CSS::CSSStyleDeclaration const* declaration, StyleProperties const& style_for_revert, Important important) const +void StyleComputer::set_all_properties(DOM::Element& element, Optional pseudo_element, StyleProperties& style, CSSStyleValue const& value, DOM::Document& document, CSS::CSSStyleDeclaration const* declaration, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer, Important important) const { for (auto i = to_underlying(CSS::first_longhand_property_id); i <= to_underlying(CSS::last_longhand_property_id); ++i) { auto property_id = (CSS::PropertyID)i; - if (value.is_revert() || value.is_revert_layer()) { + if (value.is_revert()) { style.revert_property(property_id, style_for_revert); continue; } + if (value.is_revert_layer()) { + style.revert_property(property_id, style_for_revert_layer); + continue; + } + if (value.is_unset()) { if (is_inherited_property(property_id)) { style.set_property( @@ -841,25 +853,23 @@ void StyleComputer::set_all_properties(DOM::Element& element, Optionalis_unresolved()) property_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingContext { document }, element, pseudo_element, property_id, property_value->as_unresolved()); if (!property_value->is_unresolved()) - set_property_expanding_shorthands(style, property_id, property_value, declaration, style_for_revert); + set_property_expanding_shorthands(style, property_id, property_value, declaration, style_for_revert, style_for_revert_layer); style.set_property_important(property_id, important); - set_property_expanding_shorthands(style, property_id, value, declaration, style_for_revert, important); + set_property_expanding_shorthands(style, property_id, value, declaration, style_for_revert, style_for_revert_layer, important); } } -void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Optional pseudo_element, Vector const& matching_rules, CascadeOrigin cascade_origin, Important important) const +void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Optional pseudo_element, Vector const& matching_rules, CascadeOrigin cascade_origin, Important important, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer) const { - auto style_for_revert = style.clone(); - for (auto const& match : matching_rules) { for (auto const& property : match.rule->declaration().properties()) { if (important != property.important) continue; if (property.property_id == CSS::PropertyID::All) { - set_all_properties(element, pseudo_element, style, property.value, m_document, &match.rule->declaration(), style_for_revert, important); + set_all_properties(element, pseudo_element, style, property.value, m_document, &match.rule->declaration(), style_for_revert, style_for_revert_layer, important); continue; } @@ -867,7 +877,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e if (property.value->is_unresolved()) property_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingContext { document() }, element, pseudo_element, property.property_id, property.value->as_unresolved()); if (!property_value->is_unresolved()) - set_property_expanding_shorthands(style, property.property_id, property_value, &match.rule->declaration(), style_for_revert, important); + set_property_expanding_shorthands(style, property.property_id, property_value, &match.rule->declaration(), style_for_revert, style_for_revert_layer, important); } } @@ -878,7 +888,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e continue; if (property.property_id == CSS::PropertyID::All) { - set_all_properties(element, pseudo_element, style, property.value, m_document, inline_style, style_for_revert, important); + set_all_properties(element, pseudo_element, style, property.value, m_document, inline_style, style_for_revert, style_for_revert_layer, important); continue; } @@ -886,7 +896,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e if (property.value->is_unresolved()) property_value = Parser::Parser::resolve_unresolved_style_value(Parser::ParsingContext { document() }, element, pseudo_element, property.property_id, property.value->as_unresolved()); if (!property_value->is_unresolved()) - set_property_expanding_shorthands(style, property.property_id, property_value, inline_style, style_for_revert, important); + set_property_expanding_shorthands(style, property.property_id, property_value, inline_style, style_for_revert, style_for_revert_layer, important); } } } @@ -1424,12 +1434,17 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element // Then we apply the declarations from the matched rules in cascade order: // Normal user agent declarations - cascade_declarations(style, element, pseudo_element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::No); + auto previous_origin_style = style.clone(); + auto previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::No, previous_origin_style, previous_layer_style); // Normal user declarations - cascade_declarations(style, element, pseudo_element, matching_rule_set.user_rules, CascadeOrigin::User, Important::No); + previous_origin_style = style.clone(); + previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, matching_rule_set.user_rules, CascadeOrigin::User, Important::No, previous_origin_style, previous_layer_style); // Author presentational hints (NOTE: The spec doesn't say exactly how to prioritize these.) + previous_origin_style = style.clone(); if (!pseudo_element.has_value()) { element.apply_presentational_hints(style); @@ -1452,7 +1467,8 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element // Normal author declarations, ordered by @layer, with un-@layer-ed rules last for (auto const& layer : matching_rule_set.author_rules) { - cascade_declarations(style, element, pseudo_element, layer.rules, CascadeOrigin::Author, Important::No); + previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, layer.rules, CascadeOrigin::Author, Important::No, previous_origin_style, previous_layer_style); } // Animation declarations [css-animations-2] @@ -1518,15 +1534,21 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element } // Important author declarations, with un-@layer-ed rules first, followed by each @layer in reverse order. + previous_origin_style = style.clone(); for (auto const& layer : matching_rule_set.author_rules.in_reverse()) { - cascade_declarations(style, element, pseudo_element, layer.rules, CascadeOrigin::Author, Important::Yes); + previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, layer.rules, CascadeOrigin::Author, Important::Yes, previous_origin_style, previous_layer_style); } // Important user declarations - cascade_declarations(style, element, pseudo_element, matching_rule_set.user_rules, CascadeOrigin::User, Important::Yes); + previous_origin_style = style.clone(); + previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, matching_rule_set.user_rules, CascadeOrigin::User, Important::Yes, previous_origin_style, previous_layer_style); // Important user agent declarations - cascade_declarations(style, element, pseudo_element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes); + previous_origin_style = style.clone(); + previous_layer_style = style.clone(); + cascade_declarations(style, element, pseudo_element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes, previous_origin_style, previous_layer_style); // Transition declarations [css-transitions-1] // Note that we have to do these after finishing computing the style, diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index 243936b6e23521..9ac1c8fda9aed3 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -114,7 +114,7 @@ class StyleComputer { No, }; static void for_each_property_expanding_shorthands(PropertyID, CSSStyleValue const&, AllowUnresolved, Function const& set_longhand_property); - static void set_property_expanding_shorthands(StyleProperties&, PropertyID, CSSStyleValue const&, CSS::CSSStyleDeclaration const*, StyleProperties const& style_for_revert, Important = Important::No); + static void set_property_expanding_shorthands(StyleProperties&, PropertyID, CSSStyleValue const&, CSS::CSSStyleDeclaration const*, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer, Important = Important::No); static NonnullRefPtr get_inherit_value(JS::Realm& initial_value_context_realm, CSS::PropertyID, DOM::Element const*, Optional = {}); explicit StyleComputer(DOM::Document&); @@ -181,7 +181,7 @@ class StyleComputer { void compute_defaulted_property_value(StyleProperties&, DOM::Element const*, CSS::PropertyID, Optional) const; - void set_all_properties(DOM::Element&, Optional, StyleProperties&, CSSStyleValue const&, DOM::Document&, CSS::CSSStyleDeclaration const*, StyleProperties const& style_for_revert, Important = Important::No) const; + void set_all_properties(DOM::Element&, Optional, StyleProperties&, CSSStyleValue const&, DOM::Document&, CSS::CSSStyleDeclaration const*, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer, Important = Important::No) const; template void for_each_stylesheet(CascadeOrigin, Callback) const; @@ -204,7 +204,7 @@ class StyleComputer { Vector author_rules; }; - void cascade_declarations(StyleProperties&, DOM::Element&, Optional, Vector const&, CascadeOrigin, Important) const; + void cascade_declarations(StyleProperties&, DOM::Element&, Optional, Vector const&, CascadeOrigin, Important, StyleProperties const& style_for_revert, StyleProperties const& style_for_revert_layer) const; void build_rule_cache(); void build_rule_cache_if_needed() const; From b427f1bb2ba4be959763d105f7acf3fa69fdb83b Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Wed, 25 Sep 2024 15:11:48 +0100 Subject: [PATCH 29/43] LibWeb/CSS: Clarify comment about cascading presentational hints The spec allows us to either treat them as part of the UA origin, or as its own origin before author styles. This second behaviour turns out to be what we are currently doing, which is nice! Funnily enough this was clarified in the spec barely a month after this original comment was written. :^) (cherry picked from commit dcf55dd4924e5369d75a3533af2b869033a0ebfd) --- Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 385249c6b6dcd7..bb0595c63f147b 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1443,7 +1443,12 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element previous_layer_style = style.clone(); cascade_declarations(style, element, pseudo_element, matching_rule_set.user_rules, CascadeOrigin::User, Important::No, previous_origin_style, previous_layer_style); - // Author presentational hints (NOTE: The spec doesn't say exactly how to prioritize these.) + // Author presentational hints + // The spec calls this a special "Author presentational hint origin": + // "For the purpose of cascading this author presentational hint origin is treated as an independent origin; + // however for the purpose of the revert keyword (but not for the revert-layer keyword) it is considered + // part of the author origin." + // https://drafts.csswg.org/css-cascade-5/#author-presentational-hint-origin previous_origin_style = style.clone(); if (!pseudo_element.has_value()) { element.apply_presentational_hints(style); From 30e64420e214265a1b66c8c157da1702413768c5 Mon Sep 17 00:00:00 2001 From: Jamie Mansfield Date: Wed, 25 Sep 2024 21:19:02 +0100 Subject: [PATCH 30/43] LibWeb/Fetch: Handle edge cases in 'get, decode, and split' See: - https://github.com/whatwg/fetch/pull/1769 - https://github.com/whatwg/fetch/commit/3153e5e (cherry picked from commit 84351dfa51dc8bd0046c3a9ec3e574c58fe9f790) --- .../Fetch/Infrastructure/HTTP/Headers.cpp | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Headers.cpp b/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Headers.cpp index 96a886cce87231..77d372c995ab15 100644 --- a/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Headers.cpp +++ b/Userland/Libraries/LibWeb/Fetch/Infrastructure/HTTP/Headers.cpp @@ -133,37 +133,26 @@ Optional> get_decode_and_split_header_value(ReadonlyBytes value) // 2. Let position be a position variable for input, initially pointing at the start of input. auto lexer = GenericLexer { input }; - // 3. Let values be a list of strings, initially empty. + // 3. Let values be a list of strings, initially « ». Vector values; // 4. Let temporaryValue be the empty string. StringBuilder temporary_value_builder; - // 5. While position is not past the end of input: - while (!lexer.is_eof()) { + // 5. While true: + while (true) { // 1. Append the result of collecting a sequence of code points that are not U+0022 (") or U+002C (,) from input, given position, to temporaryValue. // NOTE: The result might be the empty string. temporary_value_builder.append(lexer.consume_until(is_any_of("\","sv))); - // 2. If position is not past the end of input, then: - if (!lexer.is_eof()) { - // 1. If the code point at position within input is U+0022 ("), then: - if (lexer.peek() == '"') { - // 1. Append the result of collecting an HTTP quoted string from input, given position, to temporaryValue. - temporary_value_builder.append(collect_an_http_quoted_string(lexer)); + // 2. If position is not past the end of input and the code point at position within input is U+0022 ("): + if (!lexer.is_eof() && lexer.peek() == '"') { + // 1. Append the result of collecting an HTTP quoted string from input, given position, to temporaryValue. + temporary_value_builder.append(collect_an_http_quoted_string(lexer)); - // 2. If position is not past the end of input, then continue. - if (!lexer.is_eof()) - continue; - } - // 2. Otherwise: - else { - // 1. Assert: the code point at position within input is U+002C (,). - VERIFY(lexer.peek() == ','); - - // 2. Advance position by 1. - lexer.ignore(1); - } + // 2. If position is not past the end of input, then continue. + if (!lexer.is_eof()) + continue; } // 3. Remove all HTTP tab or space from the start and end of temporaryValue. @@ -174,10 +163,17 @@ Optional> get_decode_and_split_header_value(ReadonlyBytes value) // 5. Set temporaryValue to the empty string. temporary_value_builder.clear(); - } - // 8. Return values. - return values; + // 6. If position is past the end of input, then return values. + if (lexer.is_eof()) + return values; + + // 7. Assert: the code point at position within input is U+002C (,). + VERIFY(lexer.peek() == ','); + + // 8. Advance position by 1. + lexer.ignore(1); + } } // https://fetch.spec.whatwg.org/#concept-header-list-append From 5bc61b65a4e9e81f2b26fd26815466124f40027a Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 26 Sep 2024 07:42:49 +0200 Subject: [PATCH 31/43] LibJS: Don't infinite loop on unknown console.log formatting specifiers (cherry picked from commit ef9208047dc8770f6263b483d7a442df703bc42b) --- .../regress/console-log-bogus-formatting-specifier.txt | 1 + .../regress/console-log-bogus-formatting-specifier.html | 7 +++++++ Userland/Libraries/LibJS/Console.cpp | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 Tests/LibWeb/Text/expected/regress/console-log-bogus-formatting-specifier.txt create mode 100644 Tests/LibWeb/Text/input/regress/console-log-bogus-formatting-specifier.html diff --git a/Tests/LibWeb/Text/expected/regress/console-log-bogus-formatting-specifier.txt b/Tests/LibWeb/Text/expected/regress/console-log-bogus-formatting-specifier.txt new file mode 100644 index 00000000000000..23b535695b95d3 --- /dev/null +++ b/Tests/LibWeb/Text/expected/regress/console-log-bogus-formatting-specifier.txt @@ -0,0 +1 @@ +PASS (didn't hang) diff --git a/Tests/LibWeb/Text/input/regress/console-log-bogus-formatting-specifier.html b/Tests/LibWeb/Text/input/regress/console-log-bogus-formatting-specifier.html new file mode 100644 index 00000000000000..80a777c6377938 --- /dev/null +++ b/Tests/LibWeb/Text/input/regress/console-log-bogus-formatting-specifier.html @@ -0,0 +1,7 @@ + + diff --git a/Userland/Libraries/LibJS/Console.cpp b/Userland/Libraries/LibJS/Console.cpp index 23b1701734b4e4..7472c52f04a12b 100644 --- a/Userland/Libraries/LibJS/Console.cpp +++ b/Userland/Libraries/LibJS/Console.cpp @@ -818,7 +818,7 @@ ThrowCompletionOr> ConsoleClient::formatter(MarkedVector Optional { size_t start_index = 0; while (start_index < target.length()) { - auto maybe_index = target.find('%'); + auto maybe_index = target.find('%', start_index); if (!maybe_index.has_value()) return {}; From f89aa4fa45793f35fd628005ecbc7c9581e106ef Mon Sep 17 00:00:00 2001 From: Tim Ledbetter Date: Thu, 26 Sep 2024 15:40:23 +0100 Subject: [PATCH 32/43] LibWeb: Fire error event when script has an execution error We now use the "report an exception" AO when a script has an execution error. This has mostly replaced the older "report the exception" AO in various specifications. Using this newer AO ensures that `window.onerror` is invoked when a script has an execution error. (cherry picked from commit 579a289d3db849657987c3310e7b1d71d290b566) --- .../LibWeb/Text/expected/HTML/Window-onerror.txt | 1 + Tests/LibWeb/Text/input/HTML/Window-onerror.html | 16 ++++++++++++++++ .../Libraries/LibWeb/DOM/EventDispatcher.cpp | 7 +++++-- .../LibWeb/HTML/Scripting/ClassicScript.cpp | 7 +++++-- .../LibWeb/HTML/WindowOrWorkerGlobalScope.cpp | 7 +++++++ .../LibWeb/HTML/WindowOrWorkerGlobalScope.h | 1 + 6 files changed, 35 insertions(+), 4 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/HTML/Window-onerror.txt create mode 100644 Tests/LibWeb/Text/input/HTML/Window-onerror.html diff --git a/Tests/LibWeb/Text/expected/HTML/Window-onerror.txt b/Tests/LibWeb/Text/expected/HTML/Window-onerror.txt new file mode 100644 index 00000000000000..81ce1a1e6d2a9a --- /dev/null +++ b/Tests/LibWeb/Text/expected/HTML/Window-onerror.txt @@ -0,0 +1 @@ +onerror event fired: Error: Uncaught error diff --git a/Tests/LibWeb/Text/input/HTML/Window-onerror.html b/Tests/LibWeb/Text/input/HTML/Window-onerror.html new file mode 100644 index 00000000000000..525f1f70a61d7f --- /dev/null +++ b/Tests/LibWeb/Text/input/HTML/Window-onerror.html @@ -0,0 +1,16 @@ + + + diff --git a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp index 2ffa115579d23e..73da452c6652c2 100644 --- a/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp +++ b/Userland/Libraries/LibWeb/DOM/EventDispatcher.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -91,8 +92,10 @@ bool EventDispatcher::inner_invoke(Event& event, Vector(&global); + VERIFY(window_or_worker); + window_or_worker->report_an_exception(*result.release_error().value()); // FIXME: 2. Set legacyOutputDidListenersThrowFlag if given. (Only used by IndexedDB currently) } diff --git a/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp b/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp index f4e590048a2f86..69f6586075de6f 100644 --- a/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp +++ b/Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace Web::HTML { @@ -126,8 +127,10 @@ JS::Completion ClassicScript::run(RethrowErrors rethrow_errors, JS::GCPtr(&settings.global_object()); + VERIFY(window_or_worker); + window_or_worker->report_an_exception(*evaluation_status.value()); // 2. Clean up after running script with settings. settings.clean_up_after_running_script(); diff --git a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp index 4cddc7f9ba6e4c..dfefaeaf161132 100644 --- a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp +++ b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.cpp @@ -747,6 +747,13 @@ JS::NonnullGCPtr WindowOrWorkerGlobalScopeMixin::supported_entry_typ // https://html.spec.whatwg.org/multipage/webappapis.html#dom-reporterror void WindowOrWorkerGlobalScopeMixin::report_error(JS::Value e) +{ + // The reportError(e) method steps are to report an exception e for this. + report_an_exception(e); +} + +// https://html.spec.whatwg.org/multipage/webappapis.html#report-an-exception +void WindowOrWorkerGlobalScopeMixin::report_an_exception(JS::Value const& e) { auto& target = static_cast(this_impl()); auto& realm = relevant_realm(target); diff --git a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h index 88635bc66c65b5..5aa68db9e95387 100644 --- a/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h +++ b/Userland/Libraries/LibWeb/HTML/WindowOrWorkerGlobalScope.h @@ -77,6 +77,7 @@ class WindowOrWorkerGlobalScopeMixin { JS::NonnullGCPtr indexed_db(); void report_error(JS::Value e); + void report_an_exception(JS::Value const& e); [[nodiscard]] JS::NonnullGCPtr crypto(); From 8188edde635e4ab70fee10bf4535ec613e79af4a Mon Sep 17 00:00:00 2001 From: Chase Willden Date: Thu, 26 Sep 2024 19:35:58 -0600 Subject: [PATCH 33/43] Base: Navigate DOM tree with arrows (cherry picked from commit 9d82e8112493575f78f66b1a0dc65ae0e791e70b) --- Base/res/ladybird/inspector.js | 49 +++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/Base/res/ladybird/inspector.js b/Base/res/ladybird/inspector.js index d35a75e993d326..8d9093064380f6 100644 --- a/Base/res/ladybird/inspector.js +++ b/Base/res/ladybird/inspector.js @@ -614,30 +614,47 @@ document.addEventListener("DOMContentLoaded", () => { document.addEventListener("keydown", event => { const UP_ARROW_KEYCODE = 38; const DOWN_ARROW_KEYCODE = 40; + const RIGHT_ARROW_KEYCODE = 39; + const LEFT_ARROW_KEYCODE = 37; const RETURN_KEYCODE = 13; const SPACE_KEYCODE = 32; - if (document.activeElement.tagName !== "INPUT") { - if (event.keyCode == UP_ARROW_KEYCODE || event.keyCode == DOWN_ARROW_KEYCODE) { - let selectedIndex = visibleDOMNodes.indexOf(selectedDOMNode); - if (selectedIndex < 0) { - return; - } + const move = delta => { + let selectedIndex = visibleDOMNodes.indexOf(selectedDOMNode); + if (selectedIndex < 0) { + return; + } - let newIndex; + let newIndex = selectedIndex + delta; - if (event.keyCode == UP_ARROW_KEYCODE) { - newIndex = selectedIndex - 1; - } else if (event.keyCode == DOWN_ARROW_KEYCODE) { - newIndex = selectedIndex + 1; - } + if (visibleDOMNodes[newIndex]) { + inspectDOMNode(visibleDOMNodes[newIndex]); + } + }; - if (visibleDOMNodes[newIndex]) { - inspectDOMNode(visibleDOMNodes[newIndex]); - } + if (document.activeElement.tagName !== "INPUT") { + const isSummary = selectedDOMNode.parentNode.tagName === "SUMMARY"; + const isDiv = selectedDOMNode.parentNode.tagName === "DIV"; + + if (event.keyCode == UP_ARROW_KEYCODE) { + move(-1); + } else if (event.keyCode == DOWN_ARROW_KEYCODE) { + move(1); } else if (event.keyCode == RETURN_KEYCODE || event.keyCode == SPACE_KEYCODE) { - if (selectedDOMNode.parentNode.tagName === "SUMMARY") { + if (isSummary) { + selectedDOMNode.parentNode.click(); + } + } else if (event.keyCode == RIGHT_ARROW_KEYCODE) { + if (isSummary && selectedDOMNode.parentNode.parentNode.open === false) { + selectedDOMNode.parentNode.click(); + } else if (selectedDOMNode.parentNode.parentNode.open === true && !isDiv) { + move(1); + } + } else if (event.keyCode == LEFT_ARROW_KEYCODE) { + if (isSummary && selectedDOMNode.parentNode.parentNode.open === true) { selectedDOMNode.parentNode.click(); + } else if (selectedDOMNode.parentNode.parentNode.open === false || isDiv) { + move(-1); } } } From ebe0797e24e9001bc9ac09d9e16d69daad92b389 Mon Sep 17 00:00:00 2001 From: Gingeh <39150378+Gingeh@users.noreply.github.com> Date: Wed, 25 Sep 2024 17:38:59 +1000 Subject: [PATCH 34/43] LibWeb: Use substring matching for content filters (cherry picked from commit 2e5edcf27e7d2a212043a7ac1b906217b357b964) --- Base/home/anon/.config/BrowserContentFilters.txt | 4 ++-- Userland/Libraries/LibWeb/Loader/ContentFilter.cpp | 12 ++---------- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/Base/home/anon/.config/BrowserContentFilters.txt b/Base/home/anon/.config/BrowserContentFilters.txt index 6efc23b0205971..3cc87438777fd8 100644 --- a/Base/home/anon/.config/BrowserContentFilters.txt +++ b/Base/home/anon/.config/BrowserContentFilters.txt @@ -406,8 +406,8 @@ glassboxdigital.io go-mpulse.net go2speed.org google-analytics.com -google.com/gen_204\? -google.com/log\? +google.com/gen_204? +google.com/log? googleadservices.com googleoptimize.com googlesyndication.com diff --git a/Userland/Libraries/LibWeb/Loader/ContentFilter.cpp b/Userland/Libraries/LibWeb/Loader/ContentFilter.cpp index d9a8c7cdf38b37..113a8cbdf55134 100644 --- a/Userland/Libraries/LibWeb/Loader/ContentFilter.cpp +++ b/Userland/Libraries/LibWeb/Loader/ContentFilter.cpp @@ -27,7 +27,7 @@ bool ContentFilter::is_filtered(const URL::URL& url) const auto url_string = url.to_byte_string(); for (auto& pattern : m_patterns) { - if (url_string.matches(pattern.text, CaseSensitivity::CaseSensitive)) + if (url_string.find(pattern.text).has_value()) return true; } return false; @@ -38,15 +38,7 @@ ErrorOr ContentFilter::set_patterns(ReadonlySpan patterns) m_patterns.clear_with_capacity(); for (auto const& pattern : patterns) { - StringBuilder builder; - - if (!pattern.starts_with('*')) - TRY(builder.try_append('*')); - TRY(builder.try_append(pattern)); - if (!pattern.ends_with('*')) - TRY(builder.try_append('*')); - - TRY(m_patterns.try_empend(TRY(builder.to_string()))); + TRY(m_patterns.try_empend(pattern)); } return {}; From 0eca32be070258a2ce3230f32fb1fb65e6d054d4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 29 Sep 2024 10:54:46 +0200 Subject: [PATCH 35/43] LibWeb: Always blockify the root element MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is what the spec tells us to do: The root element’s display type is always blockified, and its principal box always establishes an independent formatting context. Additionally, a display of contents computes to block on the root element. Spec link: https://drafts.csswg.org/css-display/#root Fixes #1562 (cherry picked from commit f1be662f683155705f851bcf440fe30d0e606a87) --- Tests/LibWeb/Layout/expected/css-all-unset.txt | 6 +++--- .../LibWeb/Layout/expected/html-display-contents.txt | 11 +++++++++++ Tests/LibWeb/Layout/expected/html-display-inline.txt | 11 +++++++++++ .../Layout/expected/replaced-within-max-content.txt | 2 +- .../expected/svg/svg-as-image-implicit-viewbox.txt | 2 +- Tests/LibWeb/Layout/expected/svg/svg-as-image.txt | 2 +- .../svg-with-zero-intrinsic-size-and-no-viewbox.txt | 6 +++--- .../expected/zero-height-viewport-svg-image.txt | 2 +- .../Layout/expected/zero-width-viewport-svg-image.txt | 2 +- Tests/LibWeb/Layout/input/html-display-contents.html | 1 + Tests/LibWeb/Layout/input/html-display-inline.html | 1 + Userland/Libraries/LibWeb/CSS/StyleComputer.cpp | 10 +++++++++- 12 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 Tests/LibWeb/Layout/expected/html-display-contents.txt create mode 100644 Tests/LibWeb/Layout/expected/html-display-inline.txt create mode 100644 Tests/LibWeb/Layout/input/html-display-contents.html create mode 100644 Tests/LibWeb/Layout/input/html-display-inline.html diff --git a/Tests/LibWeb/Layout/expected/css-all-unset.txt b/Tests/LibWeb/Layout/expected/css-all-unset.txt index c16f70bf4b038d..4ec6d920d4ad28 100644 --- a/Tests/LibWeb/Layout/expected/css-all-unset.txt +++ b/Tests/LibWeb/Layout/expected/css-all-unset.txt @@ -1,5 +1,5 @@ -Viewport <#document> at (0,0) content-size 800x600 children: inline - InlineNode +Viewport <#document> at (0,0) content-size 800x600 children: not-inline + BlockContainer at (0,0) content-size 800x17 [BFC] children: inline InlineNode InlineNode