From eca0acf4598c173c71765b961bd2079c0bb48cd2 Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Tue, 25 Apr 2023 09:07:53 +0200 Subject: [PATCH] Allow script to scroll `overflow: scroll` elements Before the code was only allowing `overflow: hidden` elements to scroll. This fixes that issue and also clean up the code that deals with detecting whether the body is a "potentially scrollable" in quirks mode. --- components/script/dom/element.rs | 80 +++++++++++-------- components/script/dom/window.rs | 26 +++--- .../css/css-ui/text-overflow-021.html.ini | 2 + ...chment-fixed-during-smooth-scroll.html.ini | 2 - ...round-change-during-smooth-scroll.html.ini | 4 - .../css/cssom-view/elementScroll-002.html.ini | 3 - .../scroll-behavior-default-css.html.ini | 3 - .../scroll-behavior-element.html.ini | 45 ----------- .../scroll-behavior-smooth-positions.html.ini | 54 ------------- .../scrolling-quirks-vs-nonquirks.html.ini | 10 --- .../fieldset-overflow-cssomview.html.ini | 4 - 11 files changed, 60 insertions(+), 173 deletions(-) create mode 100644 tests/wpt/metadata/css/css-ui/text-overflow-021.html.ini delete mode 100644 tests/wpt/metadata/css/cssom-view/add-background-attachment-fixed-during-smooth-scroll.html.ini delete mode 100644 tests/wpt/metadata/css/cssom-view/background-change-during-smooth-scroll.html.ini delete mode 100644 tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-overflow-cssomview.html.ini diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index e69fd558f5c..179cb169c46 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -120,7 +120,6 @@ use style::invalidation::element::restyle_hints::RestyleHint; use style::properties::longhands::{ self, background_image, border_spacing, font_family, font_size, }; -use style::properties::longhands::{overflow_x, overflow_y}; use style::properties::{parse_style_attribute, PropertyDeclarationBlock}; use style::properties::{ComputedValues, Importance, PropertyDeclaration}; use style::rule_tree::CascadeLevel; @@ -408,46 +407,59 @@ impl Element { } // https://drafts.csswg.org/cssom-view/#potentially-scrollable - fn potentially_scrollable(&self) -> bool { - self.has_css_layout_box() && !self.has_any_visible_overflow() + fn is_potentially_scrollable_body(&self) -> bool { + let node = self.upcast::(); + debug_assert!( + node.owner_doc().GetBody().as_deref() == self.downcast::(), + "Called is_potentially_scrollable_body on element that is not the " + ); + + // "An element body (which will be the body element) is potentially + // scrollable if all of the following conditions are true: + // - body has an associated box." + if !self.has_css_layout_box() { + return false; + } + + // " - body’s parent element’s computed value of the overflow-x or + // overflow-y properties is neither visible nor clip." + if let Some(parent) = node.GetParentElement() { + if let Some(style) = parent.style() { + if !style.get_box().clone_overflow_x().is_scrollable() && + !style.get_box().clone_overflow_y().is_scrollable() + { + return false; + } + }; + } + + // " - body’s computed value of the overflow-x or overflow-y properties + // is neither visible nor clip." + if let Some(style) = self.style() { + if !style.get_box().clone_overflow_x().is_scrollable() && + !style.get_box().clone_overflow_y().is_scrollable() + { + return false; + } + }; + + true } // https://drafts.csswg.org/cssom-view/#scrolling-box fn has_scrolling_box(&self) -> bool { // TODO: scrolling mechanism, such as scrollbar (We don't have scrollbar yet) // self.has_scrolling_mechanism() - self.has_any_hidden_overflow() + self.style().map_or(false, |style| { + style.get_box().clone_overflow_x().is_scrollable() || + style.get_box().clone_overflow_y().is_scrollable() + }) } fn has_overflow(&self) -> bool { self.ScrollHeight() > self.ClientHeight() || self.ScrollWidth() > self.ClientWidth() } - // TODO: Once #19183 is closed (overflow-x/y types moved out of mako), then we could implement - // a more generic `fn has_some_overflow(&self, overflow: Overflow)` rather than have - // these two `has_any_{visible,hidden}_overflow` methods which are very structurally - // similar. - - /// Computed value of overflow-x or overflow-y is "visible" - fn has_any_visible_overflow(&self) -> bool { - self.style().map_or(false, |s| { - let box_ = s.get_box(); - - box_.clone_overflow_x() == overflow_x::computed_value::T::Visible || - box_.clone_overflow_y() == overflow_y::computed_value::T::Visible - }) - } - - /// Computed value of overflow-x or overflow-y is "hidden" - fn has_any_hidden_overflow(&self) -> bool { - self.style().map_or(false, |s| { - let box_ = s.get_box(); - - box_.clone_overflow_x() == overflow_x::computed_value::T::Hidden || - box_.clone_overflow_y() == overflow_y::computed_value::T::Hidden - }) - } - fn shadow_root(&self) -> Option> { self.rare_data() .as_ref()? @@ -1765,7 +1777,7 @@ impl Element { // Step 9 if doc.GetBody().as_deref() == self.downcast::() && doc.quirks_mode() == QuirksMode::Quirks && - !self.potentially_scrollable() + !self.is_potentially_scrollable_body() { win.scroll(x, y, behavior); return; @@ -2305,7 +2317,7 @@ impl ElementMethods for Element { // Step 7 if doc.GetBody().as_deref() == self.downcast::() && doc.quirks_mode() == QuirksMode::Quirks && - !self.potentially_scrollable() + !self.is_potentially_scrollable_body() { return win.ScrollY() as f64; } @@ -2355,7 +2367,7 @@ impl ElementMethods for Element { // Step 9 if doc.GetBody().as_deref() == self.downcast::() && doc.quirks_mode() == QuirksMode::Quirks && - !self.potentially_scrollable() + !self.is_potentially_scrollable_body() { win.scroll(win.ScrollX() as f64, y, behavior); return; @@ -2401,7 +2413,7 @@ impl ElementMethods for Element { // Step 7 if doc.GetBody().as_deref() == self.downcast::() && doc.quirks_mode() == QuirksMode::Quirks && - !self.potentially_scrollable() + !self.is_potentially_scrollable_body() { return win.ScrollX() as f64; } @@ -2452,7 +2464,7 @@ impl ElementMethods for Element { // Step 9 if doc.GetBody().as_deref() == self.downcast::() && doc.quirks_mode() == QuirksMode::Quirks && - !self.potentially_scrollable() + !self.is_potentially_scrollable_body() { win.scroll(x, win.ScrollY() as f64, behavior); return; diff --git a/components/script/dom/window.rs b/components/script/dom/window.rs index 5acf727d35a..2364c1c7f72 100644 --- a/components/script/dom/window.rs +++ b/components/script/dom/window.rs @@ -1699,24 +1699,23 @@ impl Window { // TODO Step 4 - determine if a window has a viewport - // Step 5 - //TODO remove scrollbar width - let width = self.InnerWidth() as f64; - // Step 6 - //TODO remove scrollbar height - let height = self.InnerHeight() as f64; + // Step 5 & 6 + // TODO: Remove scrollbar dimensions. + let viewport = self.window_size.get().initial_viewport; // Step 7 & 8 - //TODO use overflow direction + // TODO: Consider `block-end` and `inline-end` overflow direction. let body = self.Document().GetBody(); let (x, y) = match body { Some(e) => { - let content_size = e.upcast::().bounding_content_box_or_zero(); - let content_height = content_size.size.height.to_f64_px(); - let content_width = content_size.size.width.to_f64_px(); + let scroll_area = e.upcast::().scroll_area(); ( - xfinite.min(content_width - width).max(0.0f64), - yfinite.min(content_height - height).max(0.0f64), + xfinite + .min(scroll_area.width() as f64 - viewport.width as f64) + .max(0.0f64), + yfinite + .min(scroll_area.height() as f64 - viewport.height as f64) + .max(0.0f64), ) }, None => (xfinite.max(0.0f64), yfinite.max(0.0f64)), @@ -1731,14 +1730,13 @@ impl Window { //TODO Step 11 //let document = self.Document(); // Step 12 - let global_scope = self.upcast::(); let x = x.to_f32().unwrap_or(0.0f32); let y = y.to_f32().unwrap_or(0.0f32); self.update_viewport_for_scroll(x, y); self.perform_a_scroll( x, y, - global_scope.pipeline_id().root_scroll_id(), + self.upcast::().pipeline_id().root_scroll_id(), behavior, None, ); diff --git a/tests/wpt/metadata/css/css-ui/text-overflow-021.html.ini b/tests/wpt/metadata/css/css-ui/text-overflow-021.html.ini new file mode 100644 index 00000000000..bc6acd4a26f --- /dev/null +++ b/tests/wpt/metadata/css/css-ui/text-overflow-021.html.ini @@ -0,0 +1,2 @@ +[text-overflow-021.html] + expected: FAIL diff --git a/tests/wpt/metadata/css/cssom-view/add-background-attachment-fixed-during-smooth-scroll.html.ini b/tests/wpt/metadata/css/cssom-view/add-background-attachment-fixed-during-smooth-scroll.html.ini deleted file mode 100644 index 1b477fbb9d1..00000000000 --- a/tests/wpt/metadata/css/cssom-view/add-background-attachment-fixed-during-smooth-scroll.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[add-background-attachment-fixed-during-smooth-scroll.html] - expected: TIMEOUT diff --git a/tests/wpt/metadata/css/cssom-view/background-change-during-smooth-scroll.html.ini b/tests/wpt/metadata/css/cssom-view/background-change-during-smooth-scroll.html.ini deleted file mode 100644 index ea00ae5f7e3..00000000000 --- a/tests/wpt/metadata/css/cssom-view/background-change-during-smooth-scroll.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[background-change-during-smooth-scroll.html] - expected: TIMEOUT - [background change during smooth scroll] - expected: NOTRUN diff --git a/tests/wpt/metadata/css/cssom-view/elementScroll-002.html.ini b/tests/wpt/metadata/css/cssom-view/elementScroll-002.html.ini index 5f07b905cfb..a2d32c314ea 100644 --- a/tests/wpt/metadata/css/cssom-view/elementScroll-002.html.ini +++ b/tests/wpt/metadata/css/cssom-view/elementScroll-002.html.ini @@ -1,6 +1,3 @@ [elementScroll-002.html] [simple scroll with style: 'margin' and 'overflow: scroll'] expected: FAIL - - [simple scroll with style: 'padding' and 'overflow: scroll'] - expected: FAIL diff --git a/tests/wpt/metadata/css/cssom-view/scroll-behavior-default-css.html.ini b/tests/wpt/metadata/css/cssom-view/scroll-behavior-default-css.html.ini index b084797a43f..38761f42590 100644 --- a/tests/wpt/metadata/css/cssom-view/scroll-behavior-default-css.html.ini +++ b/tests/wpt/metadata/css/cssom-view/scroll-behavior-default-css.html.ini @@ -4,6 +4,3 @@ [Smooth scrolling of an element with default scroll-behavior] expected: FAIL - - [Instant scrolling of an element with default scroll-behavior] - expected: FAIL diff --git a/tests/wpt/metadata/css/cssom-view/scroll-behavior-element.html.ini b/tests/wpt/metadata/css/cssom-view/scroll-behavior-element.html.ini index e477e663110..885aae83604 100644 --- a/tests/wpt/metadata/css/cssom-view/scroll-behavior-element.html.ini +++ b/tests/wpt/metadata/css/cssom-view/scroll-behavior-element.html.ini @@ -67,48 +67,3 @@ [Set scrollTop to element with smooth scroll-behavior] expected: FAIL - - [Element with auto scroll-behavior ; scroll() with default behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scroll() with auto behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scroll() with instant behavior] - expected: FAIL - - [Element with smooth scroll-behavior ; scroll() with instant behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollTo() with default behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollTo() with auto behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollTo() with instant behavior] - expected: FAIL - - [Element with smooth scroll-behavior ; scrollTo() with instant behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollBy() with default behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollBy() with auto behavior] - expected: FAIL - - [Element with auto scroll-behavior ; scrollBy() with instant behavior] - expected: FAIL - - [Element with smooth scroll-behavior ; scrollBy() with instant behavior] - expected: FAIL - - [Set scrollLeft to element with auto scroll-behavior] - expected: FAIL - - [Set scrollTop to element with auto scroll-behavior] - expected: FAIL - - [Aborting an ongoing smooth scrolling on an element with another smooth scrolling] - expected: FAIL diff --git a/tests/wpt/metadata/css/cssom-view/scroll-behavior-smooth-positions.html.ini b/tests/wpt/metadata/css/cssom-view/scroll-behavior-smooth-positions.html.ini index c436513c82b..ecf3c45a692 100644 --- a/tests/wpt/metadata/css/cssom-view/scroll-behavior-smooth-positions.html.ini +++ b/tests/wpt/metadata/css/cssom-view/scroll-behavior-smooth-positions.html.ini @@ -10,57 +10,3 @@ [Scroll positions when performing smooth scrolling from (0, 500) to (500, 250) using scrollIntoView() ] expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 0) to (500, 250) using scroll() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 0) to (500, 250) using scroll() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 500) to (500, 250) using scroll() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 500) to (500, 250) using scroll() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 0) to (500, 250) using scrollTo() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 0) to (500, 250) using scrollTo() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 500) to (500, 250) using scrollTo() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 500) to (500, 250) using scrollTo() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 0) to (500, 250) using scrollBy() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 0) to (500, 250) using scrollBy() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (0, 500) to (500, 250) using scrollBy() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from (1000, 500) to (500, 250) using scrollBy() ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from 0 to 500 by setting scrollLeft ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from 1000 to 500 by setting scrollLeft ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from 0 to 250 by setting scrollTop ] - expected: FAIL - - [Scroll positions when performing smooth scrolling from 500 to 250 by setting scrollTop ] - expected: FAIL - - [Scroll positions when aborting a smooth scrolling with another smooth scrolling] - expected: FAIL - - [Scroll positions when aborting a smooth scrolling with an instant scrolling] - expected: FAIL diff --git a/tests/wpt/metadata/css/cssom-view/scrolling-quirks-vs-nonquirks.html.ini b/tests/wpt/metadata/css/cssom-view/scrolling-quirks-vs-nonquirks.html.ini index 40d0827a65f..b63dd7da4f6 100644 --- a/tests/wpt/metadata/css/cssom-view/scrolling-quirks-vs-nonquirks.html.ini +++ b/tests/wpt/metadata/css/cssom-view/scrolling-quirks-vs-nonquirks.html.ini @@ -17,15 +17,6 @@ [scrollingElement in non-quirks mode] expected: FAIL - [scroll() on the root element in non-quirks mode] - expected: FAIL - - [scrollBy() on the root element in non-quirks mode] - expected: FAIL - - [scrollLeft/scrollTop on the root element in non-quirks mode] - expected: FAIL - [scrollWidth/scrollHeight on the root element in non-quirks mode] expected: FAIL @@ -43,4 +34,3 @@ [scrollLeft/scrollRight of the content in non-quirks mode] expected: FAIL - diff --git a/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-overflow-cssomview.html.ini b/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-overflow-cssomview.html.ini deleted file mode 100644 index b49fbe68d59..00000000000 --- a/tests/wpt/metadata/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-overflow-cssomview.html.ini +++ /dev/null @@ -1,4 +0,0 @@ -[fieldset-overflow-cssomview.html] - [Test cssom-view API for FIELDSET] - expected: FAIL -