Bug 1139306: Fix margin sides for right-to-left scrollbars depending on the actual position of the scrollbar as determined by layout.scrollbar.side, r=tn
authorSimon Montagu <smontagu@smontagu.org>
Tue, 12 May 2015 01:49:25 -0700
changeset 243500 f02fc3766b785511744e5261d15df6e00d8c3128
parent 243483 6afb088b0cff22dcbdf88ba0d5c1291889d1e7e8
child 243501 78cd30c12d31ce03278c8176ba191bbc86015546
push id28741
push userkwierso@gmail.com
push dateTue, 12 May 2015 23:24:40 +0000
treeherdermozilla-central@d476776d920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1139306
milestone40.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 1139306: Fix margin sides for right-to-left scrollbars depending on the actual position of the scrollbar as determined by layout.scrollbar.side, r=tn
b2g/chrome/content/content.css
browser/themes/linux/devtools/floating-scrollbars.css
browser/themes/osx/devtools/floating-scrollbars.css
browser/themes/windows/devtools/floating-scrollbars.css
layout/generic/nsGfxScrollFrame.cpp
layout/generic/nsGfxScrollFrame.h
layout/xul/nsIScrollbarMediator.h
layout/xul/nsListBoxBodyFrame.h
layout/xul/nsScrollbarFrame.cpp
layout/xul/tree/nsTreeBodyFrame.h
mobile/android/themes/core/content.css
--- a/b2g/chrome/content/content.css
+++ b/b2g/chrome/content/content.css
@@ -25,29 +25,24 @@ html xul|scrollbar[root="true"] {
 html xul|scrollbar {
   -moz-appearance: none !important;
   background-color: transparent !important;
   background-image: none !important;
   border: 0px solid transparent !important;
   pointer-events: none;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   where layout actually puts the scrollbar */
 xul|scrollbar[orient="vertical"] {
-  -moz-margin-start: -8px;
+  margin-left: -8px;
   min-width: 8px;
   max-width: 8px;
 }
 
-/* workaround for bug 1119057: as -moz-margin-start may not work as expected,
- * force a right margin value in RTL mode.  */
-[dir="rtl"] xul|scrollbar[root="true"][orient="vertical"] {
-  -moz-margin-start: unset;
-  margin-right: -8px;
-}
-
 xul|scrollbar[orient="vertical"] xul|thumb {
   max-width: 6px !important;
   min-width: 6px !important;
 }
 
 xul|scrollbar[orient="horizontal"] {
   margin-top: -8px;
   min-height: 8px;
--- a/browser/themes/linux/devtools/floating-scrollbars.css
+++ b/browser/themes/linux/devtools/floating-scrollbars.css
@@ -4,18 +4,20 @@ scrollbar {
   -moz-appearance: none !important;
   position: relative;
   background-color: transparent;
   background-image: none;
   z-index: 2147483647;
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   where layout actually puts the scrollbar */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -10px;
+  margin-left: -10px;
   min-width: 10px;
   max-width: 10px;
 }
 
 scrollbar[orient="horizontal"] {
   margin-top: -10px;
   min-height: 10px;
   max-height: 10px;
--- a/browser/themes/osx/devtools/floating-scrollbars.css
+++ b/browser/themes/osx/devtools/floating-scrollbars.css
@@ -5,18 +5,20 @@ scrollbar {
   position: relative;
   background-color: transparent;
   background-image: none;
   border: 0px solid transparent;
   z-index: 2147483647;
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   where layout actually puts the scrollbar */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -8px;
+  margin-left: -8px;
   min-width: 8px;
   max-width: 8px;
 }
 
 scrollbar[orient="horizontal"] {
   margin-top: -8px;
   min-height: 8px;
   max-height: 8px;
--- a/browser/themes/windows/devtools/floating-scrollbars.css
+++ b/browser/themes/windows/devtools/floating-scrollbars.css
@@ -4,18 +4,20 @@ scrollbar {
   -moz-appearance: none !important;
   position: relative;
   background-color: transparent;
   background-image: none;
   z-index: 2147483647;
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   where layout actually puts the scrollbar */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -10px;
+  margin-left: -10px;
   min-width: 10px;
   max-width: 10px;
 }
 
 scrollbar[orient="horizontal"] {
   margin-top: -10px;
   min-height: 10px;
   max-height: 10px;
--- a/layout/generic/nsGfxScrollFrame.cpp
+++ b/layout/generic/nsGfxScrollFrame.cpp
@@ -396,17 +396,17 @@ nsHTMLScrollFrame::TryLayout(ScrollReflo
   }
 
   nscoord vScrollbarActualWidth = aState->mInsideBorderSize.width - scrollPortSize.width;
 
   aState->mShowHScrollbar = aAssumeHScroll;
   aState->mShowVScrollbar = aAssumeVScroll;
   nsPoint scrollPortOrigin(aState->mComputedBorder.left,
                            aState->mComputedBorder.top);
-  if (!mHelper.IsScrollbarOnRight()) {
+  if (!IsScrollbarOnRight()) {
     scrollPortOrigin.x += vScrollbarActualWidth;
   }
   mHelper.mScrollPort = nsRect(scrollPortOrigin, scrollPortSize);
   return true;
 }
 
 bool
 nsHTMLScrollFrame::ScrolledContentDependsOnHeight(ScrollReflowState* aState)
@@ -4270,17 +4270,17 @@ ScrollFrameHelper::IsScrollingActive(nsD
 
 /**
  * Reflow the scroll area if it needs it and return its size. Also determine if the reflow will
  * cause any of the scrollbars to need to be reflowed.
  */
 nsresult
 nsXULScrollFrame::Layout(nsBoxLayoutState& aState)
 {
-  bool scrollbarRight = mHelper.IsScrollbarOnRight();
+  bool scrollbarRight = IsScrollbarOnRight();
   bool scrollbarBottom = true;
 
   // get the content rect
   nsRect clientRect(0,0,0,0);
   GetClientRect(clientRect);
 
   nsRect oldScrollAreaBounds = mHelper.mScrollPort;
   nsPoint oldScrollPosition = mHelper.GetLogicalScrollPosition();
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -893,16 +893,20 @@ public:
   }
   virtual void VisibilityChanged(bool aVisible) override {}
   virtual nsIFrame* GetScrollbarBox(bool aVertical) override {
     return mHelper.GetScrollbarBox(aVertical);
   }
   virtual void ScrollbarActivityStarted() const override;
   virtual void ScrollbarActivityStopped() const override;
 
+  virtual bool IsScrollbarOnRight() const override {
+    return mHelper.IsScrollbarOnRight();
+  }
+
   virtual void SetTransformingByAPZ(bool aTransforming) override {
     mHelper.SetTransformingByAPZ(aTransforming);
   }
   bool IsTransformingByAPZ() const override {
     return mHelper.IsTransformingByAPZ();
   }
   
 #ifdef DEBUG_FRAME_DUMP
@@ -1290,16 +1294,20 @@ public:
   virtual void VisibilityChanged(bool aVisible) override {}
   virtual nsIFrame* GetScrollbarBox(bool aVertical) override {
     return mHelper.GetScrollbarBox(aVertical);
   }
 
   virtual void ScrollbarActivityStarted() const override;
   virtual void ScrollbarActivityStopped() const override;
 
+  virtual bool IsScrollbarOnRight() const override {
+    return mHelper.IsScrollbarOnRight();
+  }
+
   virtual void SetTransformingByAPZ(bool aTransforming) override {
     mHelper.SetTransformingByAPZ(aTransforming);
   }
   bool IsTransformingByAPZ() const override {
     return mHelper.IsTransformingByAPZ();
   }
 
 #ifdef DEBUG_FRAME_DUMP
--- a/layout/xul/nsIScrollbarMediator.h
+++ b/layout/xul/nsIScrollbarMediator.h
@@ -72,12 +72,14 @@ public:
    */
   virtual nsIFrame* GetScrollbarBox(bool aVertical) = 0;
   /**
    * Show or hide scrollbars on 2 fingers touch.
    * Subclasses should call their ScrollbarActivity's corresponding methods.
    */
   virtual void ScrollbarActivityStarted() const = 0;
   virtual void ScrollbarActivityStopped() const = 0;
+
+  virtual bool IsScrollbarOnRight() const = 0;
 };
 
 #endif
 
--- a/layout/xul/nsListBoxBodyFrame.h
+++ b/layout/xul/nsListBoxBodyFrame.h
@@ -67,16 +67,19 @@ public:
   virtual void ThumbMoved(nsScrollbarFrame* aScrollbar,
                           int32_t aOldPos,
                           int32_t aNewPos) override;
   virtual void ScrollbarReleased(nsScrollbarFrame* aScrollbar) override {}
   virtual void VisibilityChanged(bool aVisible) override;
   virtual nsIFrame* GetScrollbarBox(bool aVertical) override;
   virtual void ScrollbarActivityStarted() const override {}
   virtual void ScrollbarActivityStopped() const override {}
+  virtual bool IsScrollbarOnRight() const override {
+    return (StyleVisibility()->mDirection == NS_STYLE_DIRECTION_LTR);
+  }
 
 
   // nsIReflowCallback
   virtual bool ReflowFinished() override;
   virtual void ReflowCallbackCanceled() override;
 
   NS_IMETHOD DoLayout(nsBoxLayoutState& aBoxLayoutState) override;
   virtual void MarkIntrinsicISizesDirty() override;
--- a/layout/xul/nsScrollbarFrame.cpp
+++ b/layout/xul/nsScrollbarFrame.cpp
@@ -164,42 +164,49 @@ nsScrollbarFrame::GetScrollbarMediator()
     }
   }
   return sbm;
 }
 
 nsresult
 nsScrollbarFrame::GetMargin(nsMargin& aMargin)
 {
+  nsresult rv = NS_ERROR_FAILURE;
   aMargin.SizeTo(0,0,0,0);
 
   if (LookAndFeel::GetInt(LookAndFeel::eIntID_UseOverlayScrollbars) != 0) {
     nsPresContext* presContext = PresContext();
     nsITheme* theme = presContext->GetTheme();
     if (theme) {
       LayoutDeviceIntSize size;
       bool isOverridable;
       theme->GetMinimumWidgetSize(presContext, this, NS_THEME_SCROLLBAR, &size,
                                   &isOverridable);
       if (IsHorizontal()) {
         aMargin.top = -presContext->DevPixelsToAppUnits(size.height);
       }
       else {
-        if (StyleVisibility()->mDirection == NS_STYLE_DIRECTION_RTL) {
-          aMargin.right = -presContext->DevPixelsToAppUnits(size.width);
-        }
-        else {
-          aMargin.left = -presContext->DevPixelsToAppUnits(size.width);
-        }
+        aMargin.left = -presContext->DevPixelsToAppUnits(size.width);
       }
-      return NS_OK;
+      rv = NS_OK;
     }
   }
 
-  return nsBox::GetMargin(aMargin);
+  if (NS_FAILED(rv)) {
+    rv = nsBox::GetMargin(aMargin);
+  }
+
+  if (NS_SUCCEEDED(rv) && !IsHorizontal()) {
+    nsIScrollbarMediator* scrollFrame = GetScrollbarMediator();
+    if (scrollFrame && !scrollFrame->IsScrollbarOnRight()) {
+      Swap(aMargin.left, aMargin.right);
+    }
+  }
+
+  return rv;
 }
 
 void
 nsScrollbarFrame::SetIncrementToLine(int32_t aDirection)
 {
   // get the scrollbar's content node
   nsIContent* content = GetContent();
   mSmoothScroll = true;
--- a/layout/xul/tree/nsTreeBodyFrame.h
+++ b/layout/xul/tree/nsTreeBodyFrame.h
@@ -149,16 +149,19 @@ public:
   virtual void ScrollbarReleased(nsScrollbarFrame* aScrollbar) override {}
   virtual void VisibilityChanged(bool aVisible) override { Invalidate(); }
   virtual nsIFrame* GetScrollbarBox(bool aVertical) override {
     ScrollParts parts = GetScrollParts();
     return aVertical ? parts.mVScrollbar : parts.mHScrollbar;
   }
   virtual void ScrollbarActivityStarted() const override;
   virtual void ScrollbarActivityStopped() const override;
+  virtual bool IsScrollbarOnRight() const override {
+    return (StyleVisibility()->mDirection == NS_STYLE_DIRECTION_LTR);
+  }
 
   // Overridden from nsIFrame to cache our pres context.
   virtual void Init(nsIContent*       aContent,
                     nsContainerFrame* aParent,
                     nsIFrame*         aPrevInFlow) override;
   virtual void DestroyFrom(nsIFrame* aDestructRoot) override;
 
   virtual nsresult GetCursor(const nsPoint& aPoint,
--- a/mobile/android/themes/core/content.css
+++ b/mobile/android/themes/core/content.css
@@ -28,21 +28,16 @@ xul|window xul|scrollbar[orient="vertica
   position: relative;
   margin-left: -8px;
   min-width: 8px;
   background-color: transparent !important;
   background-image: none !important;
   border: 0px solid transparent !important;
 }
 
-xul|window xul|scrollbar[orient="vertical"]:-moz-locale-dir(rtl) {
-  margin-left: 2px;
-  margin-right: -10px;
-}
-
 xul|window xul|scrollbar[orient="vertical"] xul|thumb {
   max-width: 6px !important;
   min-width: 6px !important;
 }
 
 xul|window xul|scrollbar[orient="horizontal"] {
   -moz-appearance: none !important;
   opacity: 0;