Bug 1560237 - Don't propagate scroll-behavior from <body>. r=botond
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Wed, 26 Jun 2019 20:57:05 +0000
changeset 543079 9b96152c5e6ecf97af63865f357e8106e0763ea4
parent 543078 a269192c5d3037c7841d30219516548a6fa8db1a
child 543080 dd6e7c0970d5a55be8f8ca98dd3b3af6267b5ea1
push id2131
push userffxbld-merge
push dateMon, 26 Aug 2019 18:30:20 +0000
treeherdermozilla-release@b19ffb3ca153 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbotond
bugs1560237, 1561107
milestone69.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 1560237 - Don't propagate scroll-behavior from <body>. r=botond From the CSSOM View spec[1]; The scroll-behavior property of the HTML body element is not propagated to the viewport. The reason why this change fixes the test case in this commit is that we don't have two different scrollable frames for <html> and <body> respectively if we don't propagate scroll-behavior property from <body> to <html> so that we can properly find the `flow root` of sticky position elements. In other words, in the case where both of <html> and <body> have properties that are propagated from <body> but they are different we have two scrollable frames as a candidate of the 'flow root' for the sticky position element in the test case, one is the scrollable frame for <html> and the other is the scrollable frame for <body>. That means that nsLayoutUtils::GetNearestScrollableFrame doesn't return what we want in some places, for example we have a pretty similar issue in case of overscroll-behavior which is bug 1561107. Note that the test position-sticky-root-scroller-with-scroll-behavior.html is almost copy-and-pasted from /css/css-position/position-sticky-root-scroller.html [2] in wpt, the reason why we put the test in /css/cssom-view is that there is a handy function to wait for async scroll completion. [1] https://drafts.csswg.org/cssom-view/#propdef-scroll-behavior [2] https://searchfox.org/mozilla-central/rev/928742d3ea30e0eb4a8622d260041564d81a8468/testing/web-platform/tests/css/css-position/position-sticky-root-scroller.html Differential Revision: https://phabricator.services.mozilla.com/D35739
layout/base/ScrollStyles.cpp
layout/base/ScrollStyles.h
layout/base/nsPresContext.cpp
layout/generic/nsGfxScrollFrame.cpp
layout/reftests/scrolling/scroll-behavior-1.html
layout/reftests/scrolling/scroll-behavior-7.html
layout/reftests/scrolling/scroll-behavior-9.html
testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
testing/web-platform/tests/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html
--- a/layout/base/ScrollStyles.cpp
+++ b/layout/base/ScrollStyles.cpp
@@ -46,25 +46,23 @@ void ScrollStyles::InitializeScrollSnapS
       break;
   }
 }
 
 ScrollStyles::ScrollStyles(WritingMode aWritingMode, StyleOverflow aH,
                            StyleOverflow aV, const nsStyleDisplay* aDisplay)
     : mHorizontal(aH),
       mVertical(aV),
-      mScrollBehavior(aDisplay->mScrollBehavior),
       mOverscrollBehaviorX(aDisplay->mOverscrollBehaviorX),
       mOverscrollBehaviorY(aDisplay->mOverscrollBehaviorY) {
   InitializeScrollSnapStrictness(aWritingMode, aDisplay);
 }
 
 ScrollStyles::ScrollStyles(WritingMode aWritingMode,
                            const nsStyleDisplay* aDisplay)
     : mHorizontal(aDisplay->mOverflowX),
       mVertical(aDisplay->mOverflowY),
-      mScrollBehavior(aDisplay->mScrollBehavior),
       mOverscrollBehaviorX(aDisplay->mOverscrollBehaviorX),
       mOverscrollBehaviorY(aDisplay->mOverscrollBehaviorY) {
   InitializeScrollSnapStrictness(aWritingMode, aDisplay);
 }
 
 }  // namespace mozilla
--- a/layout/base/ScrollStyles.h
+++ b/layout/base/ScrollStyles.h
@@ -18,54 +18,46 @@ struct nsStyleDisplay;
 namespace mozilla {
 
 struct ScrollStyles {
   // Always one of Scroll, Hidden, or Auto
   StyleOverflow mHorizontal;
   StyleOverflow mVertical;
   // Always one of NS_STYLE_SCROLL_BEHAVIOR_AUTO or
   // NS_STYLE_SCROLL_BEHAVIOR_SMOOTH
-  uint8_t mScrollBehavior;
   StyleOverscrollBehavior mOverscrollBehaviorX;
   StyleOverscrollBehavior mOverscrollBehaviorY;
   StyleScrollSnapStrictness mScrollSnapStrictnessX;
   StyleScrollSnapStrictness mScrollSnapStrictnessY;
 
   ScrollStyles(StyleOverflow aH, StyleOverflow aV)
       : mHorizontal(aH),
         mVertical(aV),
-        mScrollBehavior(NS_STYLE_SCROLL_BEHAVIOR_AUTO),
         mOverscrollBehaviorX(StyleOverscrollBehavior::Auto),
         mOverscrollBehaviorY(StyleOverscrollBehavior::Auto),
         mScrollSnapStrictnessX(StyleScrollSnapStrictness::None),
         mScrollSnapStrictnessY(StyleScrollSnapStrictness::None) {}
 
   ScrollStyles(WritingMode aWritingMode, const nsStyleDisplay* aDisplay);
   ScrollStyles(WritingMode aWritingMode, StyleOverflow aH, StyleOverflow aV,
                const nsStyleDisplay* aDisplay);
   void InitializeScrollSnapStrictness(WritingMode aWritingMode,
                                       const nsStyleDisplay* aDisplay);
   bool operator==(const ScrollStyles& aStyles) const {
     return aStyles.mHorizontal == mHorizontal &&
            aStyles.mVertical == mVertical &&
-           aStyles.mScrollBehavior == mScrollBehavior &&
            aStyles.mOverscrollBehaviorX == mOverscrollBehaviorX &&
            aStyles.mOverscrollBehaviorY == mOverscrollBehaviorY &&
            aStyles.mScrollSnapStrictnessX == mScrollSnapStrictnessX &&
            aStyles.mScrollSnapStrictnessY == mScrollSnapStrictnessY;
   }
   bool operator!=(const ScrollStyles& aStyles) const {
     return !(*this == aStyles);
   }
   bool IsHiddenInBothDirections() const {
     return mHorizontal == StyleOverflow::Hidden &&
            mVertical == StyleOverflow::Hidden;
   }
-  bool IsSmoothScroll(dom::ScrollBehavior aBehavior) const {
-    return aBehavior == dom::ScrollBehavior::Smooth ||
-           (aBehavior == dom::ScrollBehavior::Auto &&
-            mScrollBehavior == NS_STYLE_SCROLL_BEHAVIOR_SMOOTH);
-  }
 };
 
 }  // namespace mozilla
 
 #endif  // mozilla_ScrollStyles_h
--- a/layout/base/nsPresContext.cpp
+++ b/layout/base/nsPresContext.cpp
@@ -1010,17 +1010,16 @@ gfxSize nsPresContext::ScreenSizeInchesF
 
   return deviceSizeInches;
 }
 
 static bool CheckOverflow(ComputedStyle* aComputedStyle,
                           ScrollStyles* aStyles) {
   const nsStyleDisplay* display = aComputedStyle->StyleDisplay();
   if (display->mOverflowX == StyleOverflow::Visible &&
-      display->mScrollBehavior == NS_STYLE_SCROLL_BEHAVIOR_AUTO &&
       display->mOverscrollBehaviorX == StyleOverscrollBehavior::Auto &&
       display->mOverscrollBehaviorY == StyleOverscrollBehavior::Auto &&
       display->mScrollSnapType.strictness == StyleScrollSnapStrictness::None) {
     return false;
   }
 
   WritingMode writingMode = WritingMode(aComputedStyle);
   if (display->mOverflowX == StyleOverflow::MozHiddenUnscrollable) {
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -7127,11 +7127,11 @@ bool ScrollFrameHelper::SmoothScrollVisu
                                       ? nsGkAtoms::restore
                                       : nsGkAtoms::other);
   return true;
 }
 
 bool ScrollFrameHelper::IsSmoothScroll(dom::ScrollBehavior aBehavior) const {
   return aBehavior == dom::ScrollBehavior::Smooth ||
          (aBehavior == dom::ScrollBehavior::Auto &&
-          GetScrollStylesFromFrame().mScrollBehavior ==
+          GetFrameForStyle()->StyleDisplay()->mScrollBehavior ==
               NS_STYLE_SCROLL_BEHAVIOR_SMOOTH);
 }
--- a/layout/reftests/scrolling/scroll-behavior-1.html
+++ b/layout/reftests/scrolling/scroll-behavior-1.html
@@ -8,17 +8,17 @@
         html,body {
             color: black;
             background-color: white;
             font-size: 16px;
             padding: 0;
             margin: 0;
         }
 
-        body {
+        html {
             scroll-behavior: smooth;
         }
 
         #a_box {
             position: relative;
             left: 10px;
             top: 10px;
             width: 20px;
--- a/layout/reftests/scrolling/scroll-behavior-7.html
+++ b/layout/reftests/scrolling/scroll-behavior-7.html
@@ -8,17 +8,17 @@
         html,body {
             color: black;
             background-color: white;
             font-size: 16px;
             padding: 0;
             margin: 0;
         }
 
-        body {
+        html {
             scroll-behavior: smooth;
         }
 
         #a_box {
             position: relative;
             left: 10px;
             top: 10px;
             width: 20px;
--- a/layout/reftests/scrolling/scroll-behavior-9.html
+++ b/layout/reftests/scrolling/scroll-behavior-9.html
@@ -43,13 +43,13 @@
       window.scrollTo({left: 500, top: 500});
       // Interrupt smooth scrolling
       window.scrollTo({left: window.scrollX, top: window.scrollY});
       // If scroll was not performed smoothly, we would be at 500,500 now
     }
     document.documentElement.removeAttribute("class");
   }
   window.scrollTo({left: 0, top: 0, behavior: "instant"});
-  document.body.style.scrollBehavior = "smooth";
+  document.documentElement.style.scrollBehavior = "smooth";
   window.addEventListener("MozReftestInvalidate", doTest);
 </script>
 </body>
 </html>
--- a/testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
+++ b/testing/web-platform/meta/css/cssom-view/scroll-behavior-smooth.html.ini
@@ -1,8 +1,5 @@
 [scroll-behavior-smooth.html]
-  [BODY element scroll-behavior should not propagate to viewport]
-    expected: FAIL
-
   [HTML element scroll-behavior should propagate to viewport]
     expected:
       if (os == "android") and e10s: "FAIL"
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/cssom-view/position-sticky-root-scroller-with-scroll-behavior.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<title>position:sticky should operate correctly for the root scroller</title>
+<link rel="help" href="https://drafts.csswg.org/css-position-3/#valdef-position-sticky">
+<link rel="help" href="https://drafts.csswg.org/cssom-view/#propdef-scroll-behavior">
+<meta name="assert" content="This test checks that position:sticky elements work when using the root (document) scroller which has `scroll-behavior` property" />
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script src="support/scroll-behavior.js"></script>
+
+<style>
+html {
+  scroll-behavior: smooth;
+}
+
+body {
+  /* Assumption: 3000px is taller than any user agents test window size. */
+  height: 3000px;
+  /* A property which propagates for <html>. */
+  overflow-x: hidden;
+}
+
+#sticky {
+  position: sticky;
+  top: 50px;
+  width: 200px;
+  height: 200px;
+  background-color: green;
+}
+</style>
+
+<div id="sticky"></div>
+
+<script>
+promise_test(async () => {
+  window.scrollTo(0, 700);
+
+  await waitForScrollEnd(document.scrollingElement);
+
+  assert_equals(sticky.offsetTop, 700 + 50);
+}, 'Sticky elements work with the root (document) scroller');
+</script>