Changeset 245317 in webkit

Timestamp:

May 14, 2019 9:57:56 PM (5 years ago)

Author:

Wenson Hsieh

Message:

Missing cursor/caret showing in search field on google.com
​https://bugs.webkit.org/show_bug.cgi?id=197862
<rdar://problem/50291989>

Reviewed by Simon Fraser.

Source/WebCore:

In this bug, the search field is inside of a fixed position container, which is inside of an empty "overflow:
hidden" form element (the new layout test demonstrates a simple version of this). The layer of the fixed
position container's renderer has an overflow clipping layer of itself, and its clipping rect is non-empty, so
the heuristic initially identifies the layer as not fully clipped. However, as the heuristic ascends the
RenderLayer tree, it then finds the layer for the "overflow: hidden" form element's renderer; this layer is
completely clipped, which causes the heuristic to incorrectly believe that the editable element is completely
clipped.

To fix the bug, this patch reworks the clipping portion of the heuristic, such that we no longer need to ascend
the layer tree. Instead of computing the clip rect relative to the nearest ancestor that has an overflow clip
and then walking up the layer tree repeating this process, simply compute the clip rect relative to RenderView's
layer, and then walk up to the parent frame and repeat if necessary.

Test: editing/selection/ios/do-not-hide-selection-in-visible-field.html

• rendering/RenderLayer.cpp:

(WebCore::RenderLayer::isTransparentOrFullyClippedRespectingParentFrames const):

LayoutTests:

Add a new layout test that represents a reduced test case version of google.com's search field.

• editing/selection/ios/do-not-hide-selection-in-visible-field.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field-expected.txt (added)
• LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-05-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Missing cursor/caret showing in search field on google.com
+        https://bugs.webkit.org/show_bug.cgi?id=197862
+        <rdar://problem/50291989>
+
+        Reviewed by Simon Fraser.
+
+        Add a new layout test that represents a reduced test case version of google.com's search field.
+
+        * editing/selection/ios/do-not-hide-selection-in-visible-field.html: Added.
+
 2019-05-14  Megan Gardner  <megan_gardner@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-05-14  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Missing cursor/caret showing in search field on google.com
+        https://bugs.webkit.org/show_bug.cgi?id=197862
+        <rdar://problem/50291989>
+
+        Reviewed by Simon Fraser.
+
+        In this bug, the search field is inside of a fixed position container, which is inside of an empty "overflow:
+        hidden" form element (the new layout test demonstrates a simple version of this). The layer of the fixed
+        position container's renderer has an overflow clipping layer of itself, and its clipping rect is non-empty, so
+        the heuristic initially identifies the layer as not fully clipped. However, as the heuristic ascends the
+        RenderLayer tree, it then finds the layer for the "overflow: hidden" form element's renderer; this layer is
+        completely clipped, which causes the heuristic to incorrectly believe that the editable element is completely
+        clipped.
+
+        To fix the bug, this patch reworks the clipping portion of the heuristic, such that we no longer need to ascend
+        the layer tree. Instead of computing the clip rect relative to the nearest ancestor that has an overflow clip
+        and then walking up the layer tree repeating this process, simply compute the clip rect relative to RenderView's
+        layer, and then walk up to the parent frame and repeat if necessary.
+
+        Test: editing/selection/ios/do-not-hide-selection-in-visible-field.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::isTransparentOrFullyClippedRespectingParentFrames const):
+
 2019-05-14  Andy Estes  <aestes@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -6802 +6802 @@
     }
 
-    RenderLayer* enclosingClipLayer = nullptr;
-    for (auto* layer = this; layer; layer = enclosingClipLayer ? enclosingClipLayer->parent() : enclosingFrameRenderLayer(*layer)) {
-        enclosingClipLayer = layer->enclosingOverflowClipLayer(IncludeSelfOrNot::IncludeSelf);
-        if (!enclosingClipLayer)
-            continue;
+    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;
 
         LayoutRect layerBounds;
         ClipRect backgroundRect;
         ClipRect foregroundRect;
-        layer->calculateRects({ enclosingClipLayer, TemporaryClipRects }, LayoutRect::infiniteRect(), layerBounds, backgroundRect, foregroundRect, layer->offsetFromAncestor(enclosingClipLayer));
-        if (backgroundRect.isEmpty())
+        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;
     }