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

@@ -30,19 +30,14 @@ html xul|scrollbar {
   pointer-events: none;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   layout.scrollbar.side pref */
 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;

browser/themes/linux/devtools/floating-scrollbars.css

@@ -9,8 +9,10 @@ scrollbar {
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   layout.scrollbar.side pref */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -10px;
+  margin-left: -10px;
   min-width: 10px;
   max-width: 10px;
 }

browser/themes/osx/devtools/floating-scrollbars.css

@@ -10,8 +10,10 @@ scrollbar {
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   layout.scrollbar.side pref */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -8px;
+  margin-left: -8px;
   min-width: 8px;
   max-width: 8px;
 }

browser/themes/windows/devtools/floating-scrollbars.css

@@ -9,8 +9,10 @@ scrollbar {
   padding: 2px;
 }
 
+/* Scrollbar code will reset the margin to the correct side depending on
+   layout.scrollbar.side pref */
 scrollbar[orient="vertical"] {
-  -moz-margin-start: -10px;
+  margin-left: -10px;
   min-width: 10px;
   max-width: 10px;
 }

layout/generic/nsGfxScrollFrame.cpp

@@ -395,7 +395,7 @@ nsHTMLScrollFrame::TryLayout(ScrollReflowState* aState,
   aState->mShowVScrollbar = aAssumeVScroll;
   nsPoint scrollPortOrigin(aState->mComputedBorder.left,
                            aState->mComputedBorder.top);
-  if (!mHelper.IsScrollbarOnRight()) {
+  if (mHelper.GetScrollbarSide() == ScrollFrameHelper::eScrollbarOnLeft) {
     scrollPortOrigin.x += vScrollbarActualWidth;
   }
   mHelper.mScrollPort = nsRect(scrollPortOrigin, scrollPortSize);
@@ -1009,7 +1009,7 @@ ScrollFrameHelper::GetDesiredScrollbarSizes(nsBoxLayoutState* aState)
   if (mVScrollbarBox) {
     nsSize size = mVScrollbarBox->GetPrefSize(*aState);
     nsBox::AddMargin(mVScrollbarBox, size);
-    if (IsScrollbarOnRight())
+    if (GetScrollbarSide() == eScrollbarOnRight)
       result.left = size.width;
     else
       result.right = size.width;
@@ -3766,7 +3766,7 @@ ScrollFrameHelper::CreateAnonymousContent(
     nsAutoString dir;
     switch (resizeStyle) {
       case NS_STYLE_RESIZE_HORIZONTAL:
-        if (IsScrollbarOnRight()) {
+        if (GetScrollbarSide() == eScrollbarOnRight) {
           dir.AssignLiteral("right");
         }
         else {
@@ -4213,8 +4213,8 @@ ScrollFrameHelper::IsLTR() const
   return wm.IsVertical() ? wm.IsVerticalLR() : wm.IsBidiLTR();
 }
 
-bool
-ScrollFrameHelper::IsScrollbarOnRight() const
+ScrollFrameHelper::eScrollbarSide
+ScrollFrameHelper::GetScrollbarSide() const
 {
   nsPresContext *presContext = mOuter->PresContext();
 
@@ -4222,18 +4222,19 @@ ScrollFrameHelper::IsScrollbarOnRight() const
   // 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 IsLTR();
+    return IsLTR() ? eScrollbarOnRight : eScrollbarOnLeft;
   switch (presContext->GetCachedIntPref(kPresContext_ScrollbarSide)) {
     default:
     case 0: // UI directionality
-      return presContext->GetCachedIntPref(kPresContext_BidiDirection)
-             == IBMBIDI_TEXTDIRECTION_LTR;
+      return (presContext->GetCachedIntPref(kPresContext_BidiDirection)
+              == IBMBIDI_TEXTDIRECTION_LTR)
+        ? eScrollbarOnRight : eScrollbarOnLeft;
     case 1: // Document / content directionality
-      return IsLTR();
+      return IsLTR() ? eScrollbarOnRight : eScrollbarOnLeft;
     case 2: // Always right
-      return true;
+      return eScrollbarOnRight;
     case 3: // Always left
-      return false;
+      return eScrollbarOnLeft;
   }
 }
 
@@ -4271,7 +4272,8 @@ ScrollFrameHelper::IsScrollingActive(nsDisplayListBuilder* aBuilder) const
 nsresult
 nsXULScrollFrame::Layout(nsBoxLayoutState& aState)
 {
-  bool scrollbarRight = mHelper.IsScrollbarOnRight();
+  bool scrollbarRight =
+    (mHelper.GetScrollbarSide() == ScrollFrameHelper::eScrollbarOnRight);
   bool scrollbarBottom = true;
 
   // get the content rect
@@ -4738,7 +4740,7 @@ ScrollFrameHelper::LayoutScrollbars(nsBoxLayoutState& aState,
                "This should have been suppressed");
 
   bool hasResizer = HasResizer();
-  bool scrollbarOnLeft = !IsScrollbarOnRight();
+  bool scrollbarOnLeft = (GetScrollbarSide() == eScrollbarOnLeft);
 
   // place the scrollcorner
   if (mScrollCornerBox || mResizerBox) {
@@ -4804,9 +4806,14 @@ ScrollFrameHelper::LayoutScrollbars(nsBoxLayoutState& aState,
     vRect.width = aContentArea.width - mScrollPort.width;
     vRect.x = scrollbarOnLeft ? aContentArea.x : mScrollPort.XMost();
     if (mHasVerticalScrollbar) {
-      nsMargin margin;
-      mVScrollbarBox->GetMargin(margin);
-      vRect.Deflate(margin);
+      nsScrollbarFrame* scrollbar = do_QueryFrame(mVScrollbarBox);
+      NS_ASSERTION(scrollbar, "Frame must be a scrollbar");
+
+      if (scrollbar) {
+        nsMargin margin;
+        scrollbar->GetScrollbarMargin(margin, GetScrollbarSide());
+        vRect.Deflate(margin);
+      }
     }
     AdjustScrollbarRectForResizer(mOuter, presContext, vRect, hasResizer, true);
   }

layout/generic/nsGfxScrollFrame.h

@@ -307,7 +307,8 @@ public:
   nsMargin GetDesiredScrollbarSizes(nsBoxLayoutState* aState);
   nscoord GetNondisappearingScrollbarWidth(nsBoxLayoutState* aState);
   bool IsLTR() const;
-  bool IsScrollbarOnRight() const;
+  enum eScrollbarSide { eScrollbarOnLeft, eScrollbarOnRight };
+  eScrollbarSide GetScrollbarSide() const;
   bool IsScrollingActive(nsDisplayListBuilder* aBuilder) const;
   bool IsMaybeScrollingActive() const;
   bool IsProcessingAsyncScroll() const {

layout/xul/nsScrollbarFrame.cpp

@@ -167,8 +167,11 @@ nsScrollbarFrame::GetScrollbarMediator()
 }
 
 nsresult
-nsScrollbarFrame::GetMargin(nsMargin& aMargin)
+nsScrollbarFrame::GetScrollbarMargin(
+  nsMargin& aMargin,
+  mozilla::ScrollFrameHelper::eScrollbarSide aSide)
 {
+  nsresult rv = NS_ERROR_FAILURE;
   aMargin.SizeTo(0,0,0,0);
 
   if (LookAndFeel::GetInt(LookAndFeel::eIntID_UseOverlayScrollbars) != 0) {
@@ -183,18 +186,21 @@ nsScrollbarFrame::GetMargin(nsMargin& aMargin)
         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) && aSide == ScrollFrameHelper::eScrollbarOnLeft) {
+    Swap(aMargin.left, aMargin.right);
+  }
+
+  return rv;
 }
 
 void

layout/xul/nsScrollbarFrame.h

@@ -12,6 +12,7 @@
 
 #include "mozilla/Attributes.h"
 #include "nsBoxFrame.h"
+#include "nsGfxScrollFrame.h"
 
 class nsIScrollbarMediator;
 
@@ -81,7 +82,8 @@ public:
    */
   virtual bool DoesClipChildren() override { return true; }
 
-  virtual nsresult GetMargin(nsMargin& aMargin) override;
+  nsresult GetScrollbarMargin(nsMargin& aMargin,
+                              mozilla::ScrollFrameHelper::eScrollbarSide aSide);
 
   /**
    * The following three methods set the value of mIncrement when a

mobile/android/themes/core/content.css

@@ -33,11 +33,6 @@ xul|window xul|scrollbar[orient="vertical"] {
   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;