Bug 1544198 - Use the proper frame to get scroll-snap-type value on the root element. r=botond
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 23 Apr 2019 01:16:02 +0000
changeset 470431 52410914b543300394f6b98c8dd6e3d73fb84780
parent 470430 5b413410d2c5f93ca4240a3c4608a1182d53a2d2
child 470432 42c76408a8bdeea672dc5144c103cb34fe850b5f
push id35905
push userdvarga@mozilla.com
push dateTue, 23 Apr 2019 09:53:27 +0000
treeherdermozilla-central@831918f009f6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1544198
milestone68.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1544198 - Use the proper frame to get scroll-snap-type value on the root element. r=botond Now scroll-snap-type property on body element doesn't affect scroll position so that scrollTo-scrollBy-snaps.html is needed to be modified to specify scroll-snap-type on html. Differential Revision: https://phabricator.services.mozilla.com/D27987
gfx/layers/FrameMetrics.cpp
gfx/layers/FrameMetrics.h
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
testing/web-platform/tests/css/css-scroll-snap/scrollTo-scrollBy-snaps.html
--- a/gfx/layers/FrameMetrics.cpp
+++ b/gfx/layers/FrameMetrics.cpp
@@ -2,16 +2,18 @@
 /* vim: set ts=8 sts=2 et sw=2 tw=80: */
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "FrameMetrics.h"
 #include "gfxPrefs.h"
 #include "nsStyleConsts.h"
+#include "nsStyleStruct.h"
+#include "mozilla/WritingModes.h"
 
 namespace mozilla {
 namespace layers {
 
 const ScrollableLayerGuid::ViewID ScrollableLayerGuid::NULL_SCROLL_ID = 0;
 
 void FrameMetrics::RecalculateLayoutViewportOffset() {
   if (!mIsRootContent) {
@@ -83,16 +85,53 @@ void FrameMetrics::KeepLayoutViewportEnc
   // to go outside the scrollable rect.
   aLayoutViewport = aLayoutViewport.MoveInsideAndClamp(aScrollableRect);
 }
 
 void ScrollMetadata::SetUsesContainerScrolling(bool aValue) {
   mUsesContainerScrolling = aValue;
 }
 
+void ScrollSnapInfo::InitializeScrollSnapType(WritingMode aWritingMode,
+                                              const nsStyleDisplay* aDisplay) {
+  if (aDisplay->mScrollSnapType.strictness == StyleScrollSnapStrictness::None) {
+    return;
+  }
+
+  mScrollSnapTypeX = StyleScrollSnapStrictness::None;
+  mScrollSnapTypeY = StyleScrollSnapStrictness::None;
+
+  switch (aDisplay->mScrollSnapType.axis) {
+    case StyleScrollSnapAxis::X:
+      mScrollSnapTypeX = aDisplay->mScrollSnapType.strictness;
+      break;
+    case StyleScrollSnapAxis::Y:
+      mScrollSnapTypeY = aDisplay->mScrollSnapType.strictness;
+      break;
+    case StyleScrollSnapAxis::Block:
+      if (aWritingMode.IsVertical()) {
+        mScrollSnapTypeX = aDisplay->mScrollSnapType.strictness;
+      } else {
+        mScrollSnapTypeY = aDisplay->mScrollSnapType.strictness;
+      }
+      break;
+    case StyleScrollSnapAxis::Inline:
+      if (aWritingMode.IsVertical()) {
+        mScrollSnapTypeY = aDisplay->mScrollSnapType.strictness;
+      } else {
+        mScrollSnapTypeX = aDisplay->mScrollSnapType.strictness;
+      }
+      break;
+    case StyleScrollSnapAxis::Both:
+      mScrollSnapTypeX = aDisplay->mScrollSnapType.strictness;
+      mScrollSnapTypeY = aDisplay->mScrollSnapType.strictness;
+      break;
+  }
+}
+
 static OverscrollBehavior ToOverscrollBehavior(
     StyleOverscrollBehavior aBehavior) {
   switch (aBehavior) {
     case StyleOverscrollBehavior::Auto:
       return OverscrollBehavior::Auto;
     case StyleOverscrollBehavior::Contain:
       return OverscrollBehavior::Contain;
     case StyleOverscrollBehavior::None:
--- a/gfx/layers/FrameMetrics.h
+++ b/gfx/layers/FrameMetrics.h
@@ -20,16 +20,21 @@
 #include "mozilla/layers/LayersTypes.h"          // for ScrollDirection
 #include "mozilla/layers/ScrollableLayerGuid.h"  // for ScrollableLayerGuid
 #include "mozilla/StaticPtr.h"                   // for StaticAutoPtr
 #include "mozilla/TimeStamp.h"                   // for TimeStamp
 #include "nsString.h"
 #include "nsStyleCoord.h"  // for nsStyleCoord
 #include "PLDHashTable.h"  // for PLDHashNumber
 
+struct nsStyleDisplay;
+namespace mozilla {
+class WritingMode;
+}  // namespace mozilla
+
 namespace IPC {
 template <typename T>
 struct ParamTraits;
 }  // namespace IPC
 
 namespace mozilla {
 namespace layers {
 
@@ -727,16 +732,19 @@ struct ScrollSnapInfo {
            mSnapportSize == aOther.mSnapportSize;
   }
 
   bool HasScrollSnapping() const {
     return mScrollSnapTypeY != mozilla::StyleScrollSnapStrictness::None ||
            mScrollSnapTypeX != mozilla::StyleScrollSnapStrictness::None;
   }
 
+  void InitializeScrollSnapType(WritingMode aWritingMode,
+                                const nsStyleDisplay* aDisplay);
+
   // The scroll frame's scroll-snap-type.
   mozilla::StyleScrollSnapStrictness mScrollSnapTypeX =
       mozilla::StyleScrollSnapStrictness::None;
   mozilla::StyleScrollSnapStrictness mScrollSnapTypeY =
       mozilla::StyleScrollSnapStrictness::None;
 
   // The intervals derived from the scroll frame's scroll-snap-points.
   Maybe<nscoord> mScrollSnapIntervalX;
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -4207,20 +4207,17 @@ void ScrollFrameHelper::ScrollBy(nsIntPo
       NS_ERROR("Invalid scroll mode");
       return;
   }
 
   nsPoint newPos = mDestination + nsPoint(aDelta.x * deltaMultiplier.width,
                                           aDelta.y * deltaMultiplier.height);
 
   if (aSnap == nsIScrollableFrame::ENABLE_SNAP) {
-    ScrollStyles styles = GetScrollStylesFromFrame();
-
-    if (styles.mScrollSnapTypeY != StyleScrollSnapStrictness::None ||
-        styles.mScrollSnapTypeX != StyleScrollSnapStrictness::None) {
+    if (NeedsScrollSnap()) {
       nscoord appUnitsPerDevPixel =
           mOuter->PresContext()->AppUnitsPerDevPixel();
       deltaMultiplier = nsSize(appUnitsPerDevPixel, appUnitsPerDevPixel);
       negativeTolerance = 0.1f;
       positiveTolerance = 0;
       nsIScrollableFrame::ScrollUnit snapUnit = aUnit;
       if (aOrigin == nsGkAtoms::mouseWheel) {
         // When using a clicky scroll wheel, snap point selection works the same
@@ -5335,16 +5332,31 @@ nsIFrame* ScrollFrameHelper::GetFrameFor
     }
   } else {
     styleFrame = mOuter;
   }
 
   return styleFrame;
 }
 
+bool ScrollFrameHelper::NeedsScrollSnap() const {
+  if (StaticPrefs::layout_css_scroll_snap_v1_enabled()) {
+    nsIFrame* scrollSnapFrame = GetFrameForScrollSnap();
+    if (!scrollSnapFrame) {
+      return false;
+    }
+    return scrollSnapFrame->StyleDisplay()->mScrollSnapType.strictness !=
+           StyleScrollSnapStrictness::None;
+  }
+
+  ScrollStyles styles = GetScrollStylesFromFrame();
+  return styles.mScrollSnapTypeY != StyleScrollSnapStrictness::None ||
+         styles.mScrollSnapTypeX != StyleScrollSnapStrictness::None;
+}
+
 bool ScrollFrameHelper::IsScrollbarOnRight() const {
   nsPresContext* presContext = mOuter->PresContext();
 
   // The position of the scrollbar in top-level windows depends on the pref
   // layout.scrollbar.side. For non-top-level elements, it depends only on the
   // directionaliy of the element (equivalent to a value of "1" for the pref).
   if (!mIsRoot) {
     return IsPhysicalLTR();
@@ -6835,26 +6847,30 @@ layers::ScrollSnapInfo ScrollFrameHelper
   return result;
 }
 
 layers::ScrollSnapInfo ScrollFrameHelper::ComputeScrollSnapInfo(
     const Maybe<nsPoint>& aDestination) const {
   MOZ_ASSERT(StaticPrefs::layout_css_scroll_snap_v1_enabled());
 
   ScrollSnapInfo result;
-  ScrollStyles styles = GetScrollStylesFromFrame();
-
-  if (styles.mScrollSnapTypeY == StyleScrollSnapStrictness::None &&
-      styles.mScrollSnapTypeX == StyleScrollSnapStrictness::None) {
+
+  nsIFrame* scrollSnapFrame = GetFrameForScrollSnap();
+  if (!scrollSnapFrame) {
+    return result;
+  }
+
+  const nsStyleDisplay* disp = scrollSnapFrame->StyleDisplay();
+  if (disp->mScrollSnapType.strictness == StyleScrollSnapStrictness::None) {
     // We won't be snapping, short-circuit the computation.
     return result;
   }
 
-  result.mScrollSnapTypeX = styles.mScrollSnapTypeX;
-  result.mScrollSnapTypeY = styles.mScrollSnapTypeY;
+  WritingMode writingMode = GetFrameForDir()->GetWritingMode();
+  result.InitializeScrollSnapType(writingMode, disp);
 
   nsRect snapport = GetScrollPortRect();
   nsMargin scrollPadding = GetScrollPadding();
 
   Maybe<nsRect> snapportOnDestination;
   if (aDestination) {
     if (IsPhysicalLTR()) {
       snapport.MoveTo(aDestination.value());
@@ -6863,17 +6879,16 @@ layers::ScrollSnapInfo ScrollFrameHelper
           nsPoint(aDestination->x - snapport.Size().width, aDestination->y));
     }
     snapport.Deflate(scrollPadding);
     snapportOnDestination.emplace(snapport);
   } else {
     snapport.Deflate(scrollPadding);
   }
 
-  WritingMode writingMode = GetFrameForDir()->GetWritingMode();
   result.mSnapportSize = snapport.Size();
   CollectScrollPositionsForSnap(mScrolledFrame, mScrolledFrame,
                                 GetScrolledRect(), scrollPadding,
                                 snapportOnDestination, writingMode, result);
   return result;
 }
 
 layers::ScrollSnapInfo ScrollFrameHelper::GetScrollSnapInfo(
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -415,16 +415,18 @@ class ScrollFrameHelper : public nsIRefl
                                             // scroll-padding we use
 
   // This is the for the old unspecced scroll snap implementation.
   ScrollSnapInfo ComputeOldScrollSnapInfo() const;
   // This is the for the scroll snap v1 implementation.
   ScrollSnapInfo ComputeScrollSnapInfo(
       const Maybe<nsPoint>& aDestination) const;
 
+  bool NeedsScrollSnap() const;
+
  public:
   bool IsScrollbarOnRight() const;
   bool IsScrollingActive(nsDisplayListBuilder* aBuilder) const;
   bool IsMaybeAsynchronouslyScrolled() const {
     // If this is true, then we'll build an ASR, and that's what we want
     // to know I think.
     return mWillBuildScrollableLayer;
   }
--- a/testing/web-platform/tests/css/css-scroll-snap/scrollTo-scrollBy-snaps.html
+++ b/testing/web-platform/tests/css/css-scroll-snap/scrollTo-scrollBy-snaps.html
@@ -1,14 +1,14 @@
 <!DOCTYPE html>
 <link rel="help" href="https://drafts.csswg.org/css-scroll-snap-1" />
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <style>
-body {
+html {
   margin: 0px;
   overflow: scroll;
   scroll-snap-type: both mandatory;
 }
 div {
   position: absolute;
 }
 .scroller {