Attachment 357790 Details for Bug 192802 – Patch for EWS

[patch] Patch for EWS

bug-192802-20181219225836.patch (text/plain), 30.65 KB, created by Wenson Hsieh on 2018-12-19 22:58:37 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-12-19 22:58:37 PST

Size: 30.65 KB

patch

obsolete

>Subversion Revision: 239368
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 6f6d61c2d9bac3d786c0381b3e7f67cb6326ea9f..d65852f9beab5e0391c34f8e8322219cd43c7665 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,132 @@
>+2018-12-19  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 NOBODY (OOPS!).
>+
>+        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-18  Alex Christensen  <achristensen@webkit.org>
> 
>         REGRESSION(r239134) iOS safe browsing warning unable to show details
>diff --git a/Source/WebKit/Shared/AssistedNodeInformation.cpp b/Source/WebKit/Shared/AssistedNodeInformation.cpp
>index 5134791800315060a9ce8ff2cbaed31e93f22a72..bec9c8fb70ac6927aea39768a3b2fc6e213886fb 100644
>--- a/Source/WebKit/Shared/AssistedNodeInformation.cpp
>+++ b/Source/WebKit/Shared/AssistedNodeInformation.cpp
>@@ -64,7 +64,7 @@ std::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))
>diff --git a/Source/WebKit/Shared/AssistedNodeInformation.h b/Source/WebKit/Shared/AssistedNodeInformation.h
>index 9b1c3b0145f83f794851774e96b8586ae72d8d19..d7752544d5aa8786f1abcb68ee608f8678f99042 100644
>--- a/Source/WebKit/Shared/AssistedNodeInformation.h
>+++ b/Source/WebKit/Shared/AssistedNodeInformation.h
>@@ -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 };
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 9f65f2ee7a32791ce25a605cfb2817f0c3ed441f..b77f2957fc6c0c94e502eeb5ea38a55bd90ccca0 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -2233,7 +2233,7 @@ - (void)_zoomToFocusRect:(const WebCore::FloatRect&)focusedElementRectInDocument
> 
>     const double minimumHeightToShowContentAboveKeyboard = 106;
>     const CFTimeInterval formControlZoomAnimationDuration = 0.25;
>-    const double caretOffsetFromWindowEdge = 20;
>+    const double caretOffsetFromWindowEdge = 8;
> 
>     UIWindow *window = [_scrollView window];
> 
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>index e794e3c631ddeb53a1a469b7909429b4cba3bc61..f1f156ed0f95d5b88addb41820a06d39b1104985 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>@@ -296,9 +296,11 @@ struct WKAutoCorrectionData {
>     BOOL _shouldRestoreSelection;
>     BOOL _usingGestureForSelection;
>     BOOL _inspectorNodeSearchEnabled;
>+    BOOL _isChangingFocusUsingAccessoryTab;
>     BOOL _didAccessoryTabInitiateFocus;
>     BOOL _isExpectingFastSingleTapCommit;
>     BOOL _showDebugTapHighlightsForFastClicking;
>+    BOOL _isZoomingToRevealFocusedElement;
> 
>     BOOL _becomingFirstResponder;
>     BOOL _resigningFirstResponder;
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index dee8988c420adccd57edb2cdc9b429a1f5e9c7b4..403808c919c140fbd8be810f5b1e776a6c58c2e2 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -757,6 +757,7 @@ - (void)cleanupInteraction
>     
>     _smartMagnificationController = nil;
>     _didAccessoryTabInitiateFocus = NO;
>+    _isChangingFocusUsingAccessoryTab = NO;
>     _isExpectingFastSingleTapCommit = NO;
>     _needsDeferredEndScrollingSelectionUpdate = NO;
>     [_formInputSession invalidate];
>@@ -854,6 +855,7 @@ - (void)cleanupInteraction
> 
>     _hasSetUpInteractions = NO;
>     _suppressSelectionAssistantReasons = { };
>+    _isZoomingToRevealFocusedElement = NO;
> }
> 
> - (void)_removeDefaultGestureRecognizers
>@@ -941,9 +943,16 @@ - (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(N
>         [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
>@@ -1388,24 +1397,22 @@ - (BOOL)_requiresKeyboardResetOnReload
>     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
>@@ -1438,8 +1445,19 @@ - (UIView *)inputView
>             _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;
>@@ -3340,12 +3358,13 @@ - (void)accessoryTab:(BOOL)isNext
>     [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;
>     });
> }
> 
>@@ -4358,6 +4377,51 @@ - (UIWebFormAccessory *)formAccessoryView
>     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) {
>@@ -4397,6 +4461,8 @@ - (void)_startAssistingNode:(const WebKit::AssistedNodeInformation&)information
>     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]);
> 
>@@ -4510,7 +4576,11 @@ - (void)_startAssistingNode:(const WebKit::AssistedNodeInformation&)information
>     if (editableChanged)
>         [_webView _scheduleVisibleContentRectUpdate];
>     
>-    [self _displayFormNodeInputView];
>+    if (!shouldZoomToRevealSelectionRect(_assistedNodeInformation.elementType))
>+        [self _zoomToRevealFocusedElement];
>+
>+    [self _ensureFormAccessoryView];
>+    [self _updateAccessory];
> 
> #if PLATFORM(WATCHOS)
>     if (_isChangingFocus)
>@@ -4562,9 +4632,22 @@ - (void)_stopAssistingNode
>     [_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;
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>index b31b117f0edab398b25124064e16227faf786255..18e2ce840055b5b52cc5480789e2b5d1a46cd315 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.cpp
>@@ -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);
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>index 4315126b0648e3447fc0b9cfbc88023c001bfca9..bbe10a7c42d0373969422fc133be6c4665decd70 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/WebChromeClient.h
>@@ -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;
>diff --git a/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm b/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm
>index f6a028020324879febddb2734ea69701ffda8915..f7f4e93512c42500273203d82f461538fc26fc99 100644
>--- a/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm
>+++ b/Source/WebKit/WebProcess/WebCoreSupport/ios/WebChromeClientIOS.mm
>@@ -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);
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index be6befb8147865fc0e81e81223b6c496ab7f1bd5..8021d1628b0bb0a821692019b464bf25ebc68987 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -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)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 4770868074548087b73c36c658b0edfb4e0a6b24..674c125c7543f2b0a67f1790281a7468ab422439 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -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*);
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 9df618a4d9ad8e9b1d63206c60051dafb76db235..c00a6cb3a5b3b16befa70ba645ee191b08f1dd6d 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -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();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 821c75729f3285ce23ea580cf366877608b2de67..108025a484d49e539798a12de733bc19fbdce440 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2018-12-19  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 NOBODY (OOPS!).
>+
>+        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-18  Justin Michaud  <justin_michaud@apple.com>
> 
>         Update CSS Properties and Values API to use new cycle fallback behaviour
>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
>index 0000000000000000000000000000000000000000..3863ad0f83e865c906213757b43f2ab0f250f9ae
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..2f40636707d4402b99c53eab0c67d59aff43ec52
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/selection-is-visible-after-focusing-editable-area.html
>@@ -0,0 +1,64 @@
>+<!DOCTYPE html> 
>+<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

Actions: View | Formatted Diff | Diff

Attachments on bug 192802: 357790