Add support for overflow:scroll and overflow:hidden to layout_2020 #25645

mrobinson · 2020-01-30T09:26:23Z

This adds clipping and interactive scrolling support, but scrolling from
script is still not functional.

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes fix #___ (GitHub issue number if applicable)

There are tests for these changes OR
These changes do not require tests because ___

highfive · 2020-01-30T09:26:29Z

Heads up! This PR modifies the following files:

@emilio: components/style/properties/longhands/box.mako.rs, components/style/properties/shorthands/box.mako.rs

jdm · 2020-01-30T09:56:09Z

r? @SimonSapin

SimonSapin · 2020-01-30T13:23:40Z

components/layout_2020/fragments.rs

@@ -180,10 +178,33 @@ impl BoxFragment {
            border,
            margin,
            block_margins_collapsed_with_children,
-            scrollable_overflow,
+            scrollable_overflow: PhysicalRect::zero(),


I’m not a fan of initializing here with known-incorrect data and rely on calculate_scrollable_overflow being called later at some point for its side effect. Why is that computation delayed? If it needs to be, what do you think of making this an Option and calling unwrap when the data is used, so we don’t accidentally use a non-initialized zero?

The issue is that the BoxFragment is created and then positioned later. Does it make sense to modify layout_block_level_children to create and place BoxFragments at the same time?

Creation is separated from placement in layout_block_level_children so that the former (including recursion in a subtree) can be parallelized across siblings, while the latter needs to be sequential.

What if we defined BoxFragment::scrollable_overflow to be in a coordinate system where self.content_rect.start_corner is the origin, and the parent is responsible for doing the translation?

I have done something like this in the latest branch. Now the final position of the scrollable overflow is calculated when it is used. This shouldn't have a large overhead. I've kept the calculation inside of the child since it knows about its position when the calculation is done.

SimonSapin · 2020-01-30T13:25:22Z

components/layout_2020/display_list/mod.rs

+            let overflow_x = self.fragment.style.get_box().overflow_x;
+            let overflow_y = self.fragment.style.get_box().overflow_y;
+            let original_scroll_and_clip_info = builder.current_space_and_clip;
+            if overflow_x != ComputedOverflow::Visible || overflow_y != ComputedOverflow::Visible {


This seems to treat all values other than visible the same. Landing things incrementally is fine, but what’s the plan for fixing that later?

The only difference here, I think, is with Overflow::Auto. As far as correctness goes, I think the unnecessary creation of the scroll frame doesn't affect it. Overflow::Hidden is handled below.

SimonSapin · 2020-01-31T11:47:04Z

Looks like this needs some code formatting changes, see "Details" near "checks have failed to find the CI log. You can check the same locally by running ./mach test-tidy, and in the case of Rust code formatting fix it automatically with rustfmt by running ./mach fmt.

(I have ./mach test-tidy --no-wpt in .git/hooks/pre-push because otherwise I keep forgetting.)

Feel free to r=SimonSapin with that fixed.

mrobinson · 2020-01-31T15:09:28Z

@bors-servo r=SimonSapin

bors-servo · 2020-01-31T15:09:29Z

📌 Commit e1027cd has been approved by SimonSapin

bors-servo · 2020-01-31T15:09:35Z

⌛ Testing commit e1027cd with merge 25b526f...


        Auto merge of #25645 - mrobinson:scrollable-overflow-v2, r=SimonSapin

Add support for overflow:scroll and overflow:hidden to layout_2020 This adds clipping and interactive scrolling support, but scrolling from script is still not functional.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-01-31T15:44:04Z

💔 Test failed - status-taskcluster

jdm · 2020-01-31T15:45:02Z

  ▶ PASS [expected FAIL] /css/css-backgrounds/background-size/background-size-contain.xht
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-size/background-size-cover.xht
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-repeat/background-repeat-repeat-y.xht
  ▶ PASS [expected FAIL] /css/css-backgrounds/background-repeat/background-repeat-repeat-x.xht
  ▶ PASS [expected FAIL] /_mozilla/css/acid2_ref.html


        Add support for overflow:scroll and overflow:hidden to layout_2020

This adds clipping and interactive scrolling support, but scrolling from script is still not functional.


        Auto merge of #25645 - mrobinson:scrollable-overflow-v2, r=SimonSapin

Add support for overflow:scroll and overflow:hidden to layout_2020 This adds clipping and interactive scrolling support, but scrolling from script is still not functional.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-02-01T05:53:38Z

💔 Test failed - status-taskcluster

jdm · 2020-02-01T09:13:53Z

@bors-servo retry

bors-servo · 2020-02-01T09:13:58Z

⌛ Testing commit 7a5a320 with merge 840605d...


        Auto merge of #25645 - mrobinson:scrollable-overflow-v2, r=SimonSapin

Add support for overflow:scroll and overflow:hidden to layout_2020 This adds clipping and interactive scrolling support, but scrolling from script is still not functional.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-02-01T09:59:54Z

💔 Test failed - status-taskcluster

jdm · 2020-02-01T10:07:54Z

@bors-servo retry
#24726

bors-servo · 2020-02-01T11:09:44Z

⌛ Testing commit 7a5a320 with merge fddeee8...


        Auto merge of #25645 - mrobinson:scrollable-overflow-v2, r=SimonSapin

Add support for overflow:scroll and overflow:hidden to layout_2020 This adds clipping and interactive scrolling support, but scrolling from script is still not functional.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-02-01T12:08:15Z

💔 Test failed - status-taskcluster

CYBAI · 2020-02-01T15:45:20Z

@bors-servo retry

#25513

bors-servo · 2020-02-01T15:45:24Z

⌛ Testing commit 7a5a320 with merge c4785e3...


        Auto merge of #25645 - mrobinson:scrollable-overflow-v2, r=SimonSapin

Add support for overflow:scroll and overflow:hidden to layout_2020 This adds clipping and interactive scrolling support, but scrolling from script is still not functional.  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes fix #___ (GitHub issue number if applicable)  - [x] There are tests for these changes OR - [ ] These changes do not require tests because ___

bors-servo · 2020-02-01T15:55:42Z

☀️ Test successful - status-taskcluster
Approved by: SimonSapin
Pushing c4785e3 to master...

highfive assigned jdm Jan 30, 2020

mrobinson requested a review from SimonSapin Jan 30, 2020

highfive added the S-awaiting-review label Jan 30, 2020

mrobinson added A-layout/2020 and removed S-awaiting-review labels Jan 30, 2020

highfive assigned SimonSapin and unassigned jdm Jan 30, 2020

SimonSapin reviewed Jan 30, 2020

View changes

mrobinson force-pushed the mrobinson:scrollable-overflow-v2 branch from 8f76b4f to d8cdd90 Jan 31, 2020

highfive added the S-awaiting-review label Jan 31, 2020

SimonSapin approved these changes Jan 31, 2020

View changes

mrobinson force-pushed the mrobinson:scrollable-overflow-v2 branch from d8cdd90 to e1027cd Jan 31, 2020

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 31, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 31, 2020

Add support for overflow:scroll and overflow:hidden to layout_2020

Loading status checks…

7a5a320

This adds clipping and interactive scrolling support, but scrolling from script is still not functional.

mrobinson force-pushed the mrobinson:scrollable-overflow-v2 branch from e1027cd to 7a5a320 Jan 31, 2020

highfive added S-awaiting-review and removed S-tests-failed labels Jan 31, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 1, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Feb 1, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 1, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Feb 1, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 1, 2020

highfive added S-tests-failed and removed S-awaiting-merge labels Feb 1, 2020

highfive added S-awaiting-merge and removed S-tests-failed labels Feb 1, 2020

bors-servo merged commit 7a5a320 into servo:master Feb 1, 2020
2 checks passed

2 checks passed

Community-TC (pull_request) TaskGroup: success
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Feb 1, 2020

servo / servo

Add support for overflow:scroll and overflow:hidden to layout_2020 #25645

Add support for overflow:scroll and overflow:hidden to layout_2020 #25645

mrobinson commented Jan 30, 2020

highfive commented Jan 30, 2020

jdm commented Jan 30, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

SimonSapin commented Jan 31, 2020

mrobinson commented Jan 31, 2020

bors-servo commented Jan 31, 2020

bors-servo commented Jan 31, 2020

bors-servo commented Jan 31, 2020

jdm commented Jan 31, 2020

bors-servo commented Feb 1, 2020

jdm commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020

jdm commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020

CYBAI commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020

servo / servo

Join GitHub today

Add support for overflow:scroll and overflow:hidden to layout_2020 #25645

Add support for overflow:scroll and overflow:hidden to layout_2020 #25645

Conversation

mrobinson commented Jan 30, 2020

highfive commented Jan 30, 2020

jdm commented Jan 30, 2020

This comment has been minimized.

SimonSapin Jan 30, 2020

This comment has been minimized.

mrobinson Jan 30, 2020

This comment has been minimized.

SimonSapin Jan 30, 2020

This comment has been minimized.

mrobinson Jan 31, 2020

This comment has been minimized.

SimonSapin Jan 30, 2020

This comment has been minimized.

mrobinson Jan 30, 2020

SimonSapin commented Jan 31, 2020

mrobinson commented Jan 31, 2020

bors-servo commented Jan 31, 2020

bors-servo commented Jan 31, 2020

bors-servo commented Jan 31, 2020

jdm commented Jan 31, 2020

bors-servo commented Feb 1, 2020

jdm commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020

jdm commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020

CYBAI commented Feb 1, 2020

bors-servo commented Feb 1, 2020

bors-servo commented Feb 1, 2020