<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[244945] trunk</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/244945">244945</a></dd>
<dt>Author</dt> <dd>wenson_hsieh@apple.com</dd>
<dt>Date</dt> <dd>2019-05-03 19:40:29 -0700 (Fri, 03 May 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>REGRESSION (<a href="http://trac.webkit.org/projects/webkit/changeset/244897">r244897</a>): Caret may appear wider than normal after zooming to focus an editable element
https://bugs.webkit.org/show_bug.cgi?id=197579

Reviewed by Tim Horton.

Source/WebKit:

Fixes a couple of flaky tests (CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState and
CaretSelectionRectAfterRestoringFirstResponder) that began failing after <a href="http://trac.webkit.org/projects/webkit/changeset/244897">r244897</a>. These tests both begin by
focusing an editable element, which causes the web view to zoom in. The tests subsequently check that the caret
rect is {{ 16, 13 }, { 2, 15 }}. While the specified caret rect (coming from EditorState) is {{ 16, 13 }, { 3,
15 }}, the narrower caret rect is used because we take measures to preserve the width of the caret relative to
the view (see the inverse scaling logic in -[WKContentView selectedTextRange] for more details).

See below for more details.

* UIProcess/ios/WKContentViewInteraction.h:

Remove _isZoomingToRevealFocusedElement, now that we don't need it anymore (see -observeValueForKeyPath:).

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanupInteraction]):
(-[WKContentView observeValueForKeyPath:ofObject:change:context:]):

Stop bailing from a selection update when changing scale, while zooming to reveal the focused element. This
check was added in <a href="http://trac.webkit.org/projects/webkit/changeset/239441">r239441</a> to prevent UIWKTextInteractionAssistant's selection scrolling logic from interfering
with WKContentView-driven logic for zooming to the focused element. However, since <a href="http://trac.webkit.org/projects/webkit/changeset/244141">r244141</a>, this is no longer
necessary since selection scrolling is only driven by code in the web process.

This new update while zooming to reveal the focused element ensures that the WKTextRange returned by
-selectedTextRange after zooming will have a width that is inversely scaled using the content view's current
scale, such that it has a consistent width (relative to the web view) across different scales.

(-[WKContentView _zoomToRevealFocusedElement]):
* UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::layerTreeCommitComplete):

Remove any attempt here to notify PageClient about editor states after focus. This logic was actually incorrect,
since it didn't ensure that the layer tree commit that is being completed actually contained an editor state; as
such, the "editor state" received here could be stale.

Tools:

Fixes a couple of flaky layout tests (ModifyInputAssistantItemBarButtonGroups and
OverrideInputAssistantItemBarButtonGroups) by programmatically blurring focused elements and waiting for the
input session to change, rather than relying on -resignFirstResponder and -waitForNextPresentationUpdate to
ensure that the the focused element has been blurred.

* TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebKitChangeLog">trunk/Source/WebKit/ChangeLog</a></li>
<li><a href="#trunkSourceWebKitUIProcessiosWKContentViewInteractionh">trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h</a></li>
<li><a href="#trunkSourceWebKitUIProcessiosWKContentViewInteractionmm">trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm</a></li>
<li><a href="#trunkSourceWebKitUIProcessiosWebPageProxyIOSmm">trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm</a></li>
<li><a href="#trunkToolsChangeLog">trunk/Tools/ChangeLog</a></li>
<li><a href="#trunkToolsTestWebKitAPITestsiosKeyboardInputTestsIOSmm">trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebKitChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/ChangeLog (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/ChangeLog    2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Source/WebKit/ChangeLog       2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -1,3 +1,44 @@
</span><ins>+2019-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r244897): Caret may appear wider than normal after zooming to focus an editable element
+        https://bugs.webkit.org/show_bug.cgi?id=197579
+
+        Reviewed by Tim Horton.
+
+        Fixes a couple of flaky tests (CaretSelectionRectAfterRestoringFirstResponderWithRetainActiveFocusedState and
+        CaretSelectionRectAfterRestoringFirstResponder) that began failing after r244897. These tests both begin by
+        focusing an editable element, which causes the web view to zoom in. The tests subsequently check that the caret
+        rect is {{ 16, 13 }, { 2, 15 }}. While the specified caret rect (coming from EditorState) is {{ 16, 13 }, { 3,
+        15 }}, the narrower caret rect is used because we take measures to preserve the width of the caret relative to
+        the view (see the inverse scaling logic in -[WKContentView selectedTextRange] for more details).
+
+        See below for more details.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+
+        Remove _isZoomingToRevealFocusedElement, now that we don't need it anymore (see -observeValueForKeyPath:).
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView cleanupInteraction]):
+        (-[WKContentView observeValueForKeyPath:ofObject:change:context:]):
+
+        Stop bailing from a selection update when changing scale, while zooming to reveal the focused element. This
+        check was added in r239441 to prevent UIWKTextInteractionAssistant's selection scrolling logic from interfering
+        with WKContentView-driven logic for zooming to the focused element. However, since r244141, this is no longer
+        necessary since selection scrolling is only driven by code in the web process.
+
+        This new update while zooming to reveal the focused element ensures that the WKTextRange returned by
+        -selectedTextRange after zooming will have a width that is inversely scaled using the content view's current
+        scale, such that it has a consistent width (relative to the web view) across different scales.
+
+        (-[WKContentView _zoomToRevealFocusedElement]):
+        * UIProcess/ios/WebPageProxyIOS.mm:
+        (WebKit::WebPageProxy::layerTreeCommitComplete):
+
+        Remove any attempt here to notify PageClient about editor states after focus. This logic was actually incorrect,
+        since it didn't ensure that the layer tree commit that is being completed actually contained an editor state; as
+        such, the "editor state" received here could be stale.
+
</ins><span class="cx"> 2019-05-03  Zalan Bujtas  <zalan@apple.com>
</span><span class="cx"> 
</span><span class="cx">         [iOS] outlook.live.com: Compose email frame not fully visible and not scrollable
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessiosWKContentViewInteractionh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h     2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h        2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -321,7 +321,6 @@
</span><span class="cx">     BOOL _didAccessoryTabInitiateFocus;
</span><span class="cx">     BOOL _isExpectingFastSingleTapCommit;
</span><span class="cx">     BOOL _showDebugTapHighlightsForFastClicking;
</span><del>-    BOOL _isZoomingToRevealFocusedElement;
</del><span class="cx"> 
</span><span class="cx">     BOOL _keyboardDidRequestDismissal;
</span><span class="cx"> 
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessiosWKContentViewInteractionmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm    2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm       2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -934,7 +934,6 @@
</span><span class="cx"> 
</span><span class="cx">     _hasSetUpInteractions = NO;
</span><span class="cx">     _suppressSelectionAssistantReasons = { };
</span><del>-    _isZoomingToRevealFocusedElement = NO;
</del><span class="cx"> 
</span><span class="cx"> #if ENABLE(POINTER_EVENTS)
</span><span class="cx">     [self _resetPanningPreventionFlags];
</span><span class="lines">@@ -1031,12 +1030,6 @@
</span><span class="cx"> 
</span><span class="cx">     [self _updateTapHighlight];
</span><span class="cx"> 
</span><del>-    if (_isZoomingToRevealFocusedElement) {
-        // When zooming to the focused element, avoid additionally scrolling to reveal the selection. Since the scale transform has not yet been
-        // applied to the content view at this point, we'll end up scrolling to reveal a rect that is computed using the wrong scale.
-        return;
-    }
-
</del><span class="cx">     _selectionNeedsUpdate = YES;
</span><span class="cx">     [self _updateChangedSelection:YES];
</span><span class="cx"> }
</span><span class="lines">@@ -1671,7 +1664,6 @@
</span><span class="cx">     if (_suppressSelectionAssistantReasons.contains(WebKit::EditableRootIsTransparentOrFullyClipped) || _suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTooSmall))
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    SetForScope<BOOL> isZoomingToRevealFocusedElementForScope { _isZoomingToRevealFocusedElement, YES };
</del><span class="cx">     // In case user scaling is force enabled, do not use that scaling when zooming in with an input field.
</span><span class="cx">     // Zooming above the page's default scale factor should only happen when the user performs it.
</span><span class="cx">     [self _zoomToFocusRect:_focusedElementInformation.elementRect
</span></span></pre></div>
<a id="trunkSourceWebKitUIProcessiosWebPageProxyIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm     2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm        2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -430,11 +430,6 @@
</span><span class="cx"> void WebPageProxy::layerTreeCommitComplete()
</span><span class="cx"> {
</span><span class="cx">     pageClient().layerTreeCommitComplete();
</span><del>-
-    if (m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement && !m_editorState.isMissingPostLayoutData) {
-        pageClient().didReceiveEditorStateUpdateAfterFocus();
-        m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
-    }
</del><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void WebPageProxy::selectWithGesture(const WebCore::IntPoint point, WebCore::TextGranularity granularity, uint32_t gestureType, uint32_t gestureState, bool isInteractingWithFocusedElement, WTF::Function<void(const WebCore::IntPoint&, uint32_t, uint32_t, uint32_t, CallbackBase::Error)>&& callbackFunction)
</span></span></pre></div>
<a id="trunkToolsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Tools/ChangeLog (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/ChangeLog    2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Tools/ChangeLog       2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2019-05-03  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        REGRESSION (r244897): Caret may appear wider than normal after zooming to focus an editable element
+        https://bugs.webkit.org/show_bug.cgi?id=197579
+
+        Reviewed by Tim Horton.
+
+        Fixes a couple of flaky layout tests (ModifyInputAssistantItemBarButtonGroups and
+        OverrideInputAssistantItemBarButtonGroups) by programmatically blurring focused elements and waiting for the
+        input session to change, rather than relying on -resignFirstResponder and -waitForNextPresentationUpdate to
+        ensure that the the focused element has been blurred.
+
+        * TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm:
+
</ins><span class="cx"> 2019-05-02  Alexey Proskuryakov  <ap@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Add a tool to block spammer accounts
</span></span></pre></div>
<a id="trunkToolsTestWebKitAPITestsiosKeyboardInputTestsIOSmm"></a>
<div class="modfile"><h4>Modified: trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm (244944 => 244945)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm     2019-05-04 02:25:03 UTC (rev 244944)
+++ trunk/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm        2019-05-04 02:40:29 UTC (rev 244945)
</span><span class="lines">@@ -213,13 +213,10 @@
</span><span class="cx">     item.leadingBarButtonGroups = @[ leadingItems ];
</span><span class="cx">     item.trailingBarButtonGroups = @[ trailingItems ];
</span><span class="cx"> 
</span><del>-    bool doneWaitingForInputSession = false;
</del><span class="cx">     [inputDelegate setFocusStartsInputSessionPolicyHandler:[&] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
</span><del>-        doneWaitingForInputSession = true;
</del><span class="cx">         return _WKFocusStartsInputSessionPolicyAllow;
</span><span class="cx">     }];
</span><del>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
</del><ins>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
</ins><span class="cx"> 
</span><span class="cx">     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);
</span><span class="lines">@@ -226,13 +223,9 @@
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
</span><span class="cx"> 
</span><span class="cx">     // Now blur and refocus the editable area, and check that the same leading and trailing button items are present.
</span><del>-    [webView resignFirstResponder];
-    [webView waitForNextPresentationUpdate];
</del><ins>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.blur()"];
+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
</ins><span class="cx"> 
</span><del>-    doneWaitingForInputSession = false;
-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
-
</del><span class="cx">     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
</span><span class="lines">@@ -244,14 +237,10 @@
</span><span class="cx">     auto webView = adoptNS([[InputAssistantItemTestingWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
</span><span class="cx">     [webView _setInputDelegate:inputDelegate.get()];
</span><span class="cx">     [webView synchronouslyLoadHTMLString:@"<body contenteditable>"];
</span><del>-
-    bool doneWaitingForInputSession = false;
</del><span class="cx">     [inputDelegate setFocusStartsInputSessionPolicyHandler:[&] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
</span><del>-        doneWaitingForInputSession = true;
</del><span class="cx">         return _WKFocusStartsInputSessionPolicyAllow;
</span><span class="cx">     }];
</span><del>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
</del><ins>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
</ins><span class="cx"> 
</span><span class="cx">     UIBarButtonItemGroup *leadingItems = [InputAssistantItemTestingWebView leadingItemsForWebView:webView.get()];
</span><span class="cx">     UIBarButtonItemGroup *trailingItems = [InputAssistantItemTestingWebView trailingItemsForWebView:webView.get()];
</span><span class="lines">@@ -260,13 +249,9 @@
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
</span><span class="cx"> 
</span><span class="cx">     // Now blur and refocus the editable area, and check that the same leading and trailing button items are present.
</span><del>-    [webView resignFirstResponder];
-    [webView waitForNextPresentationUpdate];
</del><ins>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.blur()"];
+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
</ins><span class="cx"> 
</span><del>-    doneWaitingForInputSession = false;
-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
-
</del><span class="cx">     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);
</span><span class="cx">     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
</span></span></pre>
</div>
</div>

</body>
</html>