diff --git a/gfx/layers/apz/test/gtest/TestSnapping.cpp b/gfx/layers/apz/test/gtest/TestSnapping.cpp index 1b1e09403d91..a6acc54f6748 100644 --- a/gfx/layers/apz/test/gtest/TestSnapping.cpp +++ b/gfx/layers/apz/test/gtest/TestSnapping.cpp @@ -29,11 +29,15 @@ TEST_F(APZCSnappingTesterMock, Bug1265510) { ScrollSnapInfo snap; snap.mScrollSnapStrictnessY = StyleScrollSnapStrictness::Mandatory; + snap.mSnapportSize = CSSSize::ToAppUnits( + layerVisibleRegion[0].GetBounds().Size() * LayerToCSSScale(1.0)); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(0 * AppUnitsPerCSSPixel()), nsRect())); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(100 * AppUnitsPerCSSPixel()), nsRect())); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(0 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 0, 10, 10)))); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(100 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 100, 10, 10)))); ModifyFrameMetrics(root, [&](ScrollMetadata& aSm, FrameMetrics&) { aSm.SetSnapInfo(ScrollSnapInfo(snap)); @@ -102,11 +106,15 @@ TEST_F(APZCSnappingTesterMock, Snap_After_Pinch) { // Set up some basic scroll snapping ScrollSnapInfo snap; snap.mScrollSnapStrictnessY = StyleScrollSnapStrictness::Mandatory; + snap.mSnapportSize = CSSSize::ToAppUnits( + layerVisibleRegion[0].GetBounds().Size() * LayerToCSSScale(1.0)); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(0 * AppUnitsPerCSSPixel()), nsRect())); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(100 * AppUnitsPerCSSPixel()), nsRect())); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(0 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 0, 10, 10)))); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(100 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 100, 10, 10)))); // Save the scroll snap info on the root APZC. // Also mark the root APZC as "root content", since APZC only allows @@ -156,10 +164,14 @@ TEST_F(APZCSnappingTesterMock, SnapOnPanEndWithZeroVelocity) { // Set up two snap points, 30 and 100. ScrollSnapInfo snap; snap.mScrollSnapStrictnessY = StyleScrollSnapStrictness::Mandatory; - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(30 * AppUnitsPerCSSPixel()), nsRect())); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(100 * AppUnitsPerCSSPixel()), nsRect())); + snap.mSnapportSize = CSSSize::ToAppUnits( + layerVisibleRegion[0].GetBounds().Size() * LayerToCSSScale(1.0)); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(30 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 30, 10, 10)))); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(100 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 100, 10, 10)))); // Save the scroll snap info on the root APZC. ModifyFrameMetrics(root, [&](ScrollMetadata& aSm, FrameMetrics& aMetrics) { @@ -227,10 +239,14 @@ TEST_F(APZCSnappingTesterMock, SnapOnPanEndWithPositiveVelocity) { // Set up two snap points, 30 and 100. ScrollSnapInfo snap; snap.mScrollSnapStrictnessY = StyleScrollSnapStrictness::Mandatory; - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(30 * AppUnitsPerCSSPixel()), nsRect())); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(100 * AppUnitsPerCSSPixel()), nsRect())); + snap.mSnapportSize = CSSSize::ToAppUnits( + layerVisibleRegion[0].GetBounds().Size() * LayerToCSSScale(1.0)); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(30 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 30, 10, 10)))); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(100 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 100, 10, 10)))); // Save the scroll snap info on the root APZC. ModifyFrameMetrics(root, [&](ScrollMetadata& aSm, FrameMetrics& aMetrics) { diff --git a/gfx/layers/apz/test/gtest/TestSnappingOnMomentum.cpp b/gfx/layers/apz/test/gtest/TestSnappingOnMomentum.cpp index 3a57732666e6..fc573443cdc7 100644 --- a/gfx/layers/apz/test/gtest/TestSnappingOnMomentum.cpp +++ b/gfx/layers/apz/test/gtest/TestSnappingOnMomentum.cpp @@ -27,11 +27,14 @@ TEST_F(APZCSnappingOnMomentumTesterMock, Snap_On_Momentum) { // Set up some basic scroll snapping ScrollSnapInfo snap; snap.mScrollSnapStrictnessY = StyleScrollSnapStrictness::Mandatory; - - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(0 * AppUnitsPerCSSPixel()), nsRect())); - snap.mSnapTargets.AppendElement(ScrollSnapInfo::SnapTarget( - Nothing(), Some(100 * AppUnitsPerCSSPixel()), nsRect())); + snap.mSnapportSize = CSSSize::ToAppUnits( + layerVisibleRegion[0].GetBounds().Size() * LayerToCSSScale(1.0)); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(0 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 0, 10, 10)))); + snap.mSnapTargets.AppendElement( + ScrollSnapInfo::SnapTarget(Nothing(), Some(100 * AppUnitsPerCSSPixel()), + CSSRect::ToAppUnits(CSSRect(0, 100, 10, 10)))); ModifyFrameMetrics(root, [&](ScrollMetadata& aSm, FrameMetrics&) { aSm.SetSnapInfo(ScrollSnapInfo(snap)); diff --git a/layout/generic/ScrollSnap.cpp b/layout/generic/ScrollSnap.cpp index de96c1261eb1..56fe41c45aac 100644 --- a/layout/generic/ScrollSnap.cpp +++ b/layout/generic/ScrollSnap.cpp @@ -39,6 +39,7 @@ class CalcSnapPoints final { return std::abs( NSCoordSaturatingSubtract(mSecondBestEdge.y, mBestEdge.y, nscoord_MAX)); } + const nsPoint& Destination() const { return mDestination; } protected: ScrollUnit mUnit; @@ -207,6 +208,18 @@ void CalcSnapPoints::AddEdge(nscoord aEdge, nscoord aDestination, static void ProcessSnapPositions(CalcSnapPoints& aCalcSnapPoints, const ScrollSnapInfo& aSnapInfo) { for (const auto& target : aSnapInfo.mSnapTargets) { + nsPoint snapPoint(target.mSnapPositionX ? *target.mSnapPositionX + : aCalcSnapPoints.Destination().x, + target.mSnapPositionY ? *target.mSnapPositionY + : aCalcSnapPoints.Destination().y); + nsRect snappedPort = nsRect(snapPoint, aSnapInfo.mSnapportSize); + // Ignore snap points if snapping to the point would leave the snap area + // outside of the snapport. + // https://drafts.csswg.org/css-scroll-snap-1/#snap-scope + if (!snappedPort.Intersects(target.mSnapArea)) { + continue; + } + if (target.mSnapPositionX) { aCalcSnapPoints.AddVerticalEdge(*target.mSnapPositionX); } diff --git a/layout/generic/nsGfxScrollFrame.cpp b/layout/generic/nsGfxScrollFrame.cpp index f1c43c4b68dc..385048d4dff1 100644 --- a/layout/generic/nsGfxScrollFrame.cpp +++ b/layout/generic/nsGfxScrollFrame.cpp @@ -7627,7 +7627,6 @@ static void AppendScrollPositionsForSnap(const nsIFrame* aFrame, const nsIFrame* aScrolledFrame, const nsRect& aScrolledRect, const nsMargin& aScrollPadding, - const Maybe& aSnapport, WritingMode aWritingModeOnScroller, ScrollSnapInfo& aSnapInfo) { nsRect targetRect = nsLayoutUtils::TransformFrameRectToAncestor( @@ -7639,13 +7638,6 @@ static void AppendScrollPositionsForSnap(const nsIFrame* aFrame, nsRect snapArea = InflateByScrollMargin(targetRect, scrollMargin, aScrolledRect); - // Ignore elements outside of the snapport when we scroll to the given - // destination. - // https://drafts.csswg.org/css-scroll-snap-1/#snap-scope - if (aSnapport && !aSnapport->Intersects(snapArea)) { - return; - } - // These snap range shouldn't be involved with scroll-margin since we just // need the visible range of the target element. if (snapArea.width > aSnapInfo.mSnapportSize.width) { @@ -7764,13 +7756,13 @@ static void AppendScrollPositionsForSnap(const nsIFrame* aFrame, /** * Collect the scroll positions corresponding to snap positions of frames in the * subtree rooted at |aFrame|, relative to |aScrolledFrame|, into |aSnapInfo|. - * If |aSnapport| is given, elements outside of the range of |aSnapport| will be - * ignored. */ -static void CollectScrollPositionsForSnap( - nsIFrame* aFrame, nsIFrame* aScrolledFrame, const nsRect& aScrolledRect, - const nsMargin& aScrollPadding, const Maybe& aSnapport, - WritingMode aWritingModeOnScroller, ScrollSnapInfo& aSnapInfo) { +static void CollectScrollPositionsForSnap(nsIFrame* aFrame, + nsIFrame* aScrolledFrame, + const nsRect& aScrolledRect, + const nsMargin& aScrollPadding, + WritingMode aWritingModeOnScroller, + ScrollSnapInfo& aSnapInfo) { // Snap positions only affect the nearest ancestor scroll container on the // element's containing block chain. nsIScrollableFrame* sf = do_QueryFrame(aFrame); @@ -7786,12 +7778,12 @@ static void CollectScrollPositionsForSnap( styleDisplay->mScrollSnapAlign.block != StyleScrollSnapAlignKeyword::None) { AppendScrollPositionsForSnap(f, aScrolledFrame, aScrolledRect, - aScrollPadding, aSnapport, - aWritingModeOnScroller, aSnapInfo); + aScrollPadding, aWritingModeOnScroller, + aSnapInfo); } CollectScrollPositionsForSnap(f, aScrolledFrame, aScrolledRect, - aScrollPadding, aSnapport, - aWritingModeOnScroller, aSnapInfo); + aScrollPadding, aWritingModeOnScroller, + aSnapInfo); } } } @@ -7867,20 +7859,12 @@ layers::ScrollSnapInfo ScrollFrameHelper::ComputeScrollSnapInfo( nsRect snapport = GetScrollPortRect(); nsMargin scrollPadding = GetScrollPadding(); - - Maybe snapportOnDestination; - if (aDestination) { - snapport.MoveTo(aDestination.value()); - snapport.Deflate(scrollPadding); - snapportOnDestination.emplace(snapport); - } else { - snapport.Deflate(scrollPadding); - } + snapport.Deflate(scrollPadding); result.mSnapportSize = snapport.Size(); CollectScrollPositionsForSnap(mScrolledFrame, mScrolledFrame, - GetScrolledRect(), scrollPadding, - snapportOnDestination, writingMode, result); + GetScrolledRect(), scrollPadding, writingMode, + result); return result; } diff --git a/testing/web-platform/meta/css/css-scroll-snap/input/keyboard.html.ini b/testing/web-platform/meta/css/css-scroll-snap/input/keyboard.html.ini index 6deae32c6c2b..44dd85c259bf 100644 --- a/testing/web-platform/meta/css/css-scroll-snap/input/keyboard.html.ini +++ b/testing/web-platform/meta/css/css-scroll-snap/input/keyboard.html.ini @@ -1,27 +1,11 @@ [keyboard.html] expected: TIMEOUT [If there is no valid snap offset on the arrow key's direction other than the current offset, and the scroll-snap-type is proximity, go to the original intended offset] - expected: NOTRUN - - [If the original intended offset is valid as making a snap area cover the snapport, but there's a defined snap offset in between, use the defined snap offset.] - expected: NOTRUN - - [Snaps to top-left after pressing ArrowUp] - expected: TIMEOUT - - [Snaps to top-left after pressing ArrowLeft] - expected: NOTRUN + expected: [TIMEOUT, NOTRUN, FAIL] [Snaps to top-right after pressing ArrowRight] expected: if os == "mac": [NOTRUN, PASS, FAIL] - NOTRUN - - [If the original intended offset is valid as making a snap area cover thesnapport, and there's no other snap offset in between, use the originalintended offset] - expected: NOTRUN - - [If there is no valid snap offset on the arrow key's direction other than the current offset, and the scroll-snap-type is mandatory, stay at the current offset.] - expected: NOTRUN [Snaps to bottom-left after pressing ArrowDown] expected: @@ -30,4 +14,9 @@ if (os == "android") and swgl: PASS if (os == "android") and not swgl: FAIL if (os == "win") and swgl: PASS - [PASS, FAIL] + + [If the original intended offset is valid as making a snap area cover the snapport, but there's a defined snap offset in between, use the defined snap offset.] + expected: [TIMEOUT, PASS] + + [If there is no valid snap offset on the arrow key's direction other than the current offset, and the scroll-snap-type is mandatory, stay at the current offset.] + expected: [NOTRUN, PASS] diff --git a/testing/web-platform/meta/css/css-scroll-snap/scroll-snap-stop.html.ini b/testing/web-platform/meta/css/css-scroll-snap/scroll-snap-stop.html.ini index c6a07bc66037..5f7b0f6085ab 100644 --- a/testing/web-platform/meta/css/css-scroll-snap/scroll-snap-stop.html.ini +++ b/testing/web-platform/meta/css/css-scroll-snap/scroll-snap-stop.html.ini @@ -7,7 +7,3 @@ [A scroll with intended direction and end position should not pass a snap area with scroll-snap-stop: always.] expected: FAIL - - [A scroll with intended end position should always choose the closest snap position regardless of the scroll-snap-stop value.] - expected: FAIL - diff --git a/testing/web-platform/meta/css/css-scroll-snap/scrollTo-scrollBy-snaps.html.ini b/testing/web-platform/meta/css/css-scroll-snap/scrollTo-scrollBy-snaps.html.ini deleted file mode 100644 index ac9d069ee366..000000000000 --- a/testing/web-platform/meta/css/css-scroll-snap/scrollTo-scrollBy-snaps.html.ini +++ /dev/null @@ -1,25 +0,0 @@ -[scrollTo-scrollBy-snaps.html] - [assign scrollLeft and scrollTop for {left: 10000, top: -100} on div lands on (1000, 0)] - expected: FAIL - - [assign scrollLeft and scrollTop for {left: 10000, top: -100} on viewport-defining element lands on (1000, 0)] - expected: FAIL - - [scrollBy({left: 10000, top: -100}) on div lands on (1000, 0)] - expected: FAIL - - [scrollTo({left: 10000, top: -100}) on div lands on (1000, 0)] - expected: FAIL - - [scrollTo({left: 10000, top: -100}) on viewport-defining element lands on (1000, 0)] - expected: FAIL - - [scrollTo({left: 10000, top: -100}) on window lands on (1000, 0)] - expected: FAIL - - [scrollBy({left: 10000, top: -100}) on window lands on (1000, 0)] - expected: FAIL - - [scrollBy({left: 10000, top: -100}) on viewport-defining element lands on (1000, 0)] - expected: FAIL - diff --git a/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-x-axis.html.ini b/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-x-axis.html.ini new file mode 100644 index 000000000000..3ea6608de5d5 --- /dev/null +++ b/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-x-axis.html.ini @@ -0,0 +1,4 @@ +[snap-to-visible-areas-x-axis.html] + [Only snap to visible area on X axis, even when the non-visible ones are closer] + expected: FAIL + bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1768393 diff --git a/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-y-axis.html.ini b/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-y-axis.html.ini new file mode 100644 index 000000000000..67cda40fd7e8 --- /dev/null +++ b/testing/web-platform/meta/css/css-scroll-snap/snap-to-visible-areas-y-axis.html.ini @@ -0,0 +1,4 @@ +[snap-to-visible-areas-y-axis.html] + [Only snap to visible area on Y axis, even when the non-visible ones are closer] + expected: FAIL + bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1768393