Auto merge of #29706 - mrobinson:temporary-fix-for-scroll-root, r=jdm

Fix scrolling on root element

eca0acf459 uncovered a bug in the way that the scrolling area of `window` was calculated and broke scrolling on the root element. This change does two things in order to fix that:

1. Does a partial revert of eca0acf459 in order to get scrolling from script working again on the window object.
2. Has the compositor always generate a frame for scrolls starting from script and waits for them. This is speculative fix for flakiness in root scrolling tests on CI.

The changes to the compositor expose some hidden failures, which is why some tests are marked as failing now.

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
This commit is contained in:
bors-servo 2023-05-07 19:04:53 +02:00 committed by GitHub
commit 01f8121642
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 55 additions and 20 deletions

View file

@ -490,7 +490,8 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
self.ready_to_save_state, self.ready_to_save_state,
ReadyState::WaitingForConstellationReply ReadyState::WaitingForConstellationReply
); );
if is_ready && !self.waiting_on_pending_frame { if is_ready && !self.waiting_on_pending_frame && !self.waiting_for_results_of_scroll
{
self.ready_to_save_state = ReadyState::ReadyToSaveImage; self.ready_to_save_state = ReadyState::ReadyToSaveImage;
if self.is_running_problem_test { if self.is_running_problem_test {
println!("ready to save image!"); println!("ready to save image!");
@ -623,8 +624,11 @@ impl<Window: WindowMethods + ?Sized> IOCompositor<Window> {
scroll_id, scroll_id,
clamping, clamping,
)) => { )) => {
self.waiting_for_results_of_scroll = true;
let mut txn = webrender_api::Transaction::new(); let mut txn = webrender_api::Transaction::new();
txn.scroll_node_with_id(point, scroll_id, clamping); txn.scroll_node_with_id(point, scroll_id, clamping);
txn.generate_frame();
self.webrender_api self.webrender_api
.send_transaction(self.webrender_document, txn); .send_transaction(self.webrender_document, txn);
}, },

View file

@ -1708,13 +1708,18 @@ impl Window {
let body = self.Document().GetBody(); let body = self.Document().GetBody();
let (x, y) = match body { let (x, y) = match body {
Some(e) => { Some(e) => {
let scroll_area = e.upcast::<Node>().scroll_area(); // This doesn't properly take into account the overflow set on <body>
// and the root element, which might affect how much the root can
// scroll. That requires properly handling propagating those values
// according to the rules defined in in the specification at:
// https://w3c.github.io/csswg-drafts/css-overflow/#overflow-propagation
let scroll_area = e.upcast::<Node>().bounding_content_box_or_zero();
( (
xfinite xfinite
.min(scroll_area.width() as f64 - viewport.width as f64) .min(scroll_area.width().to_f64_px() - viewport.width as f64)
.max(0.0f64), .max(0.0f64),
yfinite yfinite
.min(scroll_area.height() as f64 - viewport.height as f64) .min(scroll_area.height().to_f64_px() - viewport.height as f64)
.max(0.0f64), .max(0.0f64),
) )
}, },

View file

@ -1,2 +0,0 @@
[long_scroll_composited.html]
expected: FAIL

View file

@ -34,3 +34,21 @@
[scrollLeft/scrollTop on the HTML body element in non-quirks mode] [scrollLeft/scrollTop on the HTML body element in non-quirks mode]
expected: FAIL 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
[scroll() on the HTML body element in quirks mode]
expected: FAIL
[scrollBy() on the HTML body element in quirks mode]
expected: FAIL
[scrollLeft/scrollTop on the HTML body element in quirks mode]
expected: FAIL

View file

@ -1,2 +0,0 @@
[backdrop-filter-edge-behavior.html]
expected: FAIL

View file

@ -0,0 +1,2 @@
[filtered-html-is-not-container.html]
expected: FAIL

View file

@ -0,0 +1,2 @@
[filtered-inline-is-container.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[position-sticky-bottom-002.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[position-sticky-flexbox.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[position-sticky-rendering.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[position-sticky-top-002.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[long_scroll_composited.html]
expected: FAIL

View file

@ -34,3 +34,21 @@
[scrollLeft/scrollRight of the content in non-quirks mode] [scrollLeft/scrollRight of the content in non-quirks mode]
expected: FAIL 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
[scroll() on the HTML body element in quirks mode]
expected: FAIL
[scrollBy() on the HTML body element in quirks mode]
expected: FAIL
[scrollLeft/scrollTop on the HTML body element in quirks mode]
expected: FAIL

View file

@ -0,0 +1,2 @@
[filtered-inline-is-container.html]
expected: FAIL

View file

@ -1,2 +0,0 @@
[scroll_root.html]
expected: FAIL