Attachment 369857 Details for Bug 197862 – Patch

[patch] Patch

bug-197862-20190514091342.patch (text/plain), 7.76 KB, created by Wenson Hsieh on 2019-05-14 09:13:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-14 09:13:42 PDT

Size: 7.76 KB

patch

obsolete

>Subversion Revision: 245270
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ec1fd0759b74f1f95d3094c89cca1bacff95fbfe..d2a0e639b1701ee4a9a4ccdfb11a692fbd096115 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-05-13  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 NOBODY (OOPS!).
>+
>+        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-13  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, build fix after 245258, missing ThreadSpecific.h include
>diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp
>index 7a1dc38fcf2de78a320e5fc58704156770114d93..eb83566a2b701700a1c0c64653e793013f87d200 100644
>--- a/Source/WebCore/rendering/RenderLayer.cpp
>+++ b/Source/WebCore/rendering/RenderLayer.cpp
>@@ -6793,16 +6793,23 @@ bool RenderLayer::isTransparentOrFullyClippedRespectingParentFrames() const
>             return true;
>     }
> 
>-    RenderLayer* enclosingClipLayer = nullptr;
>-    for (auto* layer = this; layer; layer = enclosingClipLayer ? enclosingClipLayer->parent() : enclosingFrameRenderLayer(*layer)) {
>-        enclosingClipLayer = layer->enclosingOverflowClipLayer(IncludeSelfOrNot::IncludeSelf);
>-        if (!enclosingClipLayer)
>+    for (auto* layer = this; layer; layer = enclosingFrameRenderLayer(*layer)) {
>+        auto* frameView = layer->renderer().document().view();
>+        if (!frameView)
>+            continue;
>+
>+        auto* renderView = frameView->renderView();
>+        if (!renderView)
>+            continue;
>+
>+        auto* renderViewLayer = renderView->layer();
>+        if (!renderView->layer())
>             continue;
> 
>         LayoutRect layerBounds;
>         ClipRect backgroundRect;
>         ClipRect foregroundRect;
>-        layer->calculateRects({ enclosingClipLayer, TemporaryClipRects }, LayoutRect::infiniteRect(), layerBounds, backgroundRect, foregroundRect, layer->offsetFromAncestor(enclosingClipLayer));
>+        layer->calculateRects({ renderViewLayer, TemporaryClipRects }, LayoutRect::infiniteRect(), layerBounds, backgroundRect, foregroundRect, layer->offsetFromAncestor(renderViewLayer));
>         if (backgroundRect.isEmpty())
>             return true;
>     }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 604e40764674a5051ca94cf49514be6f5e3d36b4..edc29e85b440c1c4af197d2dc4cd51867188dc30 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-05-13  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 NOBODY (OOPS!).
>+
>+        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-13  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] When running layout tests that tap in the same location, subsequent tests fail to fire click handlers
>diff --git a/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field-expected.txt b/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..c91e1830b44d954b8abfef84b04723e75fdb1787
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field-expected.txt
>@@ -0,0 +1,15 @@
>+
>+Click me
>+This test verifies platform selection UI is not incorrectly suppressed when the editable element is visible. To manually run the test, tap the button to focus the search field, and confirm that the caret view appears in the field.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS caretRect.top is 7
>+PASS caretRect.left is 11
>+PASS caretRect.width is 2
>+PASS caretRect.height is 25
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field.html b/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..550abe3e90e694a2829eef6a036d43cd81ae5559
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/do-not-hide-selection-in-visible-field.html
>@@ -0,0 +1,74 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+    <meta name="viewport" content="width=device-width, initial-scale=1">
>+    <script src="../../../resources/ui-helper.js"></script>
>+    <script src="../../../resources/js-test.js"></script>
>+    <style>
>+        html, body {
>+            width: 100%;
>+            height: 100%;
>+            margin: 0;
>+            padding: 0;
>+        }
>+
>+        form {
>+            overflow: hidden;
>+        }
>+
>+        input, p {
>+            font-size: 20px;
>+            width: 300px;
>+        }
>+
>+        .fixed {
>+            position: fixed;
>+            width: 100%;
>+            height: 100%;
>+            top: 0;
>+            left: 0;
>+            overflow: auto;
>+        }
>+
>+        button {
>+            position: absolute;
>+            top: 40px;
>+        }
>+    </style>
>+</head>
>+<body>
>+    <form>
>+        <div class="fixed">
>+            <input type="search" />
>+        </div>
>+    </form>
>+    <button>Click me</button>
>+    <p id="description"></p>
>+    <p id="console"></p>
>+</body>
>+<script>
>+    jsTestIsAsync = true;
>+
>+    description("This test verifies platform selection UI is not incorrectly suppressed when the editable element is visible. To manually run the test, tap the button to focus the search field, and confirm that the caret view appears in the field.");
>+
>+    const button = document.querySelector("button");
>+    button.addEventListener("click", () => document.querySelector("input").focus());
>+
>+    addEventListener("load", async () => {
>+        if (!window.testRunner)
>+            return;
>+
>+        await UIHelper.activateElementAndWaitForInputSession(button);
>+
>+        caretRect = null;
>+        while (!caretRect || !caretRect.width || !caretRect.height)
>+            caretRect = await UIHelper.getUICaretViewRect();
>+
>+        shouldBe("caretRect.top", "7");
>+        shouldBe("caretRect.left", "11");
>+        shouldBe("caretRect.width", "2");
>+        shouldBe("caretRect.height", "25");
>+        finishJSTest();
>+    });
>+</script>
>+</html>
>\ No newline at end of file

Flags:

simon.fraser: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197862: 369857 | 369920