Changeset 249339 in webkit

Timestamp:

Aug 30, 2019 12:28:57 PM (5 years ago)

Author:

Wenson Hsieh

Message:

Caret does not appear in text field inside a transformed, overflow: hidden container
​https://bugs.webkit.org/show_bug.cgi?id=201317
<rdar://problem/54859264>

Reviewed by Simon Fraser.

Source/WebCore:

This patch refactors the heuristic for determining whether to suppress selection gestures and UI in a way that
fixes the corner case encountered in this bug. To understand why this test case fails with our existing
heuristic, consider the below test case.

Let's say we have an input field inside an "overflow: hidden;" container, which is positioned in such a way that
it is completely clipped by its enclosing container which is also "overflow: hidden". Our existing logic would
appropriately identify this as a hidden editable element.

However, let's now apply a transform to the input field's closest "overflow: hidden" ancestor, such that the
field is now visible. Since RenderLayer::offsetFromAncestor doesn't take transforms into account when we try to
find the offset of the "overflow: hidden" layer relative to the root view, we end up passing an offsetFromRoot
of (0, 100vw) to RenderLayer::calculateClipRects, which computes a background clip rect of (0, 0, 100vw, 100vh).

This means that at the end of RenderLayer::calculateClipRects, we end up intersecting the background clip rect
(0, 0, 100vw, 100vh) against (100vw, 0, 100vw, 100vh), which results in the empty rect, and subsequently makes
us believe we're editing a hidden editable element.

Instead of tacking on more logic to isTransparentOrFullyClippedRespectingParentFrames, we can fix this by using
RenderObject::computeVisibleRectInContainer instead, performing a similar walk up the render tree to compute the
visible rect of each focused element or subframe relative to its root. This is capable of taking transforms into
account. See comments below for more details.

Test: editing/selection/ios/show-selection-in-transformed-container-2.html

• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::isTransparentRespectingParentFrames const):

Split out isTransparentOrFullyClippedRespectingParentFrames into two methods: RenderLayer's
isTransparentRespectingParentFrames, and RenderObject's hasNonEmptyVisibleRectRespectingParentFrames. The
transparency check starts at the enclosing layer and walks up the layer tree, while the non-empty visible rect
check looks for renderers that are completely empty relative to their root views.

• rendering/RenderLayer.h:
• rendering/RenderObject.cpp:

(WebCore::RenderObject::hasNonEmptyVisibleRectRespectingParentFrames const):

Rewrite logic for detecting completely clipped editable areas (that formerly lived in
isTransparentOrFullyClippedRespectingParentFrames) to use computeVisibleRectInContainer instead.

• rendering/RenderObject.h:

Source/WebKit:

Adjust isTransparentOrFullyClipped to use the new methods in RenderLayer and RenderObject. See WebCore ChangeLog
for more details.

• WebProcess/WebPage/ios/WebPageIOS.mm:

(WebKit::WebPage::isTransparentOrFullyClipped const):

LayoutTests:

Add a new layout test that covers this scenario. See WebCore ChangeLog for additional detail.

• editing/selection/ios/show-selection-in-transformed-container-2-expected.txt: Added.
• editing/selection/ios/show-selection-in-transformed-container-2.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/ios/show-selection-in-transformed-container-2-expected.txt (added)
• LayoutTests/editing/selection/ios/show-selection-in-transformed-container-2.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (2 diffs)
• Source/WebCore/rendering/RenderLayer.h (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderObject.h (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-08-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Caret does not appear in text field inside a transformed, overflow: hidden container
+        https://bugs.webkit.org/show_bug.cgi?id=201317
+        <rdar://problem/54859264>
+
+        Reviewed by Simon Fraser.
+
+        Add a new layout test that covers this scenario. See WebCore ChangeLog for additional detail.
+
+        * editing/selection/ios/show-selection-in-transformed-container-2-expected.txt: Added.
+        * editing/selection/ios/show-selection-in-transformed-container-2.html: Added.
+
 2019-08-30  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-08-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Caret does not appear in text field inside a transformed, overflow: hidden container
+        https://bugs.webkit.org/show_bug.cgi?id=201317
+        <rdar://problem/54859264>
+
+        Reviewed by Simon Fraser.
+
+        This patch refactors the heuristic for determining whether to suppress selection gestures and UI in a way that
+        fixes the corner case encountered in this bug. To understand why this test case fails with our existing
+        heuristic, consider the below test case.
+
+        Let's say we have an input field inside an "overflow: hidden;" container, which is positioned in such a way that
+        it is completely clipped by its enclosing container which is also "overflow: hidden". Our existing logic would
+        appropriately identify this as a hidden editable element.
+
+        However, let's now apply a transform to the input field's closest "overflow: hidden" ancestor, such that the
+        field is now visible. Since RenderLayer::offsetFromAncestor doesn't take transforms into account when we try to
+        find the offset of the "overflow: hidden" layer relative to the root view, we end up passing an offsetFromRoot
+        of (0, 100vw) to RenderLayer::calculateClipRects, which computes a background clip rect of (0, 0, 100vw, 100vh).
+
+        This means that at the end of RenderLayer::calculateClipRects, we end up intersecting the background clip rect
+        (0, 0, 100vw, 100vh) against (100vw, 0, 100vw, 100vh), which results in the empty rect, and subsequently makes
+        us believe we're editing a hidden editable element.
+
+        Instead of tacking on more logic to isTransparentOrFullyClippedRespectingParentFrames, we can fix this by using
+        RenderObject::computeVisibleRectInContainer instead, performing a similar walk up the render tree to compute the
+        visible rect of each focused element or subframe relative to its root. This is capable of taking transforms into
+        account. See comments below for more details.
+
+        Test: editing/selection/ios/show-selection-in-transformed-container-2.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::isTransparentRespectingParentFrames const):
+
+        Split out isTransparentOrFullyClippedRespectingParentFrames into two methods: RenderLayer's
+        isTransparentRespectingParentFrames, and RenderObject's hasNonEmptyVisibleRectRespectingParentFrames. The
+        transparency check starts at the enclosing layer and walks up the layer tree, while the non-empty visible rect
+        check looks for renderers that are completely empty relative to their root views.
+
+        * rendering/RenderLayer.h:
+        * rendering/RenderObject.cpp:
+        (WebCore::RenderObject::hasNonEmptyVisibleRectRespectingParentFrames const):
+
+        Rewrite logic for detecting completely clipped editable areas (that formerly lived in
+        isTransparentOrFullyClippedRespectingParentFrames) to use computeVisibleRectInContainer instead.
+
+        * rendering/RenderObject.h:
+
 2019-08-30  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -6836 +6836 @@
 }
 
-bool RenderLayer::isTransparentOrFullyClippedRespectingParentFrames() const
+bool RenderLayer::isTransparentRespectingParentFrames() const
 {
     static const double minimumVisibleOpacity = 0.01;
@@ -6844 +6844 @@
         currentOpacity *= layer->renderer().style().opacity();
         if (currentOpacity < minimumVisibleOpacity)
-            return true;
-    }
-
-    auto hasEmptyClipRect = [] (const RenderLayer& layer) -> bool {
-        auto* frameView = layer.renderer().document().view();
-        if (!frameView)
-            return false;
-
-        auto* renderView = frameView->renderView();
-        if (!renderView)
-            return false;
-
-        auto* renderViewLayer = renderView->layer();
-        if (!renderViewLayer)
-            return false;
-
-        if (is<HTMLFrameOwnerElement>(layer.renderer().element()) && layer.visibleSize().isEmpty())
-            return true;
-
-        LayoutRect layerBounds;
-        ClipRect backgroundRect;
-        ClipRect foregroundRect;
-        layer.calculateRects({ renderViewLayer, TemporaryClipRects }, LayoutRect::infiniteRect(), layerBounds, backgroundRect, foregroundRect, layer.offsetFromAncestor(renderViewLayer));
-        return backgroundRect.isEmpty();
-    };
-
-    for (auto* layer = this; layer; layer = enclosingFrameRenderLayer(*layer)) {
-        if (hasEmptyClipRect(*layer))
             return true;
     }

trunk/Source/WebCore/rendering/RenderLayer.h

@@ -898 +898 @@
     bool paintingFrequently() const { return m_paintFrequencyTracker.paintingFrequently(); }
 
-    WEBCORE_EXPORT bool isTransparentOrFullyClippedRespectingParentFrames() const;
+    WEBCORE_EXPORT bool isTransparentRespectingParentFrames() const;
 
     void invalidateEventRegion();

trunk/Source/WebCore/rendering/RenderObject.cpp

@@ -1903 +1903 @@
 }
 
+bool RenderObject::hasNonEmptyVisibleRectRespectingParentFrames() const
+{
+    auto enclosingFrameRenderer = [] (const RenderObject& renderer) {
+        auto* ownerElement = renderer.document().ownerElement();
+        return ownerElement ? ownerElement->renderer() : nullptr;
+    };
+
+    auto hasEmptyVisibleRect = [] (const RenderObject& renderer) {
+        VisibleRectContext context { false, false, { VisibleRectContextOption::UseEdgeInclusiveIntersection, VisibleRectContextOption::ApplyCompositedClips }};
+        auto& box = renderer.enclosingBoxModelObject();
+        auto clippedBounds = box.computeVisibleRectInContainer(box.borderBoundingBox(), &box.view(), context);
+        return !clippedBounds || clippedBounds->isEmpty();
+    };
+
+    for (auto* renderer = this; renderer; renderer = enclosingFrameRenderer(*renderer)) {
+        if (hasEmptyVisibleRect(*renderer))
+            return true;
+    }
+
+    return false;
+}
+
 #if ENABLE(TREE_DEBUGGING)
 

trunk/Source/WebCore/rendering/RenderObject.h

@@ -684 +684 @@
     virtual Optional<LayoutRect> computeVisibleRectInContainer(const LayoutRect&, const RenderLayerModelObject* repaintContainer, VisibleRectContext) const;
     virtual Optional<FloatRect> computeFloatVisibleRectInContainer(const FloatRect&, const RenderLayerModelObject* repaintContainer, VisibleRectContext) const;
+
+    WEBCORE_EXPORT bool hasNonEmptyVisibleRectRespectingParentFrames() const;
 
     virtual unsigned int length() const { return 1; }

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-30  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Caret does not appear in text field inside a transformed, overflow: hidden container
+        https://bugs.webkit.org/show_bug.cgi?id=201317
+        <rdar://problem/54859264>
+
+        Reviewed by Simon Fraser.
+
+        Adjust isTransparentOrFullyClipped to use the new methods in RenderLayer and RenderObject. See WebCore ChangeLog
+        for more details.
+
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::isTransparentOrFullyClipped const):
+
 2019-08-30  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

@@ -198 +198 @@
 
     auto* enclosingLayer = renderer->enclosingLayer();
-    return enclosingLayer && enclosingLayer->isTransparentOrFullyClippedRespectingParentFrames();
+    if (enclosingLayer && enclosingLayer->isTransparentRespectingParentFrames())
+        return true;
+
+    return renderer->hasNonEmptyVisibleRectRespectingParentFrames();
 }