[wpt-sync] Sync PR 16714 - Correctly handle scroll-snap-type changes to 'none'
Categories
(Core :: CSS Parsing and Computation, defect, P4)
Tracking
()
Tracking | Status | |
---|---|---|
firefox69 | --- | fixed |
People
(Reporter: mozilla.org, Unassigned)
References
()
Details
(Whiteboard: [wptsync downstream])
Sync web-platform-tests PR 16714 into mozilla-central (this bug is closed when the sync is complete).
PR: https://github.com/web-platform-tests/wpt/pull/16714
Details from upstream follow.
Majid Valipour <majidvp@chromium.org> wrote:
Correctly handle scroll-snap-type changes to 'none'
Previously when a scroll container's snap type is changed to 'none' its
data was discarded including all of its snap areas. However this is
incorrect. Because while the snap type is 'none', the element is still
a scroll container which per spec [1] means that is should continue to
captures the snap areas in its subtree for whom it is the nearest
ancestor scroll container . The only difference is that it no longer
snaps.The fix is that we no longer remove the snap container data just
because is has a 'none' snap type and instead keep it and its snap
areas. But we check the snap type before performing any snap.To ensure this does not introduce any performance regression, this CL
also includes an optimization where we avoid re-calculating
snap_container_data when the snap type is 'none'. So keeping these snap
data should not be cheap.Note that there is another problem where if the current snap container
is no longer a scroll container (e.g., overflow: scroll => overflow:
visible) we release its snap areas and they become "orphan". But if we
are to do this correctly, we should re-assign these areas to the next
stroller in the chain. Similarly when an element becomes a scroll
container, it can potentially take over snap areas from its parent snap
container.This patch does not address that situation yet but fixes the easier
problem.[1] https://drafts.csswg.org/css-scroll-snap/#overview
Bug: 953575
Test:
- wpt/css/css-scroll-snap/scroll-snap-type-change.html => Changing snap-type should work correctly
- wpt/css/css-scroll-snap/scroll-snap-type.html => Add a specific test for type 'none' to ensure it does not snap
Change-Id: Ie493ad68ecba818ed41c0ee103ccf44725ff6e3f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1589899
Reviewed-by: Majid Valipour \<majidvp@chromium.org>
Reviewed-by: David Bokan \<bokan@chromium.org>
Commit-Queue: Majid Valipour \<majidvp@chromium.org>
Cr-Commit-Position: refs/heads/master@{#657460}
Assignee | ||
Updated•5 years ago
|
Assignee | ||
Comment 1•5 years ago
|
||
Pushed to try https://treeherder.mozilla.org/#/jobs?repo=try&revision=498151510a42bdfa16671f1dee4095aba40551bf
Assignee | ||
Comment 2•5 years ago
|
||
Pushed to try (stability) https://treeherder.mozilla.org/#/jobs?repo=try&revision=1a2e494dff0bb27d030c2deffbd7925194a1d876
Assignee | ||
Comment 3•5 years ago
|
||
Failed to get results from try push
Pushed by james@hoppipolla.co.uk: https://hg.mozilla.org/integration/mozilla-inbound/rev/e3e0438fa26b [wpt PR 16714] - Correctly handle scroll-snap-type changes to 'none', a=testonly
Comment 5•5 years ago
|
||
bugherder |
Description
•