- 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();
}
}