RESOLVED FIXED 210469
Scroll snap specified on :root doesn't work 
https://bugs.webkit.org/show_bug.cgi?id=210469
Summary Scroll snap specified on :root doesn't work
Simon Fraser (smfr)
Reported 2020-04-13 17:08:37 PDT
Created attachment 396361 [details] test This doesn't work: :root { scroll-snap-type: mandatory; margin: 20px; }
Attachments
test (592 bytes, text/html)
2020-04-13 17:08 PDT, Simon Fraser (smfr) 		no flags
Details
Work in progress (5.35 KB, patch)
2020-04-13 22:19 PDT, Simon Fraser (smfr) 		no flags
DetailsFormatted DiffDiff
Patch (41.46 KB, patch)
2020-11-05 11:14 PST, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Patch (41.53 KB, patch)
2020-11-05 11:16 PST, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Patch (41.51 KB, patch)
2020-11-06 01:01 PST, Martin Robinson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2020-04-13 17:09:10 PDT
<rdar://problem/61746676>
Simon Fraser (smfr)
Comment 2 2020-04-13 22:19:06 PDT
Created attachment 396380 [details] Work in progress
Simon Fraser (smfr)
Comment 3 2020-04-13 22:22:24 PDT
Cathie or Frederic, feel free to pick this up. I'm a bit concerned about ignoring scroll-snap style on the body for compat reasons (bug 200643), but I think it's important to make it work on :root { } and html { }. The element argument passed to updateSnapOffsetsForScrollableArea is pretty suspect, and we should certainly not be using the body's geometry. Tests should include an example where the body is independently scrollable from the document (overflow:scroll style on the body).
Martin Robinson
Comment 4 2020-11-05 11:14:33 PST
Created attachment 413329 [details] Patch
Martin Robinson
Comment 5 2020-11-05 11:16:10 PST
Created attachment 413330 [details] Patch
Simon Fraser (smfr)
Comment 6 2020-11-05 11:58:59 PST
Comment on attachment 413330 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=413330&action=review > Source/WebCore/page/scrolling/AxisScrollSnapOffsets.cpp:166 > + const HashSet<const RenderBox*>& boxesWithScrollSnapPositions = scrollingElementBox.view().boxesWithScrollSnapPositions(); auto > Source/WebCore/rendering/RenderLayer.cpp:3595 > + updateSnapOffsetsForScrollableArea(*this, *box, box->style(), box->paddingBoxRect()); Does paddingBoxRect() do the right thing if scrollbars are visible? (e.g. bottom snap with vertical and horizontal scrollbar)?
Martin Robinson
Comment 7 2020-11-06 00:58:54 PST
(In reply to Simon Fraser (smfr) from comment #6) Thanks for the review. > > Source/WebCore/rendering/RenderLayer.cpp:3595 > > + updateSnapOffsetsForScrollableArea(*this, *box, box->style(), box->paddingBoxRect()); > > Does paddingBoxRect() do the right thing if scrollbars are visible? (e.g. > bottom snap with vertical and horizontal scrollbar)? It seems like this is working properly.
Martin Robinson
Comment 8 2020-11-06 01:01:00 PST
Created attachment 413411 [details] Patch
EWS
Comment 9 2020-11-06 01:45:00 PST
Committed r269506: <https://trac.webkit.org/changeset/269506> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413411 [details].
Note You need to log in before you can comment on or make changes to this bug.