Title: [239441] trunk
Revision
239441
Author
wenson_hs...@apple.com
Date
2018-12-20 08:28:55 -0800 (Thu, 20 Dec 2018)

Log Message

[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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (239440 => 239441)


--- trunk/LayoutTests/ChangeLog	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/LayoutTests/ChangeLog	2018-12-20 16:28:55 UTC (rev 239441)
@@ -1,3 +1,17 @@
+2018-12-20  Wenson Hsieh  <wenson_hs...@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.kirsl...@sony.com>
 
         [WinCairo] Unreviewed test gardening.

Added: trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt (0 => 239441)


--- trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt	2018-12-20 16:28:55 UTC (rev 239441)
@@ -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

Added: trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html (0 => 239441)


--- trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html	2018-12-20 16:28:55 UTC (rev 239441)
@@ -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=""
+    <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

Modified: trunk/Source/WebKit/ChangeLog (239440 => 239441)


--- trunk/Source/WebKit/ChangeLog	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/ChangeLog	2018-12-20 16:28:55 UTC (rev 239441)
@@ -1,3 +1,132 @@
+2018-12-20  Wenson Hsieh  <wenson_hs...@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  <pgrif...@igalia.com>
 
         [GTK][WPE] Grant the sandbox read access to XDG_DATA_HOME/prgname

Modified: trunk/Source/WebKit/Shared/AssistedNodeInformation.cpp (239440 => 239441)


--- trunk/Source/WebKit/Shared/AssistedNodeInformation.cpp	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/Shared/AssistedNodeInformation.cpp	2018-12-20 16:28:55 UTC (rev 239441)
@@ -64,7 +64,7 @@
 void AssistedNodeInformation::encode(IPC::Encoder& encoder) const
 {
     encoder << elementRect;
-    encoder << selectionRect;
+    encoder << elementInteractionLocation;
     encoder << minimumScaleFactor;
     encoder << maximumScaleFactor;
     encoder << maximumScaleFactorIgnoringAlwaysScalable;
@@ -112,7 +112,7 @@
     if (!decoder.decode(result.elementRect))
         return false;
 
-    if (!decoder.decode(result.selectionRect))
+    if (!decoder.decode(result.elementInteractionLocation))
         return false;
 
     if (!decoder.decode(result.minimumScaleFactor))

Modified: trunk/Source/WebKit/Shared/AssistedNodeInformation.h (239440 => 239441)


--- trunk/Source/WebKit/Shared/AssistedNodeInformation.h	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/Shared/AssistedNodeInformation.h	2018-12-20 16:28:55 UTC (rev 239441)
@@ -95,7 +95,7 @@
 
 struct AssistedNodeInformation {
     WebCore::IntRect elementRect;
-    WebCore::IntRect selectionRect;
+    WebCore::IntPoint elementInteractionLocation;
     double minimumScaleFactor { -INFINITY };
     double maximumScaleFactor { INFINITY };
     double maximumScaleFactorIgnoringAlwaysScalable { INFINITY };

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (239440 => 239441)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2018-12-20 16:28:55 UTC (rev 239441)
@@ -2247,7 +2247,7 @@
 
     const double minimumHeightToShowContentAboveKeyboard = 106;
     const CFTimeInterval formControlZoomAnimationDuration = 0.25;
-    const double caretOffsetFromWindowEdge = 20;
+    const double caretOffsetFromWindowEdge = 8;
 
     UIWindow *window = [_scrollView window];
 

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h (239440 => 239441)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h	2018-12-20 16:28:55 UTC (rev 239441)
@@ -301,9 +301,11 @@
     BOOL _shouldRestoreSelection;
     BOOL _usingGestureForSelection;
     BOOL _inspectorNodeSearchEnabled;
+    BOOL _isChangingFocusUsingAccessoryTab;
     BOOL _didAccessoryTabInitiateFocus;
     BOOL _isExpectingFastSingleTapCommit;
     BOOL _showDebugTapHighlightsForFastClicking;
+    BOOL _isZoomingToRevealFocusedElement;
 
     BOOL _becomingFirstResponder;
     BOOL _resigningFirstResponder;

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (239440 => 239441)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2018-12-20 16:28:55 UTC (rev 239441)
@@ -757,6 +757,7 @@
     
     _smartMagnificationController = nil;
     _didAccessoryTabInitiateFocus = NO;
+    _isChangingFocusUsingAccessoryTab = NO;
     _isExpectingFastSingleTapCommit = NO;
     _needsDeferredEndScrollingSelectionUpdate = NO;
     [_formInputSession invalidate];
@@ -854,6 +855,7 @@
 
     _hasSetUpInteractions = NO;
     _suppressSelectionAssistantReasons = { };
+    _isZoomingToRevealFocusedElement = NO;
 }
 
 - (void)_removeDefaultGestureRecognizers
@@ -941,9 +943,16 @@
         [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 @@
     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 @@
             _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 @@
     [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 @@
     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 @@
     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,8 +4572,12 @@
     if (editableChanged)
         [_webView _scheduleVisibleContentRectUpdate];
     
-    [self _displayFormNodeInputView];
+    if (!shouldZoomToRevealSelectionRect(_assistedNodeInformation.elementType))
+        [self _zoomToRevealFocusedElement];
 
+    [self _ensureFormAccessoryView];
+    [self _updateAccessory];
+
 #if PLATFORM(WATCHOS)
     if (_isChangingFocus)
         [_focusedFormControlView reloadData:YES];
@@ -4563,10 +4633,23 @@
     [_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;
 

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp	2018-12-20 16:28:55 UTC (rev 239441)
@@ -203,6 +203,11 @@
     m_page.elementDidFocus(&element);
 }
 
+void WebChromeClient::elementDidRefocus(Element& element)
+{
+    m_page.elementDidRefocus(&element);
+}
+
 void WebChromeClient::elementDidBlur(Element& element)
 {
     m_page.elementDidBlur(&element);

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h	2018-12-20 16:28:55 UTC (rev 239441)
@@ -252,10 +252,6 @@
     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 @@
 #if PLATFORM(COCOA)
     void elementDidFocus(WebCore::Element&) final;
     void elementDidBlur(WebCore::Element&) final;
+    void elementDidRefocus(WebCore::Element&) final;
 
     void makeFirstResponder() final;
     void assistiveTechnologyMakeFirstResponder() final;

Modified: trunk/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm	2018-12-20 16:28:55 UTC (rev 239441)
@@ -53,11 +53,6 @@
 
 #endif
 
-void WebChromeClient::elementDidRefocus(WebCore::Element& element)
-{
-    elementDidFocus(element);
-}
-
 void WebChromeClient::didReceiveMobileDocType(bool isMobileDoctype)
 {
     m_page.didReceiveMobileDocType(isMobileDoctype);

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.cpp	2018-12-20 16:28:55 UTC (rev 239441)
@@ -5278,6 +5278,12 @@
     }
 }
 
+void WebPage::elementDidRefocus(WebCore::Node* node)
+{
+    elementDidFocus(node);
+    m_hasPendingEditorStateUpdate = true;
+}
+
 void WebPage::elementDidFocus(WebCore::Node* node)
 {
     if (m_assistedNode == node && m_isAssistingNodeDueToUserInteraction)

Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h	2018-12-20 16:28:55 UTC (rev 239441)
@@ -585,7 +585,9 @@
     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*);
 

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (239440 => 239441)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2018-12-20 16:18:26 UTC (rev 239440)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2018-12-20 16:28:55 UTC (rev 239441)
@@ -615,7 +615,7 @@
     // 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 @@
 {
     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 @@
             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();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to