Attachment 372010 Details for Bug 198816 – Patch

[patch] Patch

bug-198816-20190612175157.patch (text/plain), 9.65 KB, created by Simon Fraser (smfr) on 2019-06-12 17:51:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Simon Fraser (smfr)

Created: 2019-06-12 17:51:58 PDT

Size: 9.65 KB

patch

obsolete

>Subversion Revision: 246343
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f51216e97256723696e79470688add08117f47ff..49c1738bee88166a2525333a92e7c56187f88d87 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+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 NOBODY (OOPS!).
>+
>+        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-11  Timothy Hatcher  <timothy@apple.com>
> 
>         Flash when tapping compose button after switching to/from dark mode without restarting Mail.
>diff --git a/Source/WebCore/rendering/RenderBox.cpp b/Source/WebCore/rendering/RenderBox.cpp
>index c71e73cf723475c2e28c5ce45c005cdc7d00910c..c995bfd3d734ae43c575feccf0e86a58ea0d16fb 100644
>--- a/Source/WebCore/rendering/RenderBox.cpp
>+++ b/Source/WebCore/rendering/RenderBox.cpp
>@@ -658,6 +658,16 @@ RoundedRect::Radii RenderBox::borderRadii() const
>     return style.getRoundedBorderFor(bounds).radii();
> }
> 
>+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
> {
>     return { contentBoxLocation(), contentSize() };
>diff --git a/Source/WebCore/rendering/RenderBox.h b/Source/WebCore/rendering/RenderBox.h
>index b7865087f2f63497a3335bde412f1e1384e62036..b2afd00acf5baa60f3ca8466a7bcc6dc3cb549d9 100644
>--- a/Source/WebCore/rendering/RenderBox.h
>+++ b/Source/WebCore/rendering/RenderBox.h
>@@ -219,7 +219,7 @@ public:
> 
>     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()); }
> 
>     // IE extensions. Used to calculate offsetWidth/Height.  Overridden by inlines (RenderFlow)
>diff --git a/Source/WebCore/rendering/RenderLayerBacking.cpp b/Source/WebCore/rendering/RenderLayerBacking.cpp
>index e3b6694820433942ed4c16a8a81b57ac13bf0a8e..c8208329e7970e2dd782803307207ad4c05851a8 100644
>--- a/Source/WebCore/rendering/RenderLayerBacking.cpp
>+++ b/Source/WebCore/rendering/RenderLayerBacking.cpp
>@@ -1216,11 +1216,11 @@ void RenderLayerBacking::updateGeometry()
>     if (m_scrollContainerLayer) {
>         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())));
> 
>         ScrollOffset scrollOffset = m_owningLayer.scrollOffset();
>@@ -1230,12 +1230,12 @@ void RenderLayerBacking::updateGeometry()
> #endif
> 
>         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()));
>         }
> 
>         bool paddingBoxOffsetChanged = oldScrollingLayerOffset != m_scrollContainerLayer->offsetFromRenderer();
>@@ -1246,7 +1246,7 @@ void RenderLayerBacking::updateGeometry()
> 
>         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();
>     }
>diff --git a/Source/WebCore/rendering/RenderListBox.cpp b/Source/WebCore/rendering/RenderListBox.cpp
>index 44c6d1f693c9777b302ccf78a3af46f9c0afa99e..a696bf12c715e2208770da7b85dca09eb9e66d49 100644
>--- a/Source/WebCore/rendering/RenderListBox.cpp
>+++ b/Source/WebCore/rendering/RenderListBox.cpp
>@@ -804,8 +804,6 @@ LayoutRect RenderListBox::controlClipRect(const LayoutPoint& additionalOffset) c
>     // Clip against the padding box, to give <option>s and overlay scrollbar some extra space
>     // to get painted.
>     LayoutRect clipRect = paddingBoxRect();
>-    if (shouldPlaceBlockDirectionScrollbarOnLeft())
>-        clipRect.move(m_vBar->occupiedWidth(), 0);
>     clipRect.moveBy(additionalOffset);
>     return clipRect;
> }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 2bdb40660a5b90eef746a752cf35a6f291edbc04..ce1e4ab7d2604c048ca6a9243094fe1fa267ca0c 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+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 NOBODY (OOPS!).
>+
>+        * compositing/geometry/rtl-overflow-scroll-expected.html: Added.
>+        * compositing/geometry/rtl-overflow-scroll.html: Added.
>+
> 2019-06-11  Commit Queue  <commit-queue@webkit.org>
> 
>         Unreviewed, rolling out r246320.
>diff --git a/LayoutTests/compositing/geometry/rtl-overflow-scroll-expected.html b/LayoutTests/compositing/geometry/rtl-overflow-scroll-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..74d5bc959498aad8c8cfee4ddee25e7cb18792ea
>--- /dev/null
>+++ b/LayoutTests/compositing/geometry/rtl-overflow-scroll-expected.html
>@@ -0,0 +1,37 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <style>
>+        .scroller {
>+            direction: rtl;
>+            width: 300px;
>+            height: 300px;
>+            border: 5px solid black;
>+            padding: 2px;
>+            box-sizing: border-box;
>+            overflow: hidden;
>+        }
>+        
>+        .contents {
>+            width: 200%;
>+            height: 200%;
>+            background-color: green;
>+        }
>+        
>+        .scrollbar-hider {
>+            position: absolute;
>+            top: 8px;
>+            left: 13px;
>+            width: 16px;
>+            height: 300px;
>+            background-color: gray;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <div class="scroller">
>+        <div class="contents"></div>
>+    </div>
>+    <div class="scrollbar-hider"></div>
>+</body>
>+</html>
>diff --git a/LayoutTests/compositing/geometry/rtl-overflow-scroll.html b/LayoutTests/compositing/geometry/rtl-overflow-scroll.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..dedd8f9a4a1a9510f1e71c2d01e7b0f256b3917e
>--- /dev/null
>+++ b/LayoutTests/compositing/geometry/rtl-overflow-scroll.html
>@@ -0,0 +1,38 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+    <style>
>+        .scroller {
>+            direction: rtl;
>+            width: 300px;
>+            height: 300px;
>+            border: 5px solid black;
>+            padding: 2px;
>+            box-sizing: border-box;
>+            overflow-y: scroll;
>+            overflow-x: hidden;
>+        }
>+        
>+        .contents {
>+            width: 200%;
>+            height: 200%;
>+            background-color: green;
>+        }
>+        
>+        .scrollbar-hider {
>+            position: absolute;
>+            top: 8px;
>+            left: 13px;
>+            width: 16px;
>+            height: 300px;
>+            background-color: gray;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <div class="scroller">
>+        <div class="contents"></div>
>+    </div>
>+    <div class="scrollbar-hider"></div>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 198816: 372010