From b3d9e39bad4f4a394cf518f54af1937967fbea29 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 30 Jun 2025 15:11:44 +0200 Subject: [PATCH] LibWeb: Avoid infinite loop in HTMLElement.scrollParent We were failing to actually climb up the containing block chain, causing this API to infinite loop for anything but the most trivial cases. By fixing the loop structure, we also make a bunch of the already imported WPT tests pass. :^) --- Libraries/LibWeb/DOM/Document.h | 1 + Libraries/LibWeb/HTML/HTMLElement.cpp | 12 +++++++++--- .../expected/HTML/scrollParent-infinite-loop.txt | 1 + .../css/cssom-view/scrollParent-shadow-tree.txt | 9 +++++---- .../wpt-import/css/cssom-view/scrollParent.txt | 16 ++++++++-------- .../input/HTML/scrollParent-infinite-loop.html | 9 +++++++++ 6 files changed, 33 insertions(+), 15 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/HTML/scrollParent-infinite-loop.txt create mode 100644 Tests/LibWeb/Text/input/HTML/scrollParent-infinite-loop.html diff --git a/Libraries/LibWeb/DOM/Document.h b/Libraries/LibWeb/DOM/Document.h index c999ba30f08..b9564831282 100644 --- a/Libraries/LibWeb/DOM/Document.h +++ b/Libraries/LibWeb/DOM/Document.h @@ -98,6 +98,7 @@ enum class InvalidateLayoutTreeReason { X(HTMLElementOffsetParent) \ X(HTMLElementOffsetTop) \ X(HTMLElementOffsetWidth) \ + X(HTMLElementScrollParent) \ X(HTMLEventLoopRenderingUpdate) \ X(HTMLImageElementHeight) \ X(HTMLImageElementWidth) \ diff --git a/Libraries/LibWeb/HTML/HTMLElement.cpp b/Libraries/LibWeb/HTML/HTMLElement.cpp index e4d4a56f2c9..a99a816e71e 100644 --- a/Libraries/LibWeb/HTML/HTMLElement.cpp +++ b/Libraries/LibWeb/HTML/HTMLElement.cpp @@ -464,6 +464,9 @@ String HTMLElement::outer_text() // https://drafts.csswg.org/cssom-view/#dom-htmlelement-scrollparent GC::Ptr HTMLElement::scroll_parent() const { + // NOTE: We have to ensure that the layout is up-to-date before querying the layout tree. + const_cast(document()).update_layout(DOM::UpdateLayoutReason::HTMLElementScrollParent); + // 1. If any of the following holds true, return null and terminate this algorithm: // - The element does not have an associated box. // - The element is the root element. @@ -478,7 +481,7 @@ GC::Ptr HTMLElement::scroll_parent() const // 2. Let ancestor be the containing block of the element in the flat tree and repeat these substeps: auto ancestor = layout_node()->containing_block(); - while (true) { + while (ancestor) { // 1. If ancestor is the initial containing block, return the scrollingElement for the element’s document if it // is not closed-shadow-hidden from the element, otherwise return null. if (ancestor->is_viewport()) { @@ -490,7 +493,8 @@ GC::Ptr HTMLElement::scroll_parent() const // 2. If ancestor is not closed-shadow-hidden from the element, and is a scroll container, terminate this // algorithm and return ancestor. - if (!ancestor->dom_node()->is_closed_shadow_hidden_from(*this) && ancestor->is_scroll_container()) { + if ((ancestor->dom_node() && !ancestor->dom_node()->is_closed_shadow_hidden_from(*this)) + && ancestor->is_scroll_container()) { return const_cast(static_cast(ancestor->dom_node())); } @@ -498,8 +502,10 @@ GC::Ptr HTMLElement::scroll_parent() const // position containing block, terminate this algorithm and return null. // 4. Let ancestor be the containing block of ancestor in the flat tree. - ancestor = layout_node()->containing_block(); + ancestor = ancestor->containing_block(); } + + return nullptr; } // https://www.w3.org/TR/cssom-view-1/#dom-htmlelement-offsetparent diff --git a/Tests/LibWeb/Text/expected/HTML/scrollParent-infinite-loop.txt b/Tests/LibWeb/Text/expected/HTML/scrollParent-infinite-loop.txt new file mode 100644 index 00000000000..23b535695b9 --- /dev/null +++ b/Tests/LibWeb/Text/expected/HTML/scrollParent-infinite-loop.txt @@ -0,0 +1 @@ +PASS (didn't hang) diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent-shadow-tree.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent-shadow-tree.txt index 7766155ca97..74a5e2fa219 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent-shadow-tree.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent-shadow-tree.txt @@ -2,8 +2,9 @@ Harness status: OK Found 4 tests -4 Fail -Fail scrollParent skips intermediate closed shadow tree nodes +3 Pass +1 Fail +Pass scrollParent skips intermediate closed shadow tree nodes Fail scrollParent skips intermediate open shadow tree nodes -Fail scrollParent from inside closed shadow tree -Fail scrollParent from inside open shadow tree \ No newline at end of file +Pass scrollParent from inside closed shadow tree +Pass scrollParent from inside open shadow tree \ No newline at end of file diff --git a/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent.txt b/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent.txt index 3013b6c72af..cf40b1e9c45 100644 --- a/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent.txt +++ b/Tests/LibWeb/Text/expected/wpt-import/css/cssom-view/scrollParent.txt @@ -2,14 +2,14 @@ Harness status: OK Found 9 tests -3 Pass -6 Fail -Fail scrollParent returns the nearest scroll container. -Fail hidden element is a scroll container. +7 Pass +2 Fail +Pass scrollParent returns the nearest scroll container. +Pass hidden element is a scroll container. Pass Element with no box has null scrollParent. -Fail scrollParent follows absolute positioned containing block chain. +Pass scrollParent follows absolute positioned containing block chain. Fail scrollParent follows fixed positioned containing block chain. -Pass scrollParent of element fixed to root is null. -Fail scrollParent of child in root viewport returns document scrolling element. -Fail scrollParent of fixed element contained within root is document scrolling element. +Fail scrollParent of element fixed to root is null. +Pass scrollParent of child in root viewport returns document scrolling element. +Pass scrollParent of fixed element contained within root is document scrolling element. Pass scrollParent of body is null. \ No newline at end of file diff --git a/Tests/LibWeb/Text/input/HTML/scrollParent-infinite-loop.html b/Tests/LibWeb/Text/input/HTML/scrollParent-infinite-loop.html new file mode 100644 index 00000000000..af40ae6d5f4 --- /dev/null +++ b/Tests/LibWeb/Text/input/HTML/scrollParent-infinite-loop.html @@ -0,0 +1,9 @@ + + +
+