Bug 1544198 - Use the proper writing-mode for scroll-snap for viewport. r=jfkthame
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Tue, 23 Apr 2019 01:12:09 +0000
changeset 470428 f413cb36905d980cccb30577f827938f80cbe913
parent 470427 181f8519792032d158f03d2757724df99955cd44
child 470429 530a3f39ecbd096436a8cf38d1d73068ff1fbcb8
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)
reviewersjfkthame
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 writing-mode for scroll-snap for viewport. r=jfkthame In the CSS writing mode spec [1], the writing mode for the document should be taken from body, GetFrameForDir() is the function to get the corresponding frame for writing-mode. A web platform test for this case will be added at the last of this commit series. Unfortunately as of this commit, we can't introduce proper test cases since there is another issue on scroll-snap-type which will be fixed in subsequent commits. [1] https://drafts.csswg.org/css-writing-modes-4/#principal-flow Differential Revision: https://phabricator.services.mozilla.com/D27984
layout/generic/nsGfxScrollFrame.cpp
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -5295,16 +5295,17 @@ void ScrollFrameHelper::PostOverflowEven
   mAsyncScrollPortEvent = new AsyncScrollPortEvent(this);
   rpc->AddWillPaintObserver(mAsyncScrollPortEvent.get());
 }
 
 nsIFrame* ScrollFrameHelper::GetFrameForDir() const {
   nsIFrame* frame = mOuter;
   // XXX This is a bit on the slow side.
   if (mIsRoot) {
+    // https://drafts.csswg.org/css-writing-modes-4/#principal-flow
     // If we're the root scrollframe, we need the root element's style data.
     nsPresContext* presContext = mOuter->PresContext();
     Document* document = presContext->Document();
     Element* root = document->GetRootElement();
 
     // But for HTML and XHTML we want the body element.
     nsCOMPtr<nsIHTMLDocument> htmlDoc = do_QueryInterface(document);
     if (htmlDoc) {
@@ -6527,16 +6528,17 @@ static nsRect InflateByScrollMargin(cons
  * Append scroll positions for valid snap positions into |aSnapInfo| if
  * applicable.
  */
 static void AppendScrollPositionsForSnap(const nsIFrame* aFrame,
                                          const nsIFrame* aScrolledFrame,
                                          const nsRect& aScrolledRect,
                                          const nsMargin& aScrollPadding,
                                          const Maybe<nsRect>& aSnapport,
+                                         WritingMode aWritingModeOnScroller,
                                          ScrollSnapInfo& aSnapInfo) {
   nsRect targetRect = nsLayoutUtils::TransformFrameRectToAncestor(
       aFrame, aFrame->GetRectRelativeToSelf(), aScrolledFrame);
   // 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(targetRect)) {
     return;
@@ -6558,138 +6560,140 @@ static void AppendScrollPositionsForSnap
 
   // Shift target rect position by the scroll padding to get the padded
   // position thus we don't need to take account scroll-padding values in
   // ScrollSnapUtils::GetSnapPointForDestination() when it gets called from
   // the compositor thread.
   targetRect.y -= aScrollPadding.top;
   targetRect.x -= aScrollPadding.left;
 
-  WritingMode writingMode = aScrolledFrame->GetWritingMode();
-  LogicalRect logicalTargetRect(writingMode, targetRect,
+  LogicalRect logicalTargetRect(aWritingModeOnScroller, targetRect,
                                 aSnapInfo.mSnapportSize);
-  LogicalSize logicalSnapportRect(writingMode, aSnapInfo.mSnapportSize);
+  LogicalSize logicalSnapportRect(aWritingModeOnScroller,
+                                  aSnapInfo.mSnapportSize);
 
   Maybe<nscoord> blockDirectionPosition;
   Maybe<nscoord> inlineDirectionPosition;
 
   const nsStyleDisplay* styleDisplay = aFrame->StyleDisplay();
-  nscoord containerBSize = logicalSnapportRect.BSize(writingMode);
+  nscoord containerBSize = logicalSnapportRect.BSize(aWritingModeOnScroller);
   switch (styleDisplay->mScrollSnapAlign.block) {
     case StyleScrollSnapAlignKeyword::None:
       break;
     case StyleScrollSnapAlignKeyword::Start:
-      blockDirectionPosition.emplace(logicalTargetRect.BStart(writingMode));
+      blockDirectionPosition.emplace(
+          logicalTargetRect.BStart(aWritingModeOnScroller));
       break;
     case StyleScrollSnapAlignKeyword::End:
-      if (writingMode.IsVerticalRL()) {
-        blockDirectionPosition.emplace(containerBSize -
-                                       logicalTargetRect.BEnd(writingMode));
+      if (aWritingModeOnScroller.IsVerticalRL()) {
+        blockDirectionPosition.emplace(
+            containerBSize - logicalTargetRect.BEnd(aWritingModeOnScroller));
       } else {
         // What we need here is the scroll position instead of the snap position
         // itself, so we need, for example, the top edge of the scroll port
         // on horizontal-tb when the frame is positioned at the bottom edge of
         // the scroll port. For this reason we subtract containerBSize from
         // BEnd of the target.
-        blockDirectionPosition.emplace(logicalTargetRect.BEnd(writingMode) -
-                                       containerBSize);
+        blockDirectionPosition.emplace(
+            logicalTargetRect.BEnd(aWritingModeOnScroller) - containerBSize);
       }
       break;
     case StyleScrollSnapAlignKeyword::Center: {
-      nscoord targetCenter = (logicalTargetRect.BStart(writingMode) +
-                              logicalTargetRect.BEnd(writingMode)) /
+      nscoord targetCenter = (logicalTargetRect.BStart(aWritingModeOnScroller) +
+                              logicalTargetRect.BEnd(aWritingModeOnScroller)) /
                              2;
       nscoord halfSnapportSize = containerBSize / 2;
       // Get the center of the target to align with the center of the snapport
       // depending on direction.
-      if (writingMode.IsVerticalRL()) {
+      if (aWritingModeOnScroller.IsVerticalRL()) {
         blockDirectionPosition.emplace(halfSnapportSize - targetCenter);
       } else {
         blockDirectionPosition.emplace(targetCenter - halfSnapportSize);
       }
       break;
     }
   }
 
-  nscoord containerISize = logicalSnapportRect.ISize(writingMode);
+  nscoord containerISize = logicalSnapportRect.ISize(aWritingModeOnScroller);
   switch (styleDisplay->mScrollSnapAlign.inline_) {
     case StyleScrollSnapAlignKeyword::None:
       break;
     case StyleScrollSnapAlignKeyword::Start:
-      inlineDirectionPosition.emplace(logicalTargetRect.IStart(writingMode));
+      inlineDirectionPosition.emplace(
+          logicalTargetRect.IStart(aWritingModeOnScroller));
       break;
     case StyleScrollSnapAlignKeyword::End:
-      if (writingMode.IsInlineReversed()) {
-        inlineDirectionPosition.emplace(containerISize -
-                                        logicalTargetRect.IEnd(writingMode));
+      if (aWritingModeOnScroller.IsInlineReversed()) {
+        inlineDirectionPosition.emplace(
+            containerISize - logicalTargetRect.IEnd(aWritingModeOnScroller));
       } else {
         // Same as above BEnd case, we subtract containerISize.
-        inlineDirectionPosition.emplace(logicalTargetRect.IEnd(writingMode) -
-                                        containerISize);
+        inlineDirectionPosition.emplace(
+            logicalTargetRect.IEnd(aWritingModeOnScroller) - containerISize);
       }
       break;
     case StyleScrollSnapAlignKeyword::Center: {
-      nscoord targetCenter = (logicalTargetRect.IStart(writingMode) +
-                              logicalTargetRect.IEnd(writingMode)) /
+      nscoord targetCenter = (logicalTargetRect.IStart(aWritingModeOnScroller) +
+                              logicalTargetRect.IEnd(aWritingModeOnScroller)) /
                              2;
       nscoord halfSnapportSize = containerISize / 2;
       // Get the center of the target to align with the center of the snapport
       // depending on direction.
-      if (writingMode.IsInlineReversed()) {
+      if (aWritingModeOnScroller.IsInlineReversed()) {
         inlineDirectionPosition.emplace(halfSnapportSize - targetCenter);
       } else {
         inlineDirectionPosition.emplace(targetCenter - halfSnapportSize);
       }
       break;
     }
   }
 
   if (inlineDirectionPosition) {
-    (writingMode.IsVertical() ? aSnapInfo.mSnapPositionY
-                              : aSnapInfo.mSnapPositionX)
+    (aWritingModeOnScroller.IsVertical() ? aSnapInfo.mSnapPositionY
+                                         : aSnapInfo.mSnapPositionX)
         .AppendElement(inlineDirectionPosition.value());
   }
 
   if (blockDirectionPosition) {
-    (writingMode.IsVertical() ? aSnapInfo.mSnapPositionX
-                              : aSnapInfo.mSnapPositionY)
+    (aWritingModeOnScroller.IsVertical() ? aSnapInfo.mSnapPositionX
+                                         : aSnapInfo.mSnapPositionY)
         .AppendElement(blockDirectionPosition.value());
   }
 }
 
 /**
  * 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<nsRect>& aSnapport,
-                                          ScrollSnapInfo& aSnapInfo) {
+static void CollectScrollPositionsForSnap(
+    nsIFrame* aFrame, nsIFrame* aScrolledFrame, const nsRect& aScrolledRect,
+    const nsMargin& aScrollPadding, const Maybe<nsRect>& aSnapport,
+    WritingMode aWritingModeOnScroller, ScrollSnapInfo& aSnapInfo) {
   MOZ_ASSERT(StaticPrefs::layout_css_scroll_snap_v1_enabled());
 
   nsIFrame::ChildListIterator childLists(aFrame);
   for (; !childLists.IsDone(); childLists.Next()) {
     nsFrameList::Enumerator childFrames(childLists.CurrentList());
     for (; !childFrames.AtEnd(); childFrames.Next()) {
       nsIFrame* f = childFrames.get();
 
       const nsStyleDisplay* styleDisplay = f->StyleDisplay();
       if (styleDisplay->mScrollSnapAlign.inline_ !=
               StyleScrollSnapAlignKeyword::None ||
           styleDisplay->mScrollSnapAlign.block !=
               StyleScrollSnapAlignKeyword::None) {
         AppendScrollPositionsForSnap(f, aScrolledFrame, aScrolledRect,
-                                     aScrollPadding, aSnapport, aSnapInfo);
+                                     aScrollPadding, aSnapport,
+                                     aWritingModeOnScroller, aSnapInfo);
       }
       CollectScrollPositionsForSnap(f, aScrolledFrame, aScrolledRect,
-                                    aScrollPadding, aSnapport, aSnapInfo);
+                                    aScrollPadding, aSnapport,
+                                    aWritingModeOnScroller, aSnapInfo);
     }
   }
 }
 
 /**
  * Collect the scroll-snap-coordinates of frames in the subtree rooted at
  * |aFrame|, relative to |aScrolledFrame|, into |aOutCoords|.
  */
@@ -6833,20 +6837,21 @@ 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, result);
+                                  snapportOnDestination, writingMode, result);
     return result;
   }
 
   CollectScrollSnapCoordinates(mScrolledFrame, mScrolledFrame,
                                result.mScrollSnapCoordinates);
 
   return result;
 }