From 5285e22f2aa09152365179865f135e7bc5d254a5 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Mon, 3 Jun 2024 17:53:55 +0300 Subject: [PATCH] LibWeb+WebContent: Move scrollbar painting into WebContent The main intention of this change is to have a consistent look and behavior across all scrollbars, including elements with `overflow: scroll` and `overflow: auto`, iframes, and a page. Before: - Page's scrollbar is painted by Browser (Qt/AppKit) using the corresponding UI framework style, - Both WebContent and Browser know the scroll position offset. - WebContent uses did_request_scroll_to() IPC call to send updates. - Browser uses set_viewport_rect() to send updates. After: - Page's scrollbar is painted on WebContent side using the same style as currently used for elements with `overflow: scroll` and `overflow: auto`. A nice side effects: scrollbars are now painted for iframes, and page's scrollbar respects scrollbar-width CSS property. - Only WebContent knows scroll position offset. - did_request_scroll_to() is no longer used. - set_viewport_rect() is changed to set_viewport_size(). --- Ladybird/AppKit/UI/LadybirdWebView.mm | 16 ----- Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp | 22 ++----- Ladybird/AppKit/UI/LadybirdWebViewBridge.h | 5 +- Ladybird/AppKit/UI/Tab.mm | 4 +- Ladybird/Qt/WebContentView.cpp | 55 +++++----------- Ladybird/Qt/WebContentView.h | 6 +- .../LibWeb/Ref/css-background-clip-text.html | 4 ++ .../css-background-clip-text-ref.html | 3 + Tests/LibWeb/Ref/scroll-iframe.html | 5 +- Userland/Libraries/LibWeb/DOM/Document.cpp | 5 ++ Userland/Libraries/LibWeb/DOM/Element.cpp | 2 +- Userland/Libraries/LibWeb/HTML/Navigable.cpp | 62 ++++++------------- Userland/Libraries/LibWeb/HTML/Navigable.h | 3 +- Userland/Libraries/LibWeb/Layout/FrameBox.cpp | 2 +- .../Libraries/LibWeb/Page/EventHandler.cpp | 35 +++++++---- Userland/Libraries/LibWeb/Page/Page.cpp | 9 +++ Userland/Libraries/LibWeb/Page/Page.h | 1 + .../LibWeb/Painting/BackgroundPainting.cpp | 2 +- .../LibWeb/Painting/PaintableBox.cpp | 31 +++++++--- .../Libraries/LibWeb/Painting/PaintableBox.h | 2 + .../LibWeb/Painting/ViewportPaintable.cpp | 6 +- .../LibWeb/SVG/SVGDecodedImageData.cpp | 2 +- .../LibWebView/ViewImplementation.cpp | 12 ++-- .../Libraries/LibWebView/ViewImplementation.h | 2 +- .../WebContent/ConnectionFromClient.cpp | 4 +- .../WebContent/ConnectionFromClient.h | 2 +- Userland/Services/WebContent/PageClient.cpp | 6 +- Userland/Services/WebContent/PageClient.h | 2 +- .../Services/WebContent/WebContentServer.ipc | 2 +- Userland/Utilities/headless-browser.cpp | 21 ++----- 30 files changed, 147 insertions(+), 186 deletions(-) diff --git a/Ladybird/AppKit/UI/LadybirdWebView.mm b/Ladybird/AppKit/UI/LadybirdWebView.mm index 613444caaed..f955c792385 100644 --- a/Ladybird/AppKit/UI/LadybirdWebView.mm +++ b/Ladybird/AppKit/UI/LadybirdWebView.mm @@ -351,22 +351,6 @@ static void copy_data_to_clipboard(StringView data, NSPasteboardType pasteboard_ self.event_being_redispatched = nil; }; - 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); - - ns_position.x = max(ns_position.x, document_rect.origin.x); - ns_position.x = min(ns_position.x, document_rect.size.width - content_rect.size.width); - - ns_position.y = max(ns_position.y, document_rect.origin.y); - ns_position.y = min(ns_position.y, document_rect.size.height - content_rect.size.height); - - [self scrollToPoint:ns_position]; - [[self scrollView] reflectScrolledClipView:self]; - [self updateViewportRect:Ladybird::WebViewBridge::ForResize::No]; - }; - m_web_view_bridge->on_cursor_change = [self](auto cursor) { if (cursor == Gfx::StandardCursor::Hidden) { if (!m_hidden_cursor.has_value()) { diff --git a/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp b/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp index 20e46dc4dc3..c02e39876b3 100644 --- a/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp +++ b/Ladybird/AppKit/UI/LadybirdWebViewBridge.cpp @@ -35,20 +35,6 @@ WebViewBridge::WebViewBridge(Vector screen_rects, float de , m_preferred_color_scheme(preferred_color_scheme) { m_device_pixel_ratio = device_pixel_ratio; - - on_scroll_by_delta = [this](auto x_delta, auto y_delta) { - auto position = m_viewport_rect.location(); - position.set_x(position.x() + x_delta); - position.set_y(position.y() + y_delta); - - 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; @@ -67,9 +53,9 @@ void WebViewBridge::set_system_visibility_state(bool is_visible) void WebViewBridge::set_viewport_rect(Gfx::IntRect viewport_rect, ForResize for_resize) { viewport_rect.set_size(scale_for_device(viewport_rect.size(), m_device_pixel_ratio)); - m_viewport_rect = viewport_rect; + m_viewport_size = viewport_rect.size(); - client().async_set_viewport_rect(m_client_state.page_index, m_viewport_rect.to_type()); + client().async_set_viewport_size(m_client_state.page_index, m_viewport_size.to_type()); if (for_resize == ForResize::Yes) { handle_resize(); @@ -126,9 +112,9 @@ void WebViewBridge::update_zoom() on_zoom_level_changed(); } -Web::DevicePixelRect WebViewBridge::viewport_rect() const +Web::DevicePixelSize WebViewBridge::viewport_size() const { - return m_viewport_rect.to_type(); + return m_viewport_size.to_type(); } Gfx::IntPoint WebViewBridge::to_content_position(Gfx::IntPoint widget_position) const diff --git a/Ladybird/AppKit/UI/LadybirdWebViewBridge.h b/Ladybird/AppKit/UI/LadybirdWebViewBridge.h index 5bcc2eb75f3..59c5b36ce61 100644 --- a/Ladybird/AppKit/UI/LadybirdWebViewBridge.h +++ b/Ladybird/AppKit/UI/LadybirdWebViewBridge.h @@ -53,18 +53,17 @@ public: Function()> on_request_web_content; Function on_zoom_level_changed; - Function on_scroll; private: WebViewBridge(Vector screen_rects, float device_pixel_ratio, WebContentOptions const&, Optional webdriver_content_ipc_path, Web::CSS::PreferredColorScheme); virtual void update_zoom() override; - virtual Web::DevicePixelRect viewport_rect() const override; + virtual Web::DevicePixelSize viewport_size() const override; virtual Gfx::IntPoint to_content_position(Gfx::IntPoint widget_position) const override; virtual Gfx::IntPoint to_widget_position(Gfx::IntPoint content_position) const override; Vector m_screen_rects; - Gfx::IntRect m_viewport_rect; + Gfx::IntSize m_viewport_size; WebContentOptions m_web_content_options; Optional m_webdriver_content_ipc_path; diff --git a/Ladybird/AppKit/UI/Tab.mm b/Ladybird/AppKit/UI/Tab.mm index ac12f0e8cde..e4043775052 100644 --- a/Ladybird/AppKit/UI/Tab.mm +++ b/Ladybird/AppKit/UI/Tab.mm @@ -91,8 +91,8 @@ static constexpr CGFloat const WINDOW_HEIGHT = 800; [self.search_panel setHidden:YES]; auto* scroll_view = [[NSScrollView alloc] init]; - [scroll_view setHasVerticalScroller:YES]; - [scroll_view setHasHorizontalScroller:YES]; + [scroll_view setHasVerticalScroller:NO]; + [scroll_view setHasHorizontalScroller:NO]; [scroll_view setLineScroll:24]; [scroll_view setContentView:self.web_view]; diff --git a/Ladybird/Qt/WebContentView.cpp b/Ladybird/Qt/WebContentView.cpp index f55181137cf..0ea923da150 100644 --- a/Ladybird/Qt/WebContentView.cpp +++ b/Ladybird/Qt/WebContentView.cpp @@ -55,6 +55,9 @@ WebContentView::WebContentView(QWidget* window, WebContentOptions const& web_con , m_web_content_options(web_content_options) , m_webdriver_content_ipc_path(webdriver_content_ipc_path) { + setVerticalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + setHorizontalScrollBarPolicy(Qt::ScrollBarAlwaysOff); + m_client_state.client = parent_client; m_client_state.page_index = page_index; @@ -68,13 +71,6 @@ WebContentView::WebContentView(QWidget* window, WebContentOptions const& web_con verticalScrollBar()->setSingleStep(24); horizontalScrollBar()->setSingleStep(24); - QObject::connect(verticalScrollBar(), &QScrollBar::valueChanged, [this](int) { - update_viewport_rect(); - }); - QObject::connect(horizontalScrollBar(), &QScrollBar::valueChanged, [this](int) { - update_viewport_rect(); - }); - QObject::connect(qGuiApp, &QGuiApplication::screenRemoved, [this](QScreen*) { update_screen_rects(); }); @@ -85,29 +81,10 @@ WebContentView::WebContentView(QWidget* window, WebContentOptions const& web_con initialize_client((parent_client == nullptr) ? CreateNewClient::Yes : CreateNewClient::No); - on_did_layout = [this](auto content_size) { - verticalScrollBar()->setMinimum(0); - verticalScrollBar()->setMaximum(content_size.height() - m_viewport_rect.height()); - verticalScrollBar()->setPageStep(m_viewport_rect.height()); - horizontalScrollBar()->setMinimum(0); - horizontalScrollBar()->setMaximum(content_size.width() - m_viewport_rect.width()); - horizontalScrollBar()->setPageStep(m_viewport_rect.width()); - }; - on_ready_to_paint = [this]() { viewport()->update(); }; - on_scroll_by_delta = [this](auto x_delta, auto y_delta) { - horizontalScrollBar()->setValue(max(0, horizontalScrollBar()->value() + x_delta)); - verticalScrollBar()->setValue(max(0, verticalScrollBar()->value() + y_delta)); - }; - - on_scroll_to_point = [this](auto position) { - horizontalScrollBar()->setValue(position.x()); - verticalScrollBar()->setValue(position.y()); - }; - on_cursor_change = [this](auto cursor) { update_cursor(cursor); }; @@ -445,14 +422,14 @@ void WebContentView::paintEvent(QPaintEvent*) void WebContentView::resizeEvent(QResizeEvent* event) { QAbstractScrollArea::resizeEvent(event); - update_viewport_rect(); + update_viewport_size(); handle_resize(); } void WebContentView::set_viewport_rect(Gfx::IntRect rect) { - m_viewport_rect = rect; - client().async_set_viewport_rect(m_client_state.page_index, rect.to_type()); + m_viewport_size = rect.size(); + client().async_set_viewport_size(m_client_state.page_index, rect.size().to_type()); } void WebContentView::set_window_size(Gfx::IntSize size) @@ -469,15 +446,15 @@ void WebContentView::set_device_pixel_ratio(double device_pixel_ratio) { m_device_pixel_ratio = device_pixel_ratio; client().async_set_device_pixels_per_css_pixel(m_client_state.page_index, m_device_pixel_ratio * m_zoom_level); - update_viewport_rect(); + update_viewport_size(); handle_resize(); } -void WebContentView::update_viewport_rect() +void WebContentView::update_viewport_size() { auto scaled_width = int(viewport()->width() * m_device_pixel_ratio); auto scaled_height = int(viewport()->height() * m_device_pixel_ratio); - Gfx::IntRect rect(max(0, horizontalScrollBar()->value()), max(0, verticalScrollBar()->value()), scaled_width, scaled_height); + Gfx::IntRect rect(0, 0, scaled_width, scaled_height); set_viewport_rect(rect); } @@ -485,7 +462,7 @@ void WebContentView::update_viewport_rect() void WebContentView::update_zoom() { client().async_set_device_pixels_per_css_pixel(m_client_state.page_index, m_device_pixel_ratio * m_zoom_level); - update_viewport_rect(); + update_viewport_size(); } void WebContentView::showEvent(QShowEvent* event) @@ -661,9 +638,9 @@ void WebContentView::update_cursor(Gfx::StandardCursor cursor) } } -Web::DevicePixelRect WebContentView::viewport_rect() const +Web::DevicePixelSize WebContentView::viewport_size() const { - return m_viewport_rect.to_type(); + return m_viewport_size.to_type(); } QPoint WebContentView::map_point_to_global_position(Gfx::IntPoint position) const @@ -673,12 +650,12 @@ QPoint WebContentView::map_point_to_global_position(Gfx::IntPoint position) cons Gfx::IntPoint WebContentView::to_content_position(Gfx::IntPoint widget_position) const { - return widget_position.translated(max(0, horizontalScrollBar()->value()), max(0, verticalScrollBar()->value())); + return widget_position; } Gfx::IntPoint WebContentView::to_widget_position(Gfx::IntPoint content_position) const { - return content_position.translated(-(max(0, horizontalScrollBar()->value())), -(max(0, verticalScrollBar()->value()))); + return content_position; } bool WebContentView::event(QEvent* event) @@ -715,7 +692,7 @@ ErrorOr WebContentView::dump_layout_tree() void WebContentView::enqueue_native_event(Web::MouseEvent::Type type, QSinglePointEvent const& event) { - auto position = to_content_position({ event.position().x() * m_device_pixel_ratio, event.position().y() * m_device_pixel_ratio }); + Web::DevicePixelPoint position = { event.position().x() * m_device_pixel_ratio, event.position().y() * m_device_pixel_ratio }; auto screen_position = Gfx::IntPoint { event.globalPosition().x() * m_device_pixel_ratio, event.globalPosition().y() * m_device_pixel_ratio }; auto button = get_button_from_qt_event(event); @@ -751,7 +728,7 @@ void WebContentView::enqueue_native_event(Web::MouseEvent::Type type, QSinglePoi } } - enqueue_input_event(Web::MouseEvent { type, position.to_type(), screen_position.to_type(), button, buttons, modifiers, wheel_delta_x, wheel_delta_y, nullptr }); + enqueue_input_event(Web::MouseEvent { type, position, screen_position.to_type(), button, buttons, modifiers, wheel_delta_x, wheel_delta_y, nullptr }); } struct KeyData : Web::ChromeInputData { diff --git a/Ladybird/Qt/WebContentView.h b/Ladybird/Qt/WebContentView.h index 70793c8f15a..30aef5d28fb 100644 --- a/Ladybird/Qt/WebContentView.h +++ b/Ladybird/Qt/WebContentView.h @@ -90,11 +90,11 @@ private: // ^WebView::ViewImplementation virtual void initialize_client(CreateNewClient) override; virtual void update_zoom() override; - virtual Web::DevicePixelRect viewport_rect() const override; + virtual Web::DevicePixelSize viewport_size() const override; virtual Gfx::IntPoint to_content_position(Gfx::IntPoint widget_position) const override; virtual Gfx::IntPoint to_widget_position(Gfx::IntPoint content_position) const override; - void update_viewport_rect(); + void update_viewport_size(); void update_cursor(Gfx::StandardCursor cursor); void enqueue_native_event(Web::MouseEvent::Type, QSinglePointEvent const& event); @@ -104,7 +104,7 @@ private: bool m_should_show_line_box_borders { false }; - Gfx::IntRect m_viewport_rect; + Gfx::IntSize m_viewport_size; WebContentOptions m_web_content_options; StringView m_webdriver_content_ipc_path; diff --git a/Tests/LibWeb/Ref/css-background-clip-text.html b/Tests/LibWeb/Ref/css-background-clip-text.html index a9c3c256eaa..9e4c860f9c8 100644 --- a/Tests/LibWeb/Ref/css-background-clip-text.html +++ b/Tests/LibWeb/Ref/css-background-clip-text.html @@ -7,6 +7,10 @@ Document