Attachment 369016 Details for Bug 197579 – Patch

[patch] Patch

bug-197579-20190503162450.patch (text/plain), 11.50 KB, created by Wenson Hsieh on 2019-05-03 16:24:51 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-05-03 16:24:51 PDT

Size: 11.50 KB

patch

obsolete

>Subversion Revision: 244898
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index ad5a964dab4c1980e669bcd77f1286b46c753b4d..132bc27ed8b89eef48cdc0715916886e718a9e5a 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,44 @@
>+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 NOBODY (OOPS!).
>+
>+        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.
>+
> 2019-05-02  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         REGRESSION: Layout test editing/selection/ios/selection-after-changing-text-with-callout-menu.html is failing
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>index a61f519f75c09bddbae133797a9189ee1980bbee..05a05b0e68cfa050bc5503fcb0a3c0b1def76b31 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>@@ -321,7 +321,6 @@ struct WKAutoCorrectionData {
>     BOOL _didAccessoryTabInitiateFocus;
>     BOOL _isExpectingFastSingleTapCommit;
>     BOOL _showDebugTapHighlightsForFastClicking;
>-    BOOL _isZoomingToRevealFocusedElement;
> 
>     BOOL _keyboardDidRequestDismissal;
> 
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index db1dd7611dbe415d16eb6ec17006e55a4cf302c6..12e8f3da0113f288e7df4e77836dd11c0dba9278 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -934,7 +934,6 @@ - (void)cleanupInteraction
> 
>     _hasSetUpInteractions = NO;
>     _suppressSelectionAssistantReasons = { };
>-    _isZoomingToRevealFocusedElement = NO;
> 
> #if ENABLE(POINTER_EVENTS)
>     [self _resetPanningPreventionFlags];
>@@ -1031,12 +1030,6 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
> 
>     [self _updateTapHighlight];
> 
>-    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;
>-    }
>-
>     _selectionNeedsUpdate = YES;
>     [self _updateChangedSelection:YES];
> }
>@@ -1671,7 +1664,6 @@ - (void)_zoomToRevealFocusedElement
>     if (_suppressSelectionAssistantReasons.contains(WebKit::EditableRootIsTransparentOrFullyClipped) || _suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTooSmall))
>         return;
> 
>-    SetForScope<BOOL> isZoomingToRevealFocusedElementForScope { _isZoomingToRevealFocusedElement, YES };
>     // In case user scaling is force enabled, do not use that scaling when zooming in with an input field.
>     // Zooming above the page's default scale factor should only happen when the user performs it.
>     [self _zoomToFocusRect:_focusedElementInformation.elementRect
>diff --git a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>index a670f9ceadceab8fa63dbb678ceb76b77de965ef..9030b7b87c337b75e3fcfe5f49adc701249961b6 100644
>--- a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>@@ -430,11 +430,6 @@ bool WebPageProxy::updateLayoutViewportParameters(const WebKit::RemoteLayerTreeT
> void WebPageProxy::layerTreeCommitComplete()
> {
>     pageClient().layerTreeCommitComplete();
>-
>-    if (m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement && !m_editorState.isMissingPostLayoutData) {
>-        pageClient().didReceiveEditorStateUpdateAfterFocus();
>-        m_waitingForPostLayoutEditorStateUpdateAfterFocusingElement = false;
>-    }
> }
> 
> 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)
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index a9df0374b2bdeaaa66334be9dd93b0328e9917a1..844b7178bbbdeeff886b65f49db04612fc1f526b 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+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 NOBODY (OOPS!).
>+
>+        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:
>+
> 2019-05-02  Chris Dumez  <cdumez@apple.com>
> 
>         Add test coverage for <rdar://problem/49731231>
>diff --git a/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm b/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm
>index 777c999a18333dc634da51c299737420647797c5..655ef1309318a5cb4615119beb04c9adf114c421 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/KeyboardInputTestsIOS.mm
>@@ -213,25 +213,18 @@ TEST(KeyboardInputTests, ModifyInputAssistantItemBarButtonGroups)
>     item.leadingBarButtonGroups = @[ leadingItems ];
>     item.trailingBarButtonGroups = @[ trailingItems ];
> 
>-    bool doneWaitingForInputSession = false;
>     [inputDelegate setFocusStartsInputSessionPolicyHandler:[&] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
>-        doneWaitingForInputSession = true;
>         return _WKFocusStartsInputSessionPolicyAllow;
>     }];
>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
>-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
> 
>     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
>     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);
>     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
> 
>     // Now blur and refocus the editable area, and check that the same leading and trailing button items are present.
>-    [webView resignFirstResponder];
>-    [webView waitForNextPresentationUpdate];
>-
>-    doneWaitingForInputSession = false;
>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
>-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.blur()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
> 
>     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
>     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);
>@@ -244,14 +237,10 @@ TEST(KeyboardInputTests, OverrideInputAssistantItemBarButtonGroups)
>     auto webView = adoptNS([[InputAssistantItemTestingWebView alloc] initWithFrame:CGRectMake(0, 0, 320, 500)]);
>     [webView _setInputDelegate:inputDelegate.get()];
>     [webView synchronouslyLoadHTMLString:@"<body contenteditable>"];
>-
>-    bool doneWaitingForInputSession = false;
>     [inputDelegate setFocusStartsInputSessionPolicyHandler:[&] (WKWebView *, id <_WKFocusedElementInfo>) -> _WKFocusStartsInputSessionPolicy {
>-        doneWaitingForInputSession = true;
>         return _WKFocusStartsInputSessionPolicyAllow;
>     }];
>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
>-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
> 
>     UIBarButtonItemGroup *leadingItems = [InputAssistantItemTestingWebView leadingItemsForWebView:webView.get()];
>     UIBarButtonItemGroup *trailingItems = [InputAssistantItemTestingWebView trailingItemsForWebView:webView.get()];
>@@ -260,12 +249,8 @@ TEST(KeyboardInputTests, OverrideInputAssistantItemBarButtonGroups)
>     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.trailingBarButtonGroups containsObject:trailingItems]);
> 
>     // Now blur and refocus the editable area, and check that the same leading and trailing button items are present.
>-    [webView resignFirstResponder];
>-    [webView waitForNextPresentationUpdate];
>-
>-    doneWaitingForInputSession = false;
>-    [webView evaluateJavaScript:@"document.body.focus()" completionHandler:nil];
>-    TestWebKitAPI::Util::run(&doneWaitingForInputSession);
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.blur()"];
>+    [webView evaluateJavaScriptAndWaitForInputSessionToChange:@"document.body.focus()"];
> 
>     EXPECT_EQ([webView firstResponder], [webView textInputContentView]);
>     EXPECT_TRUE([[webView firstResponder].inputAssistantItem.leadingBarButtonGroups containsObject:leadingItems]);

Actions: View | Formatted Diff | Diff

Attachments on bug 197579: 369016