Making multiple select element RTL direction aware

Source/core/rendering/RenderListBox.cpp

Index: Source/core/rendering/RenderListBox.cpp
diff --git a/Source/core/rendering/RenderListBox.cpp b/Source/core/rendering/RenderListBox.cpp
index b4b499ab1251fc97689c44501487c752bc1490e1..a783c926e17a124ddd5aff450e97e665ded3694a 100644
--- a/Source/core/rendering/RenderListBox.cpp
+++ b/Source/core/rendering/RenderListBox.cpp
@@ -274,7 +274,7 @@ int RenderListBox::baselinePosition(FontBaseline baselineType, bool firstLine, L
 
 LayoutRect RenderListBox::itemBoundingBoxRect(const LayoutPoint& additionalOffset, int index)
 {
-    return LayoutRect(additionalOffset.x() + borderLeft() + paddingLeft(),
+    return LayoutRect(additionalOffset.x() + borderLeft() + paddingLeft() + (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? m_vBar->width() : 0),

[eseidel, 2013/10/22 05:59:21]

Same here.  Can we help explain to the reader what this is doing?

[pals, 2013/10/22 08:45:19]

Done.

         additionalOffset.y() + borderTop() + paddingTop() + itemHeight() * (index - m_indexOffset),
         contentWidth(), itemHeight());
 }
@@ -348,10 +348,20 @@ void RenderListBox::addFocusRingRects(Vector<IntRect>& rects, const LayoutPoint&
     }
 }
 
+int RenderListBox::scrollbarLeft() const
+{
+    int scrollbarLeft = 0;
+    if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
+        scrollbarLeft = borderLeft();
+    else
+        scrollbarLeft = width() - borderRight() - m_vBar->width();
+    return scrollbarLeft;
+}
+
 void RenderListBox::paintScrollbar(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     if (m_vBar) {
-        IntRect scrollRect = pixelSnappedIntRect(paintOffset.x() + width() - borderRight() - m_vBar->width(),
+        IntRect scrollRect = pixelSnappedIntRect(paintOffset.x() + scrollbarLeft(),
             paintOffset.y() + borderTop(),
             m_vBar->width(),
             height() - (borderTop() + borderBottom()));
@@ -461,7 +471,7 @@ bool RenderListBox::isPointInOverflowControl(HitTestResult& result, const Layout
     if (!m_vBar || !m_vBar->shouldParticipateInHitTesting())
         return false;
 
-    LayoutRect vertRect(accumulatedOffset.x() + width() - borderRight() - m_vBar->width(),
+    LayoutRect vertRect(accumulatedOffset.x() + scrollbarLeft(),
                         accumulatedOffset.y() + borderTop(),
                         m_vBar->width(),
                         height() - borderTop() - borderBottom());
@@ -482,7 +492,7 @@ int RenderListBox::listIndexAtOffset(const LayoutSize& offset)
         return -1;
 
     int scrollbarWidth = m_vBar ? m_vBar->width() : 0;
-    if (offset.width() < borderLeft() + paddingLeft() || offset.width() > width() - borderRight() - paddingRight() - scrollbarWidth)
+    if (offset.width() < borderLeft() + paddingLeft() + (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? scrollbarWidth : 0) || offset.width() > width() - borderRight() - paddingRight() - (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft() ? 0 : scrollbarWidth))

[eseidel, 2013/10/22 05:59:21]

This is pretty ugly.  Can we use local variables to make this more
understandable?

[pals, 2013/10/22 08:45:19]

Done.

         return -1;
 
     int newOffset = (offset.height() - borderTop() - paddingTop()) / itemHeight() + m_indexOffset;
@@ -713,7 +723,10 @@ bool RenderListBox::nodeAtPoint(const HitTestRequest& request, HitTestResult& re
 LayoutRect RenderListBox::controlClipRect(const LayoutPoint& additionalOffset) const
 {
     LayoutRect clipRect = contentBoxRect();
-    clipRect.moveBy(additionalOffset);
+    if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
+        clipRect.moveBy(additionalOffset + LayoutPoint(m_vBar->width(), 0));
+    else
+        clipRect.moveBy(additionalOffset);
     return clipRect;
 }
 
@@ -726,7 +739,10 @@ bool RenderListBox::isActive() const
 void RenderListBox::invalidateScrollbarRect(Scrollbar* scrollbar, const IntRect& rect)
 {
     IntRect scrollRect = rect;
-    scrollRect.move(width() - borderRight() - scrollbar->width(), borderTop());
+    if (style()->shouldPlaceBlockDirectionScrollbarOnLogicalLeft())
+        scrollRect.move(borderLeft(), borderTop());
+    else
+        scrollRect.move(width() - borderRight() - scrollbar->width(), borderTop());
     repaintRectangle(scrollRect);
 }
 
@@ -738,9 +754,8 @@ IntRect RenderListBox::convertFromScrollbarToContainingView(const Scrollbar* scr
 
     IntRect rect = scrollbarRect;
 
-    int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
-    rect.move(scrollbarLeft, scrollbarTop);
+    rect.move(scrollbarLeft(), scrollbarTop);
 
     return view->frameView()->convertFromRenderer(this, rect);
 }
@@ -753,9 +768,8 @@ IntRect RenderListBox::convertFromContainingViewToScrollbar(const Scrollbar* scr
 
     IntRect rect = view->frameView()->convertToRenderer(this, parentRect);
 
-    int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
-    rect.move(-scrollbarLeft, -scrollbarTop);
+    rect.move(-scrollbarLeft(), -scrollbarTop);
     return rect;
 }
 
@@ -767,9 +781,8 @@ IntPoint RenderListBox::convertFromScrollbarToContainingView(const Scrollbar* sc
 
     IntPoint point = scrollbarPoint;
 
-    int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
-    point.move(scrollbarLeft, scrollbarTop);
+    point.move(scrollbarLeft(), scrollbarTop);
 
     return view->frameView()->convertFromRenderer(this, point);
 }
@@ -782,9 +795,8 @@ IntPoint RenderListBox::convertFromContainingViewToScrollbar(const Scrollbar* sc
 
     IntPoint point = view->frameView()->convertToRenderer(this, parentPoint);
 
-    int scrollbarLeft = width() - borderRight() - scrollbar->width();
     int scrollbarTop = borderTop();
-    point.move(-scrollbarLeft, -scrollbarTop);
+    point.move(-scrollbarLeft(), -scrollbarTop);
     return point;
 }