From d96b28db131065a07802fa88376d5c77b3843a7e Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sat, 1 Jun 2024 17:30:24 +0300 Subject: [PATCH 1/6] LibWeb: Scroll into viewport from a task in set_focused_element() This is a hack needed to preserve current behaviour after making set viewport_rect() being not async in upcoming changes. For example both handle_mousedown and handle_mouseup should use the same viewport scroll offset even though handle_mousedown runs focusing steps that might cause scrolling to focused element: - handle_mousedown({ 0, 0 }) - run_focusing_steps() - set_focused_element() - scroll_into_viewport() changes viewport scroll offset - handle_mouseup({ 0, 0 }) (cherry picked from commit 50920b05953a6bc2aacb07d291d503052caadf15) --- Userland/Libraries/LibWeb/DOM/Document.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 074bb0f19e61cd..84b14910119c81 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1906,10 +1906,12 @@ void Document::set_focused_element(Element* element) // Scroll the viewport if necessary to make the newly focused element visible. if (m_focused_element) { - ScrollIntoViewOptions scroll_options; - scroll_options.block = Bindings::ScrollLogicalPosition::Nearest; - scroll_options.inline_ = Bindings::ScrollLogicalPosition::Nearest; - (void)m_focused_element->scroll_into_view(scroll_options); + m_focused_element->queue_an_element_task(HTML::Task::Source::UserInteraction, [&]() { + ScrollIntoViewOptions scroll_options; + scroll_options.block = Bindings::ScrollLogicalPosition::Nearest; + scroll_options.inline_ = Bindings::ScrollLogicalPosition::Nearest; + (void)m_focused_element->scroll_into_view(scroll_options); + }); } } From bae0f49d6fe89759dcef3803b8fe73a5dd07dbd1 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Sat, 1 Jun 2024 19:07:58 +0300 Subject: [PATCH 2/6] LibWeb: Propagate scrollbar-width property from root element to viewport (cherry picked from commit eb909118bfe9d939a1109823b6e0b8736abab138) --- Userland/Libraries/LibWeb/DOM/Document.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Userland/Libraries/LibWeb/DOM/Document.cpp b/Userland/Libraries/LibWeb/DOM/Document.cpp index 84b14910119c81..68fb3bf43f44d9 100644 --- a/Userland/Libraries/LibWeb/DOM/Document.cpp +++ b/Userland/Libraries/LibWeb/DOM/Document.cpp @@ -1026,6 +1026,15 @@ void Document::invalidate_layout() schedule_layout_update(); } +static void propagate_scrollbar_width_to_viewport(Element& root_element, Layout::Viewport& viewport) +{ + // https://drafts.csswg.org/css-scrollbars/#scrollbar-width + // UAs must apply the scrollbar-color value set on the root element to the viewport. + auto& viewport_computed_values = viewport.mutable_computed_values(); + auto& root_element_computed_values = root_element.layout_node()->computed_values(); + viewport_computed_values.set_scrollbar_width(root_element_computed_values.scrollbar_width()); +} + static void propagate_overflow_to_viewport(Element& root_element, Layout::Viewport& viewport) { // https://drafts.csswg.org/css-overflow-3/#overflow-propagation @@ -1090,6 +1099,7 @@ void Document::update_layout() if (document_element && document_element->layout_node()) { propagate_overflow_to_viewport(*document_element, *m_layout_root); + propagate_scrollbar_width_to_viewport(*document_element, *m_layout_root); } } From fed24df2598ee43c66d2c3896c5772f71c33bccf Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Mon, 3 Jun 2024 17:53:55 +0300 Subject: [PATCH 3/6] 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(). (cherry picked from commit 5285e22f2aa09152365179865f135e7bc5d254a5) --- 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 613444caaedd27..f955c7923852b1 100644 --- a/Ladybird/AppKit/UI/LadybirdWebView.mm +++ b/Ladybird/AppKit/UI/LadybirdWebView.mm @@ -351,22 +351,6 @@ - (void)setWebViewCallbacks 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 c7361929f6ee11..ab66a10afb2ce2 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 5bcc2eb75f396d..59c5b36ce6182d 100644 --- a/Ladybird/AppKit/UI/LadybirdWebViewBridge.h +++ b/Ladybird/AppKit/UI/LadybirdWebViewBridge.h @@ -53,18 +53,17 @@ class WebViewBridge final : public WebView::ViewImplementation { 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 ac12f0e8cde0a6..e404377505260f 100644 --- a/Ladybird/AppKit/UI/Tab.mm +++ b/Ladybird/AppKit/UI/Tab.mm @@ -91,8 +91,8 @@ - (instancetype)init [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 52a58eac2d79fd..3c7c49144eb509 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) @@ -662,9 +639,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 @@ -674,12 +651,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) @@ -716,7 +693,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); @@ -752,7 +729,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 70793c8f15a446..30aef5d28fb371 100644 --- a/Ladybird/Qt/WebContentView.h +++ b/Ladybird/Qt/WebContentView.h @@ -90,11 +90,11 @@ class WebContentView final // ^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 @@ class WebContentView final 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 a9c3c256eaab02..9e4c860f9c8626 100644 --- a/Tests/LibWeb/Ref/css-background-clip-text.html +++ b/Tests/LibWeb/Ref/css-background-clip-text.html @@ -7,6 +7,10 @@ Document