Changeset 246389 in webkit

Timestamp:

Jun 12, 2019, 8:21:36 PM (7 years ago)

Author:

Simon Fraser

Message:

paddingBoxRect() is wrong with RTL scrollbars on the left
​https://bugs.webkit.org/show_bug.cgi?id=198816

Reviewed by Jon Lee.

Source/WebCore:

RenderBox::paddingBoxRect() needs to offset the left side of the box for the
vertical scrollbar, if it's placed on the left.

Test: compositing/geometry/rtl-overflow-scroll.html

• rendering/RenderBox.cpp:

(WebCore::RenderBox::paddingBoxRect const):

• rendering/RenderBox.h:

(WebCore::RenderBox::paddingBoxRect const): Deleted.

• rendering/RenderLayerBacking.cpp:

(WebCore::RenderLayerBacking::updateGeometry):

• rendering/RenderListBox.cpp:

(WebCore::RenderListBox::controlClipRect const):

LayoutTests:

• compositing/geometry/rtl-overflow-scroll-expected.html: Added.
• compositing/geometry/rtl-overflow-scroll.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/geometry/rtl-overflow-scroll-expected.html (added)
• LayoutTests/compositing/geometry/rtl-overflow-scroll.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderBox.h (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerBacking.cpp (modified) (3 diffs)
• Source/WebCore/rendering/RenderListBox.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-06-12  Simon Fraser  <simon.fraser@apple.com>
+
+        paddingBoxRect() is wrong with RTL scrollbars on the left
+        https://bugs.webkit.org/show_bug.cgi?id=198816
+
+        Reviewed by Jon Lee.
+
+        * compositing/geometry/rtl-overflow-scroll-expected.html: Added.
+        * compositing/geometry/rtl-overflow-scroll.html: Added.
+
 2019-06-12  Eric Carlson  <eric.carlson@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-12  Simon Fraser  <simon.fraser@apple.com>
+
+        paddingBoxRect() is wrong with RTL scrollbars on the left
+        https://bugs.webkit.org/show_bug.cgi?id=198816
+
+        Reviewed by Jon Lee.
+
+        RenderBox::paddingBoxRect() needs to offset the left side of the box for the
+        vertical scrollbar, if it's placed on the left.
+
+        Test: compositing/geometry/rtl-overflow-scroll.html
+
+        * rendering/RenderBox.cpp:
+        (WebCore::RenderBox::paddingBoxRect const):
+        * rendering/RenderBox.h:
+        (WebCore::RenderBox::paddingBoxRect const): Deleted.
+        * rendering/RenderLayerBacking.cpp:
+        (WebCore::RenderLayerBacking::updateGeometry):
+        * rendering/RenderListBox.cpp:
+        (WebCore::RenderListBox::controlClipRect const):
+
 2019-06-12  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebCore/rendering/RenderBox.cpp

@@ -659 +659 @@
 }
 
+LayoutRect RenderBox::paddingBoxRect() const
+{
+    auto verticalScrollbarWidth = this->verticalScrollbarWidth();
+    LayoutUnit offsetForScrollbar = shouldPlaceBlockDirectionScrollbarOnLeft() ? verticalScrollbarWidth : 0;
+
+    return LayoutRect(borderLeft() + offsetForScrollbar, borderTop(),
+        width() - borderLeft() - borderRight() - verticalScrollbarWidth,
+        height() - borderTop() - borderBottom() - horizontalScrollbarHeight());
+}
+
 LayoutRect RenderBox::contentBoxRect() const
 {

trunk/Source/WebCore/rendering/RenderBox.h

@@ -220 +220 @@
     LayoutUnit paddingBoxWidth() const { return width() - borderLeft() - borderRight() - verticalScrollbarWidth(); }
     LayoutUnit paddingBoxHeight() const { return height() - borderTop() - borderBottom() - horizontalScrollbarHeight(); }
-    LayoutRect paddingBoxRect() const { return LayoutRect(borderLeft(), borderTop(), paddingBoxWidth(), paddingBoxHeight()); }
+    LayoutRect paddingBoxRect() const;
     LayoutRect paddingBoxRectIncludingScrollbar() const { return LayoutRect(borderLeft(), borderTop(), width() - borderLeft() - borderRight(), height() - borderTop() - borderBottom()); }
 

trunk/Source/WebCore/rendering/RenderLayerBacking.cpp

@@ -1217 +1217 @@
         ASSERT(m_scrolledContentsLayer);
         auto& renderBox = downcast<RenderBox>(renderer());
-        LayoutRect paddingBoxIncludingScrollbar = renderBox.paddingBoxRectIncludingScrollbar();
+        LayoutRect paddingBox = renderBox.paddingBoxRect();
         LayoutRect parentLayerBounds = clippingLayer() ? clippingBox : compositedBounds();
 
         // FIXME: need to do some pixel snapping here.
-        m_scrollContainerLayer->setPosition(FloatPoint(paddingBoxIncludingScrollbar.location() - parentLayerBounds.location()));
+        m_scrollContainerLayer->setPosition(FloatPoint(paddingBox.location() - parentLayerBounds.location()));
         m_scrollContainerLayer->setSize(roundedIntSize(LayoutSize(renderBox.paddingBoxWidth(), renderBox.paddingBoxHeight())));
 
@@ -1231 +1231 @@
 
         FloatSize oldScrollingLayerOffset = m_scrollContainerLayer->offsetFromRenderer();
-        m_scrollContainerLayer->setOffsetFromRenderer(toFloatSize(paddingBoxIncludingScrollbar.location()));
+        m_scrollContainerLayer->setOffsetFromRenderer(toFloatSize(paddingBox.location()));
 
         if (m_childClippingMaskLayer) {
             m_childClippingMaskLayer->setPosition(m_scrollContainerLayer->position());
             m_childClippingMaskLayer->setSize(m_scrollContainerLayer->size());
-            m_childClippingMaskLayer->setOffsetFromRenderer(toFloatSize(paddingBoxIncludingScrollbar.location()));
+            m_childClippingMaskLayer->setOffsetFromRenderer(toFloatSize(paddingBox.location()));
         }
 
@@ -1247 +1247 @@
         m_scrolledContentsLayer->setSize(scrollSize);
         m_scrolledContentsLayer->setScrollOffset(scrollOffset, GraphicsLayer::DontSetNeedsDisplay);
-        m_scrolledContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBoxIncludingScrollbar.location()), GraphicsLayer::DontSetNeedsDisplay);
+        m_scrolledContentsLayer->setOffsetFromRenderer(toLayoutSize(paddingBox.location()), GraphicsLayer::DontSetNeedsDisplay);
         
         adjustTiledBackingCoverage();

trunk/Source/WebCore/rendering/RenderListBox.cpp

@@ -805 +805 @@
     // to get painted.
     LayoutRect clipRect = paddingBoxRect();
-    if (shouldPlaceBlockDirectionScrollbarOnLeft())
-        clipRect.move(m_vBar->occupiedWidth(), 0);
     clipRect.moveBy(additionalOffset);
     return clipRect;