Browse Source

LibWeb: Remove direct calls of page_did_request_scroll_to()

By replacing the `page_did_request_scroll_to()` calls with a request
to perform scrolling in the corresponding navigable, we ensure that
the scrolling of iframes will scroll within them instead of triggering
scroll of top level document.
Aliaksandr Kalenik 1 year ago
parent
commit
bf14de4118

+ 4 - 0
Tests/LibWeb/Ref/reference/scroll-iframe-ref.html

@@ -0,0 +1,4 @@
+<!DOCTYPE html>
+<body style="border: 1px solid black; width: fit-content">
+<div style="width: 200px; height: 200px; background-color: blue"></div>    
+</body>

+ 23 - 0
Tests/LibWeb/Ref/scroll-iframe.html

@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<link rel="match" href="reference/scroll-iframe-ref.html" />
+<style>
+    iframe {
+        width: 200px;
+        height: 200px;
+        border: 1px solid black;
+    }
+</style>
+<body></body>
+<script>
+    const iframe = document.createElement("iframe");
+    iframe.srcdoc = `
+        <style>body { margin: 0 }</style>
+        <div style="width: 200px; height: 200px; background-color: darkblue"></div>
+        <div style="width: 200px; height: 200px; background-color: blue"></div>
+        <div style="width: 200px; height: 200px; background-color: magenta"></div>
+    `;
+    iframe.onload = function () {
+        iframe.contentWindow.scroll(0, 200);
+    };
+    document.body.appendChild(iframe);
+</script>

+ 2 - 3
Userland/Libraries/LibWeb/DOM/Element.cpp

@@ -1711,11 +1711,10 @@ static ErrorOr<void> scroll_an_element_into_view(DOM::Element& target, Bindings:
             (void)behavior;
 
             // AD-HOC:
-            auto& page = document.navigable()->traversable_navigable()->page();
             // NOTE: Since calculated position is relative to the viewport, we need to add the viewport's position to it
-            //       before passing to page_did_request_scroll_to() that expects a position relative to the page.
+            //       before passing to perform_scroll_of_viewport() that expects a position relative to the page.
             position.set_y(position.y() + scrolling_box.y());
-            page.client().page_did_request_scroll_to(position);
+            document.navigable()->perform_scroll_of_viewport(position);
         }
         // If scrolling box is associated with an element
         else {

+ 2 - 2
Userland/Libraries/LibWeb/HTML/BrowsingContext.cpp

@@ -352,8 +352,8 @@ void BrowsingContext::scroll_to(CSSPixelPoint position)
             active_document()->update_layout();
     }
 
-    if (this == &m_page->top_level_browsing_context())
-        m_page->client().page_did_request_scroll_to(position);
+    if (auto navigable = active_document()->navigable())
+        navigable->perform_scroll_of_viewport(position);
 }
 
 JS::GCPtr<BrowsingContext> BrowsingContext::top_level_browsing_context() const

+ 11 - 0
Userland/Libraries/LibWeb/HTML/Navigable.cpp

@@ -1983,6 +1983,17 @@ void Navigable::set_viewport_rect(CSSPixelRect const& rect)
     HTML::main_thread_event_loop().schedule();
 }
 
+void Navigable::perform_scroll_of_viewport(CSSPixelPoint position)
+{
+    auto viewport_rect = this->viewport_rect();
+    viewport_rect.set_location(position);
+    set_viewport_rect(viewport_rect);
+    set_needs_display();
+
+    if (is_traversable() && active_browsing_context())
+        active_browsing_context()->page().client().page_did_request_scroll_to(position);
+}
+
 void Navigable::set_size(CSSPixelSize size)
 {
     if (m_size == size)

+ 1 - 0
Userland/Libraries/LibWeb/HTML/Navigable.h

@@ -161,6 +161,7 @@ public:
     CSSPixelPoint viewport_scroll_offset() const { return m_viewport_scroll_offset; }
     CSSPixelRect viewport_rect() const { return { m_viewport_scroll_offset, m_size }; }
     void set_viewport_rect(CSSPixelRect const&);
+    void perform_scroll_of_viewport(CSSPixelPoint position);
 
     void set_needs_display();
     void set_needs_display(CSSPixelRect const&);

+ 6 - 20
Userland/Libraries/LibWeb/HTML/Window.cpp

@@ -1232,31 +1232,18 @@ double Window::scroll_y() const
     return 0;
 }
 
-// https://w3c.github.io/csswg-drafts/cssom-view/#perform-a-scroll
-static void perform_a_scroll(Page& page, double x, double y, JS::GCPtr<DOM::Node const> element, Bindings::ScrollBehavior behavior)
-{
-    // FIXME: 1. Abort any ongoing smooth scroll for box.
-    // 2. If the user agent honors the scroll-behavior property and one of the following are true:
-    // - behavior is "auto" and element is not null and its computed value of the scroll-behavior property is smooth
-    // - behavior is smooth
-    // ...then perform a smooth scroll of box to position. Once the position has finished updating, emit the scrollend
-    // event. Otherwise, perform an instant scroll of box to position. After an instant scroll emit the scrollend event.
-    // FIXME: Support smooth scrolling.
-    (void)element;
-    (void)behavior;
-    page.client().page_did_request_scroll_to({ x, y });
-}
-
 // https://w3c.github.io/csswg-drafts/cssom-view/#dom-window-scroll
 void Window::scroll(ScrollToOptions const& options)
 {
     // 4. If there is no viewport, abort these steps.
-    auto top_level_traversable = page().top_level_traversable();
+    auto navigable = associated_document().navigable();
+    if (!navigable)
+        return;
 
     // 1. If invoked with one argument, follow these substeps:
 
     // 1. Let options be the argument.
-    auto viewport_rect = top_level_traversable->viewport_rect().to_type<float>();
+    auto viewport_rect = navigable->viewport_rect().to_type<float>();
 
     // 2. Let x be the value of the left dictionary member of options, if present, or the viewport’s current scroll
     //    position on the x axis otherwise.
@@ -1276,7 +1263,7 @@ void Window::scroll(ScrollToOptions const& options)
     // 6. Let viewport height be the height of the viewport excluding the height of the scroll bar, if any.
     auto viewport_height = viewport_rect.height();
 
-    auto const document = top_level_traversable->active_document();
+    auto const document = navigable->active_document();
     VERIFY(document);
 
     // Make sure layout is up-to-date before looking at scrollable overflow metrics.
@@ -1314,8 +1301,7 @@ void Window::scroll(ScrollToOptions const& options)
 
     // 12. Perform a scroll of the viewport to position, document’s root element as the associated element, if there is
     //     one, or null otherwise, and the scroll behavior being the value of the behavior dictionary member of options.
-    auto element = JS::GCPtr<DOM::Node const> { document ? &document->root() : nullptr };
-    perform_a_scroll(page(), x, y, element, options.behavior);
+    navigable->perform_scroll_of_viewport({ x, y });
 }
 
 // https://w3c.github.io/csswg-drafts/cssom-view/#dom-window-scroll