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
☠☠ backed out by 799784735f9e ☠ ☠
authorSimon Montagu <smontagu@smontagu.org>
Tue, 21 Apr 2015 11:32:01 +0300
changeset 240121 3c3337ed60a1901a4f9e9c6cd4df4d47db5a80ba
parent 240120 59d7ae27c9da172c8a8aa20db402d980754d56bd
child 240122 1777e46a19cb02c3e41279f119f3d774364760e4
push id58749
push usersmontagu@mozilla.com
push dateTue, 21 Apr 2015 08:34:51 +0000
treeherdermozilla-inbound@3c3337ed60a1 [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/nsScrollbarFrame.cpp
layout/xul/nsScrollbarFrame.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
+   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;
 }
 
 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
+   layout.scrollbar.side pref */
 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
+   layout.scrollbar.side pref */
 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
+   layout.scrollbar.side pref */
 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
@@ -390,17 +390,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 (mHelper.GetScrollbarSide() == ScrollFrameHelper::eScrollbarOnLeft) {
     scrollPortOrigin.x += vScrollbarActualWidth;
   }
   mHelper.mScrollPort = nsRect(scrollPortOrigin, scrollPortSize);
   return true;
 }
 
 bool
 nsHTMLScrollFrame::ScrolledContentDependsOnHeight(ScrollReflowState* aState)
@@ -1004,17 +1004,17 @@ ScrollFrameHelper::GetDesiredScrollbarSi
                "Must have rendering context in layout state for size "
                "computations");
 
   nsMargin result(0, 0, 0, 0);
 
   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;
   }
 
   if (mHScrollbarBox) {
     nsSize size = mHScrollbarBox->GetPrefSize(*aState);
     nsBox::AddMargin(mHScrollbarBox, size);
@@ -3761,17 +3761,17 @@ ScrollFrameHelper::CreateAnonymousConten
                                             nsIDOMNode::ELEMENT_NODE);
     NS_ENSURE_TRUE(nodeInfo, NS_ERROR_OUT_OF_MEMORY);
 
     NS_TrustedNewXULElement(getter_AddRefs(mResizerContent), nodeInfo.forget());
 
     nsAutoString dir;
     switch (resizeStyle) {
       case NS_STYLE_RESIZE_HORIZONTAL:
-        if (IsScrollbarOnRight()) {
+        if (GetScrollbarSide() == eScrollbarOnRight) {
           dir.AssignLiteral("right");
         }
         else {
           dir.AssignLiteral("left");
         }
         break;
       case NS_STYLE_RESIZE_VERTICAL:
         dir.AssignLiteral("bottom");
@@ -4208,37 +4208,38 @@ ScrollFrameHelper::IsLTR() const
         frame = rootsFrame;
     }
   }
 
   WritingMode wm = frame->GetWritingMode();
   return wm.IsVertical() ? wm.IsVerticalLR() : wm.IsBidiLTR();
 }
 
-bool
-ScrollFrameHelper::IsScrollbarOnRight() const
+ScrollFrameHelper::eScrollbarSide
+ScrollFrameHelper::GetScrollbarSide() 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 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;
   }
 }
 
 bool
 ScrollFrameHelper::IsMaybeScrollingActive() const
 {
   const nsStyleDisplay* disp = mOuter->StyleDisplay();
   if (disp && (disp->mWillChangeBitField & NS_STYLE_WILL_CHANGE_SCROLL)) {
@@ -4266,17 +4267,18 @@ 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 =
+    (mHelper.GetScrollbarSide() == ScrollFrameHelper::eScrollbarOnRight);
   bool scrollbarBottom = true;
 
   // get the content rect
   nsRect clientRect(0,0,0,0);
   GetClientRect(clientRect);
 
   nsRect oldScrollAreaBounds = mHelper.mScrollPort;
   nsPoint oldScrollPosition = mHelper.GetLogicalScrollPosition();
@@ -4733,17 +4735,17 @@ void
 ScrollFrameHelper::LayoutScrollbars(nsBoxLayoutState& aState,
                                         const nsRect& aContentArea,
                                         const nsRect& aOldScrollArea)
 {
   NS_ASSERTION(!mSupppressScrollbarUpdate,
                "This should have been suppressed");
 
   bool hasResizer = HasResizer();
-  bool scrollbarOnLeft = !IsScrollbarOnRight();
+  bool scrollbarOnLeft = (GetScrollbarSide() == eScrollbarOnLeft);
 
   // place the scrollcorner
   if (mScrollCornerBox || mResizerBox) {
     NS_PRECONDITION(!mScrollCornerBox || mScrollCornerBox->IsBoxFrame(), "Must be a box frame!");
 
     nsRect r(0, 0, 0, 0);
     if (aContentArea.x != mScrollPort.x || scrollbarOnLeft) {
       // scrollbar (if any) on left
@@ -4799,19 +4801,24 @@ ScrollFrameHelper::LayoutScrollbars(nsBo
   nsPresContext* presContext = mScrolledFrame->PresContext();
   nsRect vRect;
   if (mVScrollbarBox) {
     NS_PRECONDITION(mVScrollbarBox->IsBoxFrame(), "Must be a box frame!");
     vRect = mScrollPort;
     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);
   }
 
   nsRect hRect;
   if (mHScrollbarBox) {
     NS_PRECONDITION(mHScrollbarBox->IsBoxFrame(), "Must be a box frame!");
     hRect = mScrollPort;
--- a/layout/generic/nsGfxScrollFrame.h
+++ b/layout/generic/nsGfxScrollFrame.h
@@ -302,17 +302,18 @@ public:
   uint32_t GetScrollbarVisibility() const {
     return (mHasVerticalScrollbar ? nsIScrollableFrame::VERTICAL : 0) |
            (mHasHorizontalScrollbar ? nsIScrollableFrame::HORIZONTAL : 0);
   }
   nsMargin GetActualScrollbarSizes() const;
   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 {
     return mAsyncScroll != nullptr || mAsyncSmoothMSDScroll != nullptr;
   }
   void ResetScrollPositionForLayerPixelAlignment()
   {
     mScrollPosForLayerPixelAlignment = GetScrollPosition();
--- a/layout/xul/nsScrollbarFrame.cpp
+++ b/layout/xul/nsScrollbarFrame.cpp
@@ -162,44 +162,50 @@ nsScrollbarFrame::GetScrollbarMediator()
     if (f && f->GetContent() == mScrollbarMediator) {
       return do_QueryFrame(f);
     }
   }
   return sbm;
 }
 
 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) {
     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) && aSide == ScrollFrameHelper::eScrollbarOnLeft) {
+    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/nsScrollbarFrame.h
+++ b/layout/xul/nsScrollbarFrame.h
@@ -7,16 +7,17 @@
 // nsScrollbarFrame
 //
 
 #ifndef nsScrollbarFrame_h__
 #define nsScrollbarFrame_h__
 
 #include "mozilla/Attributes.h"
 #include "nsBoxFrame.h"
+#include "nsGfxScrollFrame.h"
 
 class nsIScrollbarMediator;
 
 nsIFrame* NS_NewScrollbarFrame(nsIPresShell* aPresShell, nsStyleContext* aContext);
 
 class nsScrollbarFrame : public nsBoxFrame
 {
 public:
@@ -76,17 +77,18 @@ public:
    * Treat scrollbars as clipping their children; overflowing children
    * will not be allowed to set an overflow rect on this
    * frame. This means that when the scroll code decides to hide a
    * scrollframe by setting its height or width to zero, that will
    * hide the children too.
    */
   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
    * scrollbar button is pressed.
    */
   void SetIncrementToLine(int32_t aDirection);
   void SetIncrementToPage(int32_t aDirection);
   void SetIncrementToWhole(int32_t aDirection);
--- 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;