Created attachment 396361 [details] test This doesn't work: :root { scroll-snap-type: mandatory; margin: 20px; }
<rdar://problem/61746676>
Created attachment 396380 [details] Work in progress
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).
Created attachment 413329 [details] Patch
Created attachment 413330 [details] Patch
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)?
(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.
Created attachment 413411 [details] Patch
Committed r269506: <https://trac.webkit.org/changeset/269506> All reviewed patches have been landed. Closing bug and clearing flags on attachment 413411 [details].