[iOS] Focusing an editable element should scroll to reveal the selection
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Dec 2018 16:28:55 +0000 (16:28 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Dec 2018 16:28:55 +0000 (16:28 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192802
<rdar://problem/46781759>

Reviewed by Tim Horton.

Source/WebKit:

Currently, when tapping on an editable element, logic in -[WKWebView _zoomToFocusRect:…:] attempts to adjust the
visible viewport such that the rect containing the selection is visible. However, AssistedNodeInformation's
selectionRect is used here, which (as the FIXME in WebPage::getAssistedNodeInformation notes) is either the last
touch location, or the top left of the element if the touch location is outside of the element's bounding rect.
This leads to confusing and undesirable behavior when tapping near the bottom of a large contenteditable element
to focus it, since the actual selection will end up near the top of the element, yet we'll try to scroll to
reveal the bottom of the element, which causes the visible selection to scroll offscreen. Notably, this affects
scenarios involving editable web views embedded in apps, such as Mail compose.

Right now, we use the last touch location as an approximation for the selection rect because the selection may
have not yet been updated at the moment when focus moves into an editable element. To fix this, we defer the
process of zooming to the focused element rect until after the selection changes and the UI process is updated
with information about the new selection rects.

Test: editing/selection/ios/selection-is-visible-after-focusing-editable-area.html

* Shared/AssistedNodeInformation.cpp:
(WebKit::AssistedNodeInformation::encode const):
(WebKit::AssistedNodeInformation::decode):
* Shared/AssistedNodeInformation.h:

Rename selectionRect to elementInteractionLocation, to more accurately reflect its value and purpose. This isn't
strictly always the last touch location, since we may default to the focused element location instead if the
last touch location is outside of the element rect.

* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _zoomToFocusRect:selectionRect:insideFixed:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):

Tweak a constant that determines the minimum amount of margin to leave between the selection rect and the edge
of the window when scrolling to reveal the focused element. Previously, this was larger than necessary to
accomodate for the fact that the "selection rect" used when zooming to the focused element did not take the
actual selection into account at all, and was simply a 1 by 1 rect; this meant that the margin needed to be
large enough to exceed the usual height of a text caret in editable content. Since we now use the real selection
rect, we can be much less generous with this margin.

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

Don't additionally update the selection in the middle of triggering zooming to the focused element; on
particular versions of iOS, this now attempts to scroll the selection rect on-screen, which then conflicts with
zooming to reveal the focused element.

(-[WKContentView _zoomToRevealFocusedElement]):

Renamed from _displayFormNodeInputView to _zoomToRevealFocusedElement, to make the purpose of this function more
clear. Additionally, pull logic to update the accessory view out of this method, so that it's strictly concerned
with zooming to the focused element.

(-[WKContentView inputView]):

Add a FIXME describing the implications of zooming to the focused element in the implementation of -inputView.
See also: <https://bugs.webkit.org/show_bug.cgi?id=192878>.

(-[WKContentView accessoryTab:]):

Fix a subtle issue when keeping track of _didAccessoryTabInitiateFocus. Currently, this is set to YES in
-accessoryTab: and unset in _displayFormNodeInputView, but since _displayFormNodeInputView may be invoked
multiple times for the same focused element (see: -inputView), we might end up zooming to the focused element
with _didAccessoryTabInitiateFocus set to NO, even though we initiated focus with the previous/next buttons.

Instead, temporarily set a different ivar, _isChangingFocusUsingAccessoryTab, to YES in -accessoryTab:, and
unset it in the completion handler after the focused element has changed. Then, when we _startAssistingNode:,
set _didAccessoryTabInitiateFocus to _isChangingFocusUsingAccessoryTab. This ensures that the correctness of
_didAccessoryTabInitiateFocus isn't tied to the number of times -[WKContentView inputView] is invoked when
focusing an element.

(shouldZoomToRevealSelectionRect):
(rectToRevealWhenZoomingToFocusedElement):

Add a helper method to determine the selection rect to use when zooming to reveal the focused element. ASSERTs
that we have post-layout data in the EditorState.

(-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):

When "assisting" a focused element, immediately zoom to it if we don't need selection information to compute the
rect to zoom to; otherwise, defer zooming until we receive the first editor state update in the UI process that
contains information about our selection rects.

(-[WKContentView _stopAssistingNode]):
(-[WKContentView _didReceiveEditorStateUpdateAfterFocus]):

If necessary, reveal the focused element by zooming.

(-[WKContentView _updateInitialWritingDirectionIfNecessary]):

Pull this initial writing direction update logic out into a separate helper method.

(-[WKContentView _displayFormNodeInputView]): Deleted.

Replaced by _zoomToRevealFocusedElement.

* WebProcess/WebCoreSupport/WebChromeClient.cpp:
(WebKit::WebChromeClient::elementDidRefocus):

This currently calls WebChromeClient::elementDidFocus; instead, call the new WebPage::elementDidRefocus;
additionally, make this available on all PLATFORM(COCOA), rather than just IOS_FAMILY.

* WebProcess/WebCoreSupport/WebChromeClient.h:
* WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm:
(WebKit::WebChromeClient::elementDidRefocus): Deleted.

Replaced by the PLATFORM(COCOA) version.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::elementDidRefocus):

When refocusing an element, ensure that post-layout editor state data is sent to the UI process by including a
full EditorState in the next layer tree transaction.

* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::completeSyntheticClick):

Call elementDidRefocus instead of elementDidFocus, in the case where the existing focused element is clicked.

(WebKit::WebPage::getAssistedNodeInformation):

Adjust for the change from selectionRect to elementInteractionLocation.

LayoutTests:

Adds a new layout test to verify that tapping near the bottom of a tall editable element to focus it doesn't
cause the page to scroll up (and, as a result, leave the selection caret obscured).

* editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt: Added.
* editing/selection/ios/selection-is-visible-after-focusing-editable-area.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239441 268f45cc-cd09-0410-ab3c-d52691b4dbfc

15 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/Shared/AssistedNodeInformation.cpp
Source/WebKit/Shared/AssistedNodeInformation.h
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm
Source/WebKit/WebProcess/WebPage/WebPage.cpp
Source/WebKit/WebProcess/WebPage/WebPage.h
Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm

index c338870..222032a 100644 (file)
@@ -1,3 +1,17 @@
+2018-12-20  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Focusing an editable element should scroll to reveal the selection
+        https://bugs.webkit.org/show_bug.cgi?id=192802
+        <rdar://problem/46781759>
+
+        Reviewed by Tim Horton.
+
+        Adds a new layout test to verify that tapping near the bottom of a tall editable element to focus it doesn't
+        cause the page to scroll up (and, as a result, leave the selection caret obscured).
+
+        * editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt: Added.
+        * editing/selection/ios/selection-is-visible-after-focusing-editable-area.html: Added.
+
 2018-12-19  Ross Kirsling  <ross.kirsling@sony.com>
 
         [WinCairo] Unreviewed test gardening.
diff --git a/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt b/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt
new file mode 100644 (file)
index 0000000..3863ad0
--- /dev/null
@@ -0,0 +1,5 @@
+This test verifies that the selection is visible after tapping near the bottom of a large editable area. To test manually, tap near the bottom of the screen and check that the page did not scroll, and that the selection is visible at the top of the editable area.
+
+Tap here
+The initial scroll offset is: 0,0
+The final scroll offset is: 0,0
diff --git a/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html b/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html
new file mode 100644 (file)
index 0000000..2f40636
--- /dev/null
@@ -0,0 +1,64 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<head>
+    <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
+    <script src="../../../resources/ui-helper.js"></script>
+    <style>
+    div#tap-here {
+        border-radius: 50px;
+        width: 100px;
+        height: 100px;
+        position: absolute;
+        left: 100px;
+        top: 400px;
+        background-color: tomato;
+        color: white;
+        padding-top: 42px;
+        box-sizing: border-box;
+        text-align: center;
+        pointer-events: none;
+        opacity: 0.75;
+    }
+
+    pre#editor {
+        line-height: 1.5em;
+        border: silver dashed 2px;
+        height: 100vh;
+        margin-top: 0;
+    }
+    </style>
+</head>
+<body style="margin: 0">
+    <pre id="editor" contenteditable></pre>
+    <p id="description">
+    This test verifies that the selection is visible after tapping near the bottom of a large editable area.
+    To test manually, tap near the bottom of the screen and check that the page did not scroll, and that the selection
+    is visible at the top of the editable area.
+    </p>
+    <div id="tap-here">Tap here</div>
+    <div>
+        <pre>The initial scroll offset is: <span id="initial"></span></pre>
+        <pre>The final scroll offset is: <span id="final"></span></pre>
+    </div>
+</body>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText();
+    testRunner.waitUntilDone();
+}
+
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    initial.textContent = [pageXOffset, pageYOffset].toString();
+
+    await UIHelper.activateAndWaitForInputSessionAt(150, 450);
+    editor.blur();
+    await UIHelper.waitForKeyboardToHide();
+
+    final.textContent = [pageXOffset, pageYOffset].toString();
+    testRunner.notifyDone();
+});
+</script>
+</html>
\ No newline at end of file
index 9a228b1..59e23fb 100644 (file)
@@ -1,3 +1,132 @@
+2018-12-20  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Focusing an editable element should scroll to reveal the selection
+        https://bugs.webkit.org/show_bug.cgi?id=192802
+        <rdar://problem/46781759>
+
+        Reviewed by Tim Horton.
+
+        Currently, when tapping on an editable element, logic in -[WKWebView _zoomToFocusRect:…:] attempts to adjust the
+        visible viewport such that the rect containing the selection is visible. However, AssistedNodeInformation's
+        selectionRect is used here, which (as the FIXME in WebPage::getAssistedNodeInformation notes) is either the last
+        touch location, or the top left of the element if the touch location is outside of the element's bounding rect.
+        This leads to confusing and undesirable behavior when tapping near the bottom of a large contenteditable element
+        to focus it, since the actual selection will end up near the top of the element, yet we'll try to scroll to
+        reveal the bottom of the element, which causes the visible selection to scroll offscreen. Notably, this affects
+        scenarios involving editable web views embedded in apps, such as Mail compose.
+
+        Right now, we use the last touch location as an approximation for the selection rect because the selection may
+        have not yet been updated at the moment when focus moves into an editable element. To fix this, we defer the
+        process of zooming to the focused element rect until after the selection changes and the UI process is updated
+        with information about the new selection rects.
+
+        Test: editing/selection/ios/selection-is-visible-after-focusing-editable-area.html
+
+        * Shared/AssistedNodeInformation.cpp:
+        (WebKit::AssistedNodeInformation::encode const):
+        (WebKit::AssistedNodeInformation::decode):
+        * Shared/AssistedNodeInformation.h:
+
+        Rename selectionRect to elementInteractionLocation, to more accurately reflect its value and purpose. This isn't
+        strictly always the last touch location, since we may default to the focused element location instead if the
+        last touch location is outside of the element rect.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _zoomToFocusRect:selectionRect:insideFixed:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):
+
+        Tweak a constant that determines the minimum amount of margin to leave between the selection rect and the edge
+        of the window when scrolling to reveal the focused element. Previously, this was larger than necessary to
+        accomodate for the fact that the "selection rect" used when zooming to the focused element did not take the
+        actual selection into account at all, and was simply a 1 by 1 rect; this meant that the margin needed to be
+        large enough to exceed the usual height of a text caret in editable content. Since we now use the real selection
+        rect, we can be much less generous with this margin.
+
+        * UIProcess/ios/WKContentViewInteraction.h:
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView cleanupInteraction]):
+        (-[WKContentView observeValueForKeyPath:ofObject:change:context:]):
+
+        Don't additionally update the selection in the middle of triggering zooming to the focused element; on
+        particular versions of iOS, this now attempts to scroll the selection rect on-screen, which then conflicts with
+        zooming to reveal the focused element.
+
+        (-[WKContentView _zoomToRevealFocusedElement]):
+
+        Renamed from _displayFormNodeInputView to _zoomToRevealFocusedElement, to make the purpose of this function more
+        clear. Additionally, pull logic to update the accessory view out of this method, so that it's strictly concerned
+        with zooming to the focused element.
+
+        (-[WKContentView inputView]):
+
+        Add a FIXME describing the implications of zooming to the focused element in the implementation of -inputView.
+        See also: <https://bugs.webkit.org/show_bug.cgi?id=192878>.
+
+        (-[WKContentView accessoryTab:]):
+
+        Fix a subtle issue when keeping track of _didAccessoryTabInitiateFocus. Currently, this is set to YES in
+        -accessoryTab: and unset in _displayFormNodeInputView, but since _displayFormNodeInputView may be invoked
+        multiple times for the same focused element (see: -inputView), we might end up zooming to the focused element
+        with _didAccessoryTabInitiateFocus set to NO, even though we initiated focus with the previous/next buttons.
+
+        Instead, temporarily set a different ivar, _isChangingFocusUsingAccessoryTab, to YES in -accessoryTab:, and
+        unset it in the completion handler after the focused element has changed. Then, when we _startAssistingNode:,
+        set _didAccessoryTabInitiateFocus to _isChangingFocusUsingAccessoryTab. This ensures that the correctness of
+        _didAccessoryTabInitiateFocus isn't tied to the number of times -[WKContentView inputView] is invoked when
+        focusing an element.
+
+        (shouldZoomToRevealSelectionRect):
+        (rectToRevealWhenZoomingToFocusedElement):
+
+        Add a helper method to determine the selection rect to use when zooming to reveal the focused element. ASSERTs
+        that we have post-layout data in the EditorState.
+
+        (-[WKContentView _startAssistingNode:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
+
+        When "assisting" a focused element, immediately zoom to it if we don't need selection information to compute the
+        rect to zoom to; otherwise, defer zooming until we receive the first editor state update in the UI process that
+        contains information about our selection rects.
+
+        (-[WKContentView _stopAssistingNode]):
+        (-[WKContentView _didReceiveEditorStateUpdateAfterFocus]):
+
+        If necessary, reveal the focused element by zooming.
+
+        (-[WKContentView _updateInitialWritingDirectionIfNecessary]):
+
+        Pull this initial writing direction update logic out into a separate helper method.
+
+        (-[WKContentView _displayFormNodeInputView]): Deleted.
+
+        Replaced by _zoomToRevealFocusedElement.
+
+        * WebProcess/WebCoreSupport/WebChromeClient.cpp:
+        (WebKit::WebChromeClient::elementDidRefocus):
+
+        This currently calls WebChromeClient::elementDidFocus; instead, call the new WebPage::elementDidRefocus;
+        additionally, make this available on all PLATFORM(COCOA), rather than just IOS_FAMILY.
+
+        * WebProcess/WebCoreSupport/WebChromeClient.h:
+        * WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm:
+        (WebKit::WebChromeClient::elementDidRefocus): Deleted.
+
+        Replaced by the PLATFORM(COCOA) version.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::elementDidRefocus):
+
+        When refocusing an element, ensure that post-layout editor state data is sent to the UI process by including a
+        full EditorState in the next layer tree transaction.
+
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::WebPage::completeSyntheticClick):
+
+        Call elementDidRefocus instead of elementDidFocus, in the case where the existing focused element is clicked.
+
+        (WebKit::WebPage::getAssistedNodeInformation):
+
+        Adjust for the change from selectionRect to elementInteractionLocation.
+
 2018-12-20  Patrick Griffis  <pgriffis@igalia.com>
 
         [GTK][WPE] Grant the sandbox read access to XDG_DATA_HOME/prgname
index 744148a..dde4bec 100644 (file)
@@ -64,7 +64,7 @@ Optional<OptionItem> OptionItem::decode(IPC::Decoder& decoder)
 void AssistedNodeInformation::encode(IPC::Encoder& encoder) const
 {
     encoder << elementRect;
-    encoder << selectionRect;
+    encoder << elementInteractionLocation;
     encoder << minimumScaleFactor;
     encoder << maximumScaleFactor;
     encoder << maximumScaleFactorIgnoringAlwaysScalable;
@@ -112,7 +112,7 @@ bool AssistedNodeInformation::decode(IPC::Decoder& decoder, AssistedNodeInformat
     if (!decoder.decode(result.elementRect))
         return false;
 
-    if (!decoder.decode(result.selectionRect))
+    if (!decoder.decode(result.elementInteractionLocation))
         return false;
 
     if (!decoder.decode(result.minimumScaleFactor))
index 6b04f49..c247f89 100644 (file)
@@ -95,7 +95,7 @@ struct OptionItem {
 
 struct AssistedNodeInformation {
     WebCore::IntRect elementRect;
-    WebCore::IntRect selectionRect;
+    WebCore::IntPoint elementInteractionLocation;
     double minimumScaleFactor { -INFINITY };
     double maximumScaleFactor { INFINITY };
     double maximumScaleFactorIgnoringAlwaysScalable { INFINITY };
index 4d1f025..3d3c1ed 100644 (file)
@@ -2247,7 +2247,7 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
 
     const double minimumHeightToShowContentAboveKeyboard = 106;
     const CFTimeInterval formControlZoomAnimationDuration = 0.25;
-    const double caretOffsetFromWindowEdge = 20;
+    const double caretOffsetFromWindowEdge = 8;
 
     UIWindow *window = [_scrollView window];
 
index 25875dd..4d8c140 100644 (file)
@@ -301,9 +301,11 @@ struct WKAutoCorrectionData {
     BOOL _shouldRestoreSelection;
     BOOL _usingGestureForSelection;
     BOOL _inspectorNodeSearchEnabled;
+    BOOL _isChangingFocusUsingAccessoryTab;
     BOOL _didAccessoryTabInitiateFocus;
     BOOL _isExpectingFastSingleTapCommit;
     BOOL _showDebugTapHighlightsForFastClicking;
+    BOOL _isZoomingToRevealFocusedElement;
 
     BOOL _becomingFirstResponder;
     BOOL _resigningFirstResponder;
index d348eb5..5e399fd 100644 (file)
@@ -757,6 +757,7 @@ static inline bool hasAssistedNode(WebKit::AssistedNodeInformation assistedNodeI
     
     _smartMagnificationController = nil;
     _didAccessoryTabInitiateFocus = NO;
+    _isChangingFocusUsingAccessoryTab = NO;
     _isExpectingFastSingleTapCommit = NO;
     _needsDeferredEndScrollingSelectionUpdate = NO;
     [_formInputSession invalidate];
@@ -854,6 +855,7 @@ static inline bool hasAssistedNode(WebKit::AssistedNodeInformation assistedNodeI
 
     _hasSetUpInteractions = NO;
     _suppressSelectionAssistantReasons = { };
+    _isZoomingToRevealFocusedElement = NO;
 }
 
 - (void)_removeDefaultGestureRecognizers
@@ -941,9 +943,16 @@ static inline bool hasAssistedNode(WebKit::AssistedNodeInformation assistedNodeI
         [UIView _addCompletion:^(BOOL){ [_interactionViewsContainerView setHidden:NO]; }];
     }
 
+    [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];
-    [self _updateTapHighlight];
 }
 
 - (void)_enableInspectorNodeSearch
@@ -1387,24 +1396,22 @@ static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius,
     return YES;
 }
 
-- (void)_displayFormNodeInputView
+- (void)_zoomToRevealFocusedElement
 {
-    if (!_suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTransparent)) {
-        // 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:_assistedNodeInformation.elementRect
-            selectionRect:_didAccessoryTabInitiateFocus ? WebCore::IntRect() : _assistedNodeInformation.selectionRect
-            insideFixed:_assistedNodeInformation.insideFixedPosition
-            fontSize:_assistedNodeInformation.nodeFontSize
-            minimumScale:_assistedNodeInformation.minimumScaleFactor
-            maximumScale:_assistedNodeInformation.maximumScaleFactorIgnoringAlwaysScalable
-            allowScaling:_assistedNodeInformation.allowsUserScalingIgnoringAlwaysScalable && !currentUserInterfaceIdiomIsPad()
-            forceScroll:(_assistedNodeInformation.inputMode == WebCore::InputMode::None) ? !currentUserInterfaceIdiomIsPad() : [self requiresAccessoryView]];
-    }
+    if (_suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTransparent))
+        return;
 
-    _didAccessoryTabInitiateFocus = NO;
-    [self _ensureFormAccessoryView];
-    [self _updateAccessory];
+    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:_assistedNodeInformation.elementRect
+        selectionRect:_didAccessoryTabInitiateFocus ? WebCore::FloatRect() : rectToRevealWhenZoomingToFocusedElement(_assistedNodeInformation, _page->editorState())
+        insideFixed:_assistedNodeInformation.insideFixedPosition
+        fontSize:_assistedNodeInformation.nodeFontSize
+        minimumScale:_assistedNodeInformation.minimumScaleFactor
+        maximumScale:_assistedNodeInformation.maximumScaleFactorIgnoringAlwaysScalable
+        allowScaling:_assistedNodeInformation.allowsUserScalingIgnoringAlwaysScalable && !currentUserInterfaceIdiomIsPad()
+        forceScroll:(_assistedNodeInformation.inputMode == WebCore::InputMode::None) ? !currentUserInterfaceIdiomIsPad() : [self requiresAccessoryView]];
 }
 
 - (UIView *)inputView
@@ -1429,8 +1436,19 @@ static NSValue *nsSizeForTapHighlightBorderRadius(WebCore::IntSize borderRadius,
             _inputPeripheral = adoptNS([[WKFormInputControl alloc] initWithView:self]);
             break;
         }
-    } else
-        [self _displayFormNodeInputView];
+    } else {
+        // FIXME: UIKit may invoke -[WKContentView inputView] at any time when WKContentView is the first responder;
+        // as such, it doesn't make sense to change the enclosing scroll view's zoom scale and content offset to reveal
+        // the focused element here. It seems this behavior was added to match logic in legacy WebKit (refer to
+        // UIWebBrowserView). Instead, we should find the places where we currently assume that UIKit (or other clients)
+        // invoke -inputView to zoom to the focused element, and either surface SPI for clients to zoom to the focused
+        // element, or explicitly trigger the zoom from WebKit.
+        // For instance, one use case that currently relies on this detail is adjusting the zoom scale and viewport upon
+        // rotation, when a select element is focused. See <https://webkit.org/b/192878> for more information.
+        [self _zoomToRevealFocusedElement];
+        [self _ensureFormAccessoryView];
+        [self _updateAccessory];
+    }
 
     if (UIView *customInputView = [_formInputSession customInputView])
         return customInputView;
@@ -3330,12 +3348,13 @@ static void selectionChangedWithTouch(WKContentView *view, const WebCore::IntPoi
     [self _endEditing];
     _inputPeripheral = nil;
 
-    _didAccessoryTabInitiateFocus = YES; // Will be cleared in either -_displayFormNodeInputView or -cleanupInteraction.
+    _isChangingFocusUsingAccessoryTab = YES;
     [self beginSelectionChange];
     RetainPtr<WKContentView> view = self;
     _page->focusNextAssistedNode(isNext, [view](WebKit::CallbackBase::Error) {
         [view endSelectionChange];
         [view reloadInputViews];
+        view->_isChangingFocusUsingAccessoryTab = NO;
     });
 }
 
@@ -4348,6 +4367,51 @@ static NSString *contentTypeFromFieldName(WebCore::AutofillFieldName fieldName)
     return _formAccessoryView.get();
 }
 
+static bool shouldZoomToRevealSelectionRect(WebKit::InputType type)
+{
+    switch (type) {
+    case WebKit::InputType::ContentEditable:
+    case WebKit::InputType::Text:
+    case WebKit::InputType::Password:
+    case WebKit::InputType::TextArea:
+    case WebKit::InputType::Search:
+    case WebKit::InputType::Email:
+    case WebKit::InputType::URL:
+    case WebKit::InputType::Phone:
+    case WebKit::InputType::Number:
+    case WebKit::InputType::NumberPad:
+        return true;
+    default:
+        return false;
+    }
+}
+
+static WebCore::FloatRect rectToRevealWhenZoomingToFocusedElement(const WebKit::AssistedNodeInformation& elementInfo, const WebKit::EditorState& editorState)
+{
+    WebCore::IntRect elementInteractionRect(elementInfo.elementInteractionLocation, { 1, 1 });
+    if (!shouldZoomToRevealSelectionRect(elementInfo.elementType))
+        return elementInteractionRect;
+
+    if (editorState.isMissingPostLayoutData) {
+        ASSERT_NOT_REACHED();
+        return elementInteractionRect;
+    }
+
+    if (editorState.selectionIsNone)
+        return { };
+
+    WebCore::FloatRect selectionBoundingRect;
+    auto& postLayoutData = editorState.postLayoutData();
+    if (editorState.selectionIsRange) {
+        for (auto& rect : postLayoutData.selectionRects)
+            selectionBoundingRect.unite(rect.rect());
+    } else
+        selectionBoundingRect = postLayoutData.caretRectAtStart;
+
+    selectionBoundingRect.intersect(elementInfo.elementRect);
+    return selectionBoundingRect;
+}
+
 static bool isAssistableInputType(WebKit::InputType type)
 {
     switch (type) {
@@ -4387,6 +4451,8 @@ static bool isAssistableInputType(WebKit::InputType type)
     SetForScope<BOOL> isChangingFocusForScope { _isChangingFocus, hasAssistedNode(_assistedNodeInformation) };
     _inputViewUpdateDeferrer = nullptr;
 
+    _didAccessoryTabInitiateFocus = _isChangingFocusUsingAccessoryTab;
+
     id <_WKInputDelegate> inputDelegate = [_webView _inputDelegate];
     RetainPtr<WKFocusedElementInfo> focusedElementInfo = adoptNS([[WKFocusedElementInfo alloc] initWithAssistedNodeInformation:information isUserInitiated:userIsInteracting userObject:userObject]);
 
@@ -4506,7 +4572,11 @@ static bool isAssistableInputType(WebKit::InputType type)
     if (editableChanged)
         [_webView _scheduleVisibleContentRectUpdate];
     
-    [self _displayFormNodeInputView];
+    if (!shouldZoomToRevealSelectionRect(_assistedNodeInformation.elementType))
+        [self _zoomToRevealFocusedElement];
+
+    [self _ensureFormAccessoryView];
+    [self _updateAccessory];
 
 #if PLATFORM(WATCHOS)
     if (_isChangingFocus)
@@ -4563,10 +4633,23 @@ static bool isAssistableInputType(WebKit::InputType type)
     [_webView didEndFormControlInteraction];
 
     [self _stopSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTransparent];
+
+    if (!_isChangingFocus)
+        _didAccessoryTabInitiateFocus = NO;
 }
 
 - (void)_didReceiveEditorStateUpdateAfterFocus
 {
+    [self _updateInitialWritingDirectionIfNecessary];
+
+    // FIXME: If the initial writing direction just changed, we should wait until we get the next post-layout editor state
+    // before zooming to reveal the selection rect.
+    if (shouldZoomToRevealSelectionRect(_assistedNodeInformation.elementType))
+        [self _zoomToRevealFocusedElement];
+}
+
+- (void)_updateInitialWritingDirectionIfNecessary
+{
     if (!_page->isEditable())
         return;
 
index 0264d74..bf64714 100644 (file)
@@ -203,6 +203,11 @@ void WebChromeClient::elementDidFocus(Element& element)
     m_page.elementDidFocus(&element);
 }
 
+void WebChromeClient::elementDidRefocus(Element& element)
+{
+    m_page.elementDidRefocus(&element);
+}
+
 void WebChromeClient::elementDidBlur(Element& element)
 {
     m_page.elementDidBlur(&element);
index 9473ca3..b1415d1 100644 (file)
@@ -252,10 +252,6 @@ private:
     RefPtr<WebCore::ScrollingCoordinator> createScrollingCoordinator(WebCore::Page&) const final;
 #endif
 
-#if PLATFORM(IOS_FAMILY)
-    void elementDidRefocus(WebCore::Element&) final;
-#endif
-
 #if (PLATFORM(IOS_FAMILY) && HAVE(AVKIT)) || (PLATFORM(MAC) && ENABLE(VIDEO_PRESENTATION_MODE))
     bool supportsVideoFullscreen(WebCore::HTMLMediaElementEnums::VideoFullscreenMode) final;
     bool supportsVideoFullscreenStandby() final;
@@ -278,6 +274,7 @@ private:
 #if PLATFORM(COCOA)
     void elementDidFocus(WebCore::Element&) final;
     void elementDidBlur(WebCore::Element&) final;
+    void elementDidRefocus(WebCore::Element&) final;
 
     void makeFirstResponder() final;
     void assistiveTechnologyMakeFirstResponder() final;
index f6a0280..f7f4e93 100644 (file)
@@ -53,11 +53,6 @@ void WebChromeClient::didPreventDefaultForEvent()
 
 #endif
 
-void WebChromeClient::elementDidRefocus(WebCore::Element& element)
-{
-    elementDidFocus(element);
-}
-
 void WebChromeClient::didReceiveMobileDocType(bool isMobileDoctype)
 {
     m_page.didReceiveMobileDocType(isMobileDoctype);
index a87c529..c29bd4d 100644 (file)
@@ -5278,6 +5278,12 @@ void WebPage::resetAssistedNodeForFrame(WebFrame* frame)
     }
 }
 
+void WebPage::elementDidRefocus(WebCore::Node* node)
+{
+    elementDidFocus(node);
+    m_hasPendingEditorStateUpdate = true;
+}
+
 void WebPage::elementDidFocus(WebCore::Node* node)
 {
     if (m_assistedNode == node && m_isAssistingNodeDueToUserInteraction)
index d69faba..a71cb04 100644 (file)
@@ -585,7 +585,9 @@ public:
     void captureDevicesChanged();
 #endif
 
+    // FIXME: These should all take Element& instead of Node*.
     void elementDidFocus(WebCore::Node*);
+    void elementDidRefocus(WebCore::Node*);
     void elementDidBlur(WebCore::Node*);
     void resetAssistedNodeForFrame(WebFrame*);
 
index 9f91731..9f99b4b 100644 (file)
@@ -615,7 +615,7 @@ void WebPage::completeSyntheticClick(Node* nodeRespondingToClick, const WebCore:
     // If the node has been focused by JavaScript without user interaction, the
     // keyboard is not on screen.
     if (newFocusedElement && newFocusedElement == oldFocusedElement)
-        elementDidFocus(newFocusedElement.get());
+        elementDidRefocus(newFocusedElement.get());
 
     if (!tapWasHandled || !nodeRespondingToClick || !nodeRespondingToClick->isElementNode())
         send(Messages::WebPageProxy::DidNotHandleTapAsClick(roundedIntPoint(location)));
@@ -2363,9 +2363,7 @@ void WebPage::getAssistedNodeInformation(AssistedNodeInformation& information)
 {
     layoutIfNeeded();
 
-    // FIXME: information.selectionRect should be set to the actual selection rect, but when this is called at focus time
-    // we don't have a selection yet. Using the last interaction location is a reasonable approximation for now.
-    information.selectionRect = IntRect(m_lastInteractionLocation, IntSize(1, 1));
+    information.elementInteractionLocation = m_lastInteractionLocation;
 
     if (RenderObject* renderer = m_assistedNode->renderer()) {
         Frame& elementFrame = m_page->focusController().focusedOrMainFrame();
@@ -2386,11 +2384,11 @@ void WebPage::getAssistedNodeInformation(AssistedNodeInformation& information)
             frameView->setCustomFixedPositionLayoutRect(currentFixedPositionRect);
             
             if (!information.elementRect.contains(m_lastInteractionLocation))
-                information.selectionRect.setLocation(information.elementRect.location());
+                information.elementInteractionLocation = information.elementRect.location();
         } else {
             // Don't use the selection rect if interaction was outside the element rect.
             if (!information.elementRect.contains(m_lastInteractionLocation))
-                information.selectionRect = IntRect();
+                information.elementInteractionLocation = { };
         }
     } else
         information.elementRect = IntRect();