Title: [245902] trunk
Revision
245902
Author
[email protected]
Date
2019-05-30 13:06:38 -0700 (Thu, 30 May 2019)

Log Message

Missing caret when focusing an editable field if the selection was set when WKWebView wasn't first responder
https://bugs.webkit.org/show_bug.cgi?id=198356
<rdar://problem/50798593>

Reviewed by Tim Horton.

Source/WebKit:

In this bug, the DOM selection is initially set by script in a web view that is not the first responder. Then,
either the user begins editing by tapping somewhere such that the selection does not change, or an editable
element is programmatically focused and the client allows programmatic focus to show the keyboard. This is
because the selection clipping rect used by the UI process when computing the bounds of the caret view is empty,
causing the entire caret to be clipped.

This is due to two related issues: first, no updated editor state is sent to the UI process after the element is
focused, if the selection has not also changed. This means that while the selection geometry is sent over to the
UI process, the selection clipping rect (a member of the EditorState's PostLayoutData called
"focusedElementRect") becomes stale in the UI process, since the there was no focused element when the
previously computed editor state was sent to the UI process. To fix this, we schedule a full editor state update
when an element is focused, to ensure that the selection is eventually updated in the UI process.

Secondly, even once the editor state update is sent to the UI process, we will actually avoid updating any text
selection views, since there is no change in WKSelectionDrawingInfo, which currently consists of a selection
type, and either a caret rect or a list of selection rects. However, since selection drawing is also affected by
the selection clipping rect, it seems reasonable to add the selection clipping rect to the drawing info, and
trigger a selection update if this selection clipping rect has changed.

* UIProcess/ios/WKContentViewInteraction.h:
* UIProcess/ios/WKContentViewInteraction.mm:
(WebKit::WKSelectionDrawingInfo::WKSelectionDrawingInfo):

Add selectionClippingRect to WKSelectionDrawingInfo, and check against it when comparing two drawing infos.

(WebKit::operator==):
(WebKit::operator<<):
* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::elementDidFocus):

Schedule an editor state update when focusing an element. In many cases, an editor state update has already been
scheduled when focusing an element, so this becomes a no-op; however, in this scenario, it delivers updated
selection clipping rects (i.e. the focused element rect) and other updated information to the UI process.

LayoutTests:

Add a new layout test to exercise this scenario.

* editing/selection/ios/caret-when-focusing-editable-element-with-selection-expected.txt: Added.
* editing/selection/ios/caret-when-focusing-editable-element-with-selection.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (245901 => 245902)


--- trunk/LayoutTests/ChangeLog	2019-05-30 19:52:57 UTC (rev 245901)
+++ trunk/LayoutTests/ChangeLog	2019-05-30 20:06:38 UTC (rev 245902)
@@ -1,3 +1,16 @@
+2019-05-30  Wenson Hsieh  <[email protected]>
+
+        Missing caret when focusing an editable field if the selection was set when WKWebView wasn't first responder
+        https://bugs.webkit.org/show_bug.cgi?id=198356
+        <rdar://problem/50798593>
+
+        Reviewed by Tim Horton.
+
+        Add a new layout test to exercise this scenario.
+
+        * editing/selection/ios/caret-when-focusing-editable-element-with-selection-expected.txt: Added.
+        * editing/selection/ios/caret-when-focusing-editable-element-with-selection.html: Added.
+
 2019-05-30  Zalan Bujtas  <[email protected]>
 
         [iOS] Do not linkify telephone numbers inside <a> elements.

Added: trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection-expected.txt (0 => 245902)


--- trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection-expected.txt	2019-05-30 20:06:38 UTC (rev 245902)
@@ -0,0 +1,13 @@
+This test verifies that when setting the selection inside an editable element while the web view is not first responder, the caret should appear after tapping the editable element to show the keyboard. This test requires WebKitTestRunner.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS caretRect.left is 9
+PASS caretRect.top is 21
+PASS caretRect.width is 2
+PASS caretRect.height is 24
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection.html (0 => 245902)


--- trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/caret-when-focusing-editable-element-with-selection.html	2019-05-30 20:06:38 UTC (rev 245902)
@@ -0,0 +1,55 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+    <head>
+        <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+        <script src=""
+        <script src=""
+        <style>
+            html, body {
+                width: 100%;
+                height: 100%;
+                font-size: 20px;
+            }
+
+            #editor {
+                width: 100%;
+                height: 100px;
+                border: 1px solid tomato;
+            }
+        </style>
+    </head>
+    <body>
+        <p contenteditable id="editor"></p>
+        <p id="description"></p>
+        <p id="console"></p>
+    </body>
+    <script>
+        jsTestIsAsync = true;
+
+        addEventListener("load", runTest);
+
+        async function runTest()
+        {
+            description("This test verifies that when setting the selection inside an editable element while the web view is not first responder, the caret should appear after tapping the editable element to show the keyboard. This test requires WebKitTestRunner.");
+
+            const editor = document.getElementById("editor");
+            await UIHelper.setHardwareKeyboardAttached(false);
+            await UIHelper.resignFirstResponder();
+            getSelection().setPosition(editor);
+            await UIHelper.activateElement(editor);
+            do {
+                caretRect = await UIHelper.getUICaretViewRect();
+            } while (!caretRect.width || !caretRect.height);
+
+            shouldBe("caretRect.left", "9");
+            shouldBe("caretRect.top", "21");
+            shouldBe("caretRect.width", "2");
+            shouldBe("caretRect.height", "24");
+
+            editor.blur();
+            await UIHelper.waitForKeyboardToHide();
+
+            finishJSTest();
+        }
+    </script>
+</html>

Modified: trunk/Source/WebKit/ChangeLog (245901 => 245902)


--- trunk/Source/WebKit/ChangeLog	2019-05-30 19:52:57 UTC (rev 245901)
+++ trunk/Source/WebKit/ChangeLog	2019-05-30 20:06:38 UTC (rev 245902)
@@ -1,3 +1,45 @@
+2019-05-30  Wenson Hsieh  <[email protected]>
+
+        Missing caret when focusing an editable field if the selection was set when WKWebView wasn't first responder
+        https://bugs.webkit.org/show_bug.cgi?id=198356
+        <rdar://problem/50798593>
+
+        Reviewed by Tim Horton.
+
+        In this bug, the DOM selection is initially set by script in a web view that is not the first responder. Then,
+        either the user begins editing by tapping somewhere such that the selection does not change, or an editable
+        element is programmatically focused and the client allows programmatic focus to show the keyboard. This is
+        because the selection clipping rect used by the UI process when computing the bounds of the caret view is empty,
+        causing the entire caret to be clipped.
+
+        This is due to two related issues: first, no updated editor state is sent to the UI process after the element is
+        focused, if the selection has not also changed. This means that while the selection geometry is sent over to the
+        UI process, the selection clipping rect (a member of the EditorState's PostLayoutData called
+        "focusedElementRect") becomes stale in the UI process, since the there was no focused element when the
+        previously computed editor state was sent to the UI process. To fix this, we schedule a full editor state update
+        when an element is focused, to ensure that the selection is eventually updated in the UI process.
+
+        Secondly, even once the editor state update is sent to the UI process, we will actually avoid updating any text
+        selection views, since there is no change in WKSelectionDrawingInfo, which currently consists of a selection
+        type, and either a caret rect or a list of selection rects. However, since selection drawing is also affected by
+        the selection clipping rect, it seems reasonable to add the selection clipping rect to the drawing info, and
+        trigger a selection update if this selection clipping rect has changed.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (WebKit::WKSelectionDrawingInfo::WKSelectionDrawingInfo):
+
+        Add selectionClippingRect to WKSelectionDrawingInfo, and check against it when comparing two drawing infos.
+
+        (WebKit::operator==):
+        (WebKit::operator<<):
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::elementDidFocus):
+
+        Schedule an editor state update when focusing an element. In many cases, an editor state update has already been
+        scheduled when focusing an element, so this becomes a no-op; however, in this scenario, it delivers updated
+        selection clipping rects (i.e. the focused element rect) and other updated information to the UI process.
+
 2019-05-30  David Quesada  <[email protected]>
 
         REGRESSION (r245756) [Mac] 2 TestWebKitAPI.DownloadProgress* and TestWebKitAPI._WKDownload.DownloadMonitorCancel are flaky timeouts

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (245901 => 245902)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2019-05-30 19:52:57 UTC (rev 245901)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2019-05-30 20:06:38 UTC (rev 245902)
@@ -175,6 +175,7 @@
     SelectionType type;
     WebCore::IntRect caretRect;
     Vector<WebCore::SelectionRect> selectionRects;
+    WebCore::IntRect selectionClipRect;
 };
 
 WTF::TextStream& operator<<(WTF::TextStream&, const WKSelectionDrawingInfo&);

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (245901 => 245902)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-05-30 19:52:57 UTC (rev 245901)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2019-05-30 20:06:38 UTC (rev 245902)
@@ -195,6 +195,7 @@
     auto& postLayoutData = editorState.postLayoutData();
     caretRect = postLayoutData.caretRectAtEnd;
     selectionRects = postLayoutData.selectionRects;
+    selectionClipRect = postLayoutData.focusedElementRect;
 }
 
 inline bool operator==(const WKSelectionDrawingInfo& a, const WKSelectionDrawingInfo& b)
@@ -215,6 +216,9 @@
         }
     }
 
+    if (a.type != WKSelectionDrawingInfo::SelectionType::None && a.selectionClipRect != b.selectionClipRect)
+        return false;
+
     return true;
 }
 
@@ -240,6 +244,7 @@
     stream.dumpProperty("type", info.type);
     stream.dumpProperty("caret rect", info.caretRect);
     stream.dumpProperty("selection rects", info.selectionRects);
+    stream.dumpProperty("selection clip rect", info.selectionClipRect);
     return stream;
 }
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (245901 => 245902)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-05-30 19:52:57 UTC (rev 245901)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2019-05-30 20:06:38 UTC (rev 245902)
@@ -5467,6 +5467,8 @@
         send(Messages::WebPageProxy::SetEditableElementIsFocused(!element.hasTagName(WebCore::HTMLNames::selectTag)));
 #endif
         m_recentlyBlurredElement = nullptr;
+
+        scheduleFullEditorStateUpdate();
     }
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to