Title: [244546] trunk
- Revision
- 244546
- Author
- [email protected]
- Date
- 2019-04-23 10:10:10 -0700 (Tue, 23 Apr 2019)
Log Message
[iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
https://bugs.webkit.org/show_bug.cgi?id=197188
Reviewed by Wenson Hsieh.
Source/WebCore:
The bug was caused by the scroll-to-reveal code triggered by Element::updateFocusAppearance updating
the scroll position via scrolling tree update in a layer tree commit which happens after
_zoomToRevealFocusedElement in WKContentView had already scrolled the frame view.
To fix this problem, we need to defer the editor state update until the layer commit (see r244494),
and update the scrolling tree before invoking WebPageProxy::editorStateChanged which brings up
the keyboard and scroll-to-reveal the caret in the UI process side.
We also avoid revealing the focus for the second time via Document::scheduleScrollToFocusedElement
in Element::updateFocusAppearance as this timer based scrolling also happens after we had already
revealed the caret in _zoomToRevealFocusedElement. This is a bit hacky but works for most cases since
we wouldn't bring up a keyboard if the focused element was not editable anyway.
Test: editing/selection/ios/scrolling-to-focused-element-inside-iframe.html
* dom/Element.cpp:
(WebCore::Element::updateFocusAppearance): Avoid scheduling a timer based reavel of the focused element
when we're already revealing the element via selection change.
Source/WebKit:
Commit the scroll tree update before revealing the keyboard via editor state update.
* UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
(WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
LayoutTests:
Added a regression test.
* editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt: Added.
* editing/selection/ios/scrolling-to-focused-element-inside-iframe.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (244545 => 244546)
--- trunk/LayoutTests/ChangeLog 2019-04-23 16:25:30 UTC (rev 244545)
+++ trunk/LayoutTests/ChangeLog 2019-04-23 17:10:10 UTC (rev 244546)
@@ -1,3 +1,15 @@
+2019-04-23 Ryosuke Niwa <[email protected]>
+
+ [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
+ https://bugs.webkit.org/show_bug.cgi?id=197188
+
+ Reviewed by Wenson Hsieh.
+
+ Added a regression test.
+
+ * editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt: Added.
+ * editing/selection/ios/scrolling-to-focused-element-inside-iframe.html: Added.
+
2019-04-23 John Wilander <[email protected]>
Ad Click Attribution redirects to well-known location should not trigger a conversion if they are blocked by content blockers
Added: trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt (0 => 244546)
--- trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe-expected.txt 2019-04-23 17:10:10 UTC (rev 244546)
@@ -0,0 +1,8 @@
+
+
+--------
+Frame: '<!--frame1-->'
+--------
+click here This tests focusing an element inside an iframe at the bottom of the page.
+To manually test, tap on "click here" below with docked software keyboard.
+PASS
Added: trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe.html (0 => 244546)
--- trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe.html (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/scrolling-to-focused-element-inside-iframe.html 2019-04-23 17:10:10 UTC (rev 244546)
@@ -0,0 +1,57 @@
+<!DOCTYPE html><!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+</head>
+<body>
+<style>
+html, body { margin: 0; padding: 0; width: 100%; height: 100%; }
+iframe { position: absolute; top: 0; left: 0; width: 100%; height: 100%; }
+</style>
+<script src=""
+<script>
+
+const frame = document.createElement('iframe');
+document.body.appendChild(frame);
+frame.contentDocument.body.innerHTML = `
+<style>
+html, body { margin: 0; padding: 0; width: 100%; height: 100%; }
+body { background: #6cf; }
+#description { position: absolute; top: 50%; padding: 10px; }
+#container { position: absolute; top: 90%; left: 20%; width: 60%; padding: 10px; font-size: 20px; text-align: center; height: 200px; border: solid 1px #ccc; }
+</style>
+<div id="container" _onclick_="this.contentEditable = true; this.focus();">click here</div>
+<p id="description">
+ This tests focusing an element inside an iframe at the bottom of the page.<br>
+ To manually test, tap on "click here" below with docked software keyboard.<br>
+ <span id="result"></span>
+</p>
+`;
+
+let keyboardHeight = 250;
+
+function checkScrollTop() {
+ frame.contentWindow.result.textContent = visualViewport.pageTop > keyboardHeight ? 'PASS' : `FAIL - ${visualViewport.offsetTop}px`;
+}
+
+async function runTest() {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ testRunner.dumpChildFramesAsText();
+ await UIHelper.setHardwareKeyboardAttached(false);
+ await UIHelper.activateAndWaitForInputSessionAt(frame.offsetWidth / 2, frame.offsetHeight - 5);
+ await UIHelper.ensurePresentationUpdate();
+ const rect = await UIHelper.inputViewBounds();
+ keyboardHeight = rect.height;
+ checkScrollTop();
+ testRunner.notifyDone();
+}
+
+if (window.testRunner)
+ runTest();
+else
+ frame.contentWindow.container.addEventListener('focus', () => setTimeout(checkScrollTop, 500));
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (244545 => 244546)
--- trunk/Source/WebCore/ChangeLog 2019-04-23 16:25:30 UTC (rev 244545)
+++ trunk/Source/WebCore/ChangeLog 2019-04-23 17:10:10 UTC (rev 244546)
@@ -1,3 +1,29 @@
+2019-04-23 Ryosuke Niwa <[email protected]>
+
+ [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
+ https://bugs.webkit.org/show_bug.cgi?id=197188
+
+ Reviewed by Wenson Hsieh.
+
+ The bug was caused by the scroll-to-reveal code triggered by Element::updateFocusAppearance updating
+ the scroll position via scrolling tree update in a layer tree commit which happens after
+ _zoomToRevealFocusedElement in WKContentView had already scrolled the frame view.
+
+ To fix this problem, we need to defer the editor state update until the layer commit (see r244494),
+ and update the scrolling tree before invoking WebPageProxy::editorStateChanged which brings up
+ the keyboard and scroll-to-reveal the caret in the UI process side.
+
+ We also avoid revealing the focus for the second time via Document::scheduleScrollToFocusedElement
+ in Element::updateFocusAppearance as this timer based scrolling also happens after we had already
+ revealed the caret in _zoomToRevealFocusedElement. This is a bit hacky but works for most cases since
+ we wouldn't bring up a keyboard if the focused element was not editable anyway.
+
+ Test: editing/selection/ios/scrolling-to-focused-element-inside-iframe.html
+
+ * dom/Element.cpp:
+ (WebCore::Element::updateFocusAppearance): Avoid scheduling a timer based reavel of the focused element
+ when we're already revealing the element via selection change.
+
2019-04-23 Remy Demarest <[email protected]>
Fix layout issues occuring when entering full screen mode.
Modified: trunk/Source/WebCore/dom/Element.cpp (244545 => 244546)
--- trunk/Source/WebCore/dom/Element.cpp 2019-04-23 16:25:30 UTC (rev 244545)
+++ trunk/Source/WebCore/dom/Element.cpp 2019-04-23 17:10:10 UTC (rev 244546)
@@ -2785,6 +2785,7 @@
if (frame->selection().shouldChangeSelection(newSelection)) {
frame->selection().setSelection(newSelection, FrameSelection::defaultSetSelectionOptions(), Element::defaultFocusTextStateChangeIntent());
frame->selection().revealSelection(revealMode);
+ return;
}
}
Modified: trunk/Source/WebKit/ChangeLog (244545 => 244546)
--- trunk/Source/WebKit/ChangeLog 2019-04-23 16:25:30 UTC (rev 244545)
+++ trunk/Source/WebKit/ChangeLog 2019-04-23 17:10:10 UTC (rev 244546)
@@ -1,3 +1,15 @@
+2019-04-23 Ryosuke Niwa <[email protected]>
+
+ [iOS] element.focus() sometimes fails to reveal the focused element when it becomes editable dynamically
+ https://bugs.webkit.org/show_bug.cgi?id=197188
+
+ Reviewed by Wenson Hsieh.
+
+ Commit the scroll tree update before revealing the keyboard via editor state update.
+
+ * UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm:
+ (WebKit::RemoteLayerTreeDrawingAreaProxy::commitLayerTree):
+
2019-04-23 Remy Demarest <[email protected]>
Fix layout issues occuring when entering full screen mode.
Modified: trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm (244545 => 244546)
--- trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm 2019-04-23 16:25:30 UTC (rev 244545)
+++ trunk/Source/WebKit/UIProcess/RemoteLayerTree/RemoteLayerTreeDrawingAreaProxy.mm 2019-04-23 17:10:10 UTC (rev 244546)
@@ -199,9 +199,6 @@
m_transactionIDForPendingCACommit = layerTreeTransaction.transactionID();
m_activityStateChangeID = layerTreeTransaction.activityStateChangeID();
- if (layerTreeTransaction.hasEditorState())
- m_webPageProxy.editorStateChanged(layerTreeTransaction.editorState());
-
if (m_remoteLayerTreeHost->updateLayerTree(layerTreeTransaction)) {
if (layerTreeTransaction.transactionID() >= m_transactionIDForUnhidingContent)
m_webPageProxy.setRemoteLayerTreeRootNode(m_remoteLayerTreeHost->rootNode());
@@ -255,6 +252,9 @@
didRefreshDisplay();
#endif
+ if (layerTreeTransaction.hasEditorState())
+ m_webPageProxy.editorStateChanged(layerTreeTransaction.editorState());
+
if (auto milestones = layerTreeTransaction.newlyReachedPaintingMilestones())
m_webPageProxy.didReachLayoutMilestone(milestones);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
