From aa4dcda5dc40e0a054e3bc76d7bbd0b8c2b4d7e3 Mon Sep 17 00:00:00 2001 From: Timothy Flynn Date: Fri, 24 Nov 2023 12:17:16 -0500 Subject: [PATCH] LibWeb+LibWebView+Ladybird: Scale scroll-to CSS positions in PageHost The `page_did_request_scroll_to` API takes a CSS position, and thus callers should not scale to device pixels before invoking it. Instead, align this API with (most) other PageHost APIs which scale to device pixels before sending the corresponding IPC message. In the AppKit chrome, convert the provided device pixel position to a widget position. --- Ladybird/AppKit/UI/LadybirdWebView.mm | 2 +- Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp | 7 ++++++- Ladybird/AppKit/UI/LadybirdWebViewBridge.h | 1 + Userland/Libraries/LibWeb/DOM/Element.cpp | 3 +-- Userland/Services/WebContent/PageHost.cpp | 3 ++- 5 files changed, 11 insertions(+), 5 deletions(-) diff --git a/Ladybird/AppKit/UI/LadybirdWebView.mm b/Ladybird/AppKit/UI/LadybirdWebView.mm index 7d913938db..845ed3fb04 100644 --- a/Ladybird/AppKit/UI/LadybirdWebView.mm +++ b/Ladybird/AppKit/UI/LadybirdWebView.mm @@ -279,7 +279,7 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_ [self.observer onFaviconChange:bitmap]; }; - m_web_view_bridge->on_scroll_to_point = [self](auto position) { + m_web_view_bridge->on_scroll = [self](auto position) { auto content_rect = [self frame]; auto document_rect = [[self documentView] frame]; auto ns_position = Ladybird::gfx_point_to_ns_point(position); diff --git a/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp b/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp index 8765f29be4..ebbc052c4a 100644 --- a/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp +++ b/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp @@ -38,7 +38,7 @@ WebViewBridge::WebViewBridge(Vector screen_rects, float device_pix create_client(WebView::EnableCallgrindProfiling::No); on_scroll_by_delta = [this](auto x_delta, auto y_delta) { - auto position = to_widget_position(m_viewport_rect.location()); + auto position = m_viewport_rect.location(); position.set_x(position.x() + x_delta); position.set_y(position.y() + y_delta); @@ -63,6 +63,11 @@ WebViewBridge::WebViewBridge(Vector screen_rects, float device_pix if (on_scroll_to_point) on_scroll_to_point(position); }; + + on_scroll_to_point = [this](auto position) { + if (on_scroll) + on_scroll(to_widget_position(position)); + }; } WebViewBridge::~WebViewBridge() = default; diff --git a/Ladybird/AppKit/UI/LadybirdWebViewBridge.h b/Ladybird/AppKit/UI/LadybirdWebViewBridge.h index 1910651bd1..bacc73a5ac 100644 --- a/Ladybird/AppKit/UI/LadybirdWebViewBridge.h +++ b/Ladybird/AppKit/UI/LadybirdWebViewBridge.h @@ -56,6 +56,7 @@ public: Optional paintable(); Function on_zoom_level_changed; + Function on_scroll; private: WebViewBridge(Vector screen_rects, float device_pixel_ratio, Optional webdriver_content_ipc_path, Web::CSS::PreferredColorScheme); diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index 133209b503..7203b03af3 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -1669,8 +1669,7 @@ static ErrorOr scroll_an_element_into_view(DOM::Element& target, Bindings: // AD-HOC: auto* page = document.navigable()->traversable_navigable()->page(); - auto scale = page->client().device_pixels_per_css_pixel(); - page->client().page_did_request_scroll_to({ position.left() * scale, position.top() * scale }); + page->client().page_did_request_scroll_to(position.location()); } // If scrolling box is associated with an element else { diff --git a/Userland/Services/WebContent/PageHost.cpp b/Userland/Services/WebContent/PageHost.cpp index 7e0b478578..9c2ad87730 100644 --- a/Userland/Services/WebContent/PageHost.cpp +++ b/Userland/Services/WebContent/PageHost.cpp @@ -257,7 +257,8 @@ void PageHost::page_did_request_scroll(i32 x_delta, i32 y_delta) void PageHost::page_did_request_scroll_to(Web::CSSPixelPoint scroll_position) { - m_client.async_did_request_scroll_to({ scroll_position.x().to_int(), scroll_position.y().to_int() }); + auto device_scroll_position = page().css_to_device_point(scroll_position); + m_client.async_did_request_scroll_to(device_scroll_position.to_type()); } void PageHost::page_did_request_scroll_into_view(Web::CSSPixelRect const& rect)