From e8e6dbcee061d0b4605b31674c96f0f79155c2da Mon Sep 17 00:00:00 2001 From: Jelle Raaijmakers Date: Tue, 17 Jun 2025 16:55:24 +0200 Subject: [PATCH] LibWeb: Fix document element's `.scrollHeight` and `.scrollWidth` We were using the viewport's size as the viewport scrolling area, but those are completely different things. --- Libraries/LibWeb/DOM/Element.cpp | 30 ++++++++++--------- Libraries/LibWeb/DOM/Element.h | 4 +-- ...ment-documentElement-scrollWidthHeight.txt | 2 ++ ...ll-height.txt => Element-scrollHeight.txt} | 0 ...roll-width.txt => Element-scrollWidth.txt} | 0 ...ent-documentElement-scrollWidthHeight.html | 16 ++++++++++ ...-height.html => Element-scrollHeight.html} | 0 ...ll-width.html => Element-scrollWidth.html} | 0 8 files changed, 36 insertions(+), 16 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/DOM/Element-documentElement-scrollWidthHeight.txt rename Tests/LibWeb/Text/expected/DOM/{element-scroll-height.txt => Element-scrollHeight.txt} (100%) rename Tests/LibWeb/Text/expected/DOM/{element-scroll-width.txt => Element-scrollWidth.txt} (100%) create mode 100644 Tests/LibWeb/Text/input/DOM/Element-documentElement-scrollWidthHeight.html rename Tests/LibWeb/Text/input/DOM/{element-scroll-height.html => Element-scrollHeight.html} (100%) rename Tests/LibWeb/Text/input/DOM/{element-scroll-width.html => Element-scrollWidth.html} (100%) diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index 8985d2f2659..02953b4a88f 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -1791,7 +1791,7 @@ void Element::set_scroll_top(double y) } // https://drafts.csswg.org/cssom-view/#dom-element-scrollwidth -int Element::scroll_width() const +int Element::scroll_width() { // 1. Let document be the element’s node document. auto& document = this->document(); @@ -1800,23 +1800,24 @@ int Element::scroll_width() const if (!document.is_active()) return 0; + // NOTE: Ensure that layout is up-to-date before looking at metrics. + document.update_layout(UpdateLayoutReason::ElementScrollWidth); + VERIFY(document.paintable_box() && document.paintable()->scrollable_overflow_rect().has_value()); + // 3. Let viewport width be the width of the viewport excluding the width of the scroll bar, if any, // or zero if there is no viewport. auto viewport_width = document.viewport_rect().width().to_int(); - auto viewport_scroll_width = document.navigable()->viewport_size().width().to_int(); + auto viewport_scrolling_area_width = document.paintable()->scrollable_overflow_rect()->width().to_int(); // 4. If the element is the root element and document is not in quirks mode // return max(viewport scrolling area width, viewport width). if (document.document_element() == this && !document.in_quirks_mode()) - return max(viewport_scroll_width, viewport_width); - - // NOTE: Ensure that layout is up-to-date before looking at metrics. - const_cast(document).update_layout(UpdateLayoutReason::ElementScrollWidth); + return max(viewport_scrolling_area_width, viewport_width); // 5. If the element is the body element, document is in quirks mode and the element is not potentially scrollable, // return max(viewport scrolling area width, viewport width). if (document.body() == this && document.in_quirks_mode() && !is_potentially_scrollable()) - return max(viewport_scroll_width, viewport_width); + return max(viewport_scrolling_area_width, viewport_width); // 6. If the element does not have any associated box return zero and terminate these steps. if (!paintable_box()) @@ -1830,7 +1831,7 @@ int Element::scroll_width() const } // https://drafts.csswg.org/cssom-view/#dom-element-scrollheight -int Element::scroll_height() const +int Element::scroll_height() { // 1. Let document be the element’s node document. auto& document = this->document(); @@ -1839,23 +1840,24 @@ int Element::scroll_height() const if (!document.is_active()) return 0; + // NOTE: Ensure that layout is up-to-date before looking at metrics. + document.update_layout(UpdateLayoutReason::ElementScrollHeight); + VERIFY(document.paintable_box() && document.paintable()->scrollable_overflow_rect().has_value()); + // 3. Let viewport height be the height of the viewport excluding the height of the scroll bar, if any, // or zero if there is no viewport. auto viewport_height = document.viewport_rect().height().to_int(); - auto viewport_scroll_height = document.navigable()->viewport_size().height().to_int(); + auto viewport_scrolling_area_height = document.paintable()->scrollable_overflow_rect()->height().to_int(); // 4. If the element is the root element and document is not in quirks mode // return max(viewport scrolling area height, viewport height). if (document.document_element() == this && !document.in_quirks_mode()) - return max(viewport_scroll_height, viewport_height); - - // NOTE: Ensure that layout is up-to-date before looking at metrics. - const_cast(document).update_layout(UpdateLayoutReason::ElementScrollHeight); + return max(viewport_scrolling_area_height, viewport_height); // 5. If the element is the body element, document is in quirks mode and the element is not potentially scrollable, // return max(viewport scrolling area height, viewport height). if (document.body() == this && document.in_quirks_mode() && !is_potentially_scrollable()) - return max(viewport_scroll_height, viewport_height); + return max(viewport_scrolling_area_height, viewport_height); // 6. If the element does not have any associated box return zero and terminate these steps. if (!paintable_box()) diff --git a/Libraries/LibWeb/DOM/Element.h b/Libraries/LibWeb/DOM/Element.h index 44cc6efc0d7..5304bc79334 100644 --- a/Libraries/LibWeb/DOM/Element.h +++ b/Libraries/LibWeb/DOM/Element.h @@ -298,8 +298,8 @@ public: void set_scroll_top(double y); void set_scroll_left(double x); - int scroll_width() const; - int scroll_height() const; + int scroll_width(); + int scroll_height(); bool is_actually_disabled() const; diff --git a/Tests/LibWeb/Text/expected/DOM/Element-documentElement-scrollWidthHeight.txt b/Tests/LibWeb/Text/expected/DOM/Element-documentElement-scrollWidthHeight.txt new file mode 100644 index 00000000000..a30b6388b36 --- /dev/null +++ b/Tests/LibWeb/Text/expected/DOM/Element-documentElement-scrollWidthHeight.txt @@ -0,0 +1,2 @@ +document.documentElement.scrollWidth: 1242 +document.documentElement.scrollHeight: 1481 diff --git a/Tests/LibWeb/Text/expected/DOM/element-scroll-height.txt b/Tests/LibWeb/Text/expected/DOM/Element-scrollHeight.txt similarity index 100% rename from Tests/LibWeb/Text/expected/DOM/element-scroll-height.txt rename to Tests/LibWeb/Text/expected/DOM/Element-scrollHeight.txt diff --git a/Tests/LibWeb/Text/expected/DOM/element-scroll-width.txt b/Tests/LibWeb/Text/expected/DOM/Element-scrollWidth.txt similarity index 100% rename from Tests/LibWeb/Text/expected/DOM/element-scroll-width.txt rename to Tests/LibWeb/Text/expected/DOM/Element-scrollWidth.txt diff --git a/Tests/LibWeb/Text/input/DOM/Element-documentElement-scrollWidthHeight.html b/Tests/LibWeb/Text/input/DOM/Element-documentElement-scrollWidthHeight.html new file mode 100644 index 00000000000..9fea28af678 --- /dev/null +++ b/Tests/LibWeb/Text/input/DOM/Element-documentElement-scrollWidthHeight.html @@ -0,0 +1,16 @@ + + +
+ + diff --git a/Tests/LibWeb/Text/input/DOM/element-scroll-height.html b/Tests/LibWeb/Text/input/DOM/Element-scrollHeight.html similarity index 100% rename from Tests/LibWeb/Text/input/DOM/element-scroll-height.html rename to Tests/LibWeb/Text/input/DOM/Element-scrollHeight.html diff --git a/Tests/LibWeb/Text/input/DOM/element-scroll-width.html b/Tests/LibWeb/Text/input/DOM/Element-scrollWidth.html similarity index 100% rename from Tests/LibWeb/Text/input/DOM/element-scroll-width.html rename to Tests/LibWeb/Text/input/DOM/Element-scrollWidth.html