Changeset 277352 in webkit

Timestamp:

May 11, 2021, 8:46:21 PM (5 years ago)

Author:

Wenson Hsieh

Message:

[iOS] Mail compose web view doesn't scroll to reveal the selection in certain configurations
​https://bugs.webkit.org/show_bug.cgi?id=225675
rdar://77095886

Reviewed by Tim Horton.

Source/WebKit:

Currently when computing input view bounds upon receiving UIKeyboardDidChangeFrameNotification, we attempt to
map the on-screen bounds of the keyboard to the window's coordinate space, and save the result in an ivar in
WKWebView, _inputViewBounds. The keyboard frame (which corresponds to UIKeyboardFrameEndUserInfoKey in the
notification's userInfo dictionary) is given to us in screen coordinates, and we currently pass this through
-convertRect:fromWindow:, with a nil UIWindow.

However, this results in mapping the rect from the coordinate space of the window's UIWindowScene rather than
the window screen. In shipping Mail on iOS, this doesn't matter because the window containing the compose web
view shares the same coordinate space as the screen. In some other configurations of MobileMail, however, the
compose web view appears inside its own UIWindow. This causes the above coordinate conversion logic to fail,
since we attempt to map a rect given to us in screen coordinates from the compose web view's window scene's
coordinate space, instead of the screen's coordinate space.

We fix this by using -convertRect:fromCoordinateSpace: instead, and explicitly pass in
self.window.screen.coordinateSpace.

• UIProcess/API/Cocoa/WKWebViewInternal.h:
• UIProcess/API/ios/WKWebViewIOS.mm:

(-[WKWebView _updateScrollViewForTransaction:]):
(-[WKWebView _zoomToFocusRect:selectionRect:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):
(-[WKWebView _contentRectForUserInteraction]):
(-[WKWebView _updateVisibleContentRects]):
(-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):

Rename _inputViewBounds to _inputViewBoundsInWindow for clarity.

• UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:
• UIProcess/API/ios/WKWebViewTestingIOS.mm:

(-[WKWebView _inputViewBoundsInWindow]):
(-[WKWebView _inputViewBounds]): Deleted.

Tools:

Rename some testing SPI. See WebKit/ChangeLog for more information.

• WebKitTestRunner/ios/UIScriptControllerIOS.mm:

(WTR::UIScriptControllerIOS::inputViewBounds const):

Location:

trunk

Files:

• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm (modified) (6 diffs)
• Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h (modified) (1 diff)
• Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-05-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Mail compose web view doesn't scroll to reveal the selection in certain configurations
+        https://bugs.webkit.org/show_bug.cgi?id=225675
+        rdar://77095886
+
+        Reviewed by Tim Horton.
+
+        Currently when computing input view bounds upon receiving `UIKeyboardDidChangeFrameNotification`, we attempt to
+        map the on-screen bounds of the keyboard to the window's coordinate space, and save the result in an ivar in
+        `WKWebView`, `_inputViewBounds`. The keyboard frame (which corresponds to `UIKeyboardFrameEndUserInfoKey` in the
+        notification's userInfo dictionary) is given to us in screen coordinates, and we currently pass this through
+        `-convertRect:fromWindow:`, with a nil `UIWindow`.
+
+        However, this results in mapping the rect from the coordinate space of the window's `UIWindowScene` rather than
+        the window screen. In shipping Mail on iOS, this doesn't matter because the window containing the compose web
+        view shares the same coordinate space as the screen. In some other configurations of MobileMail, however, the
+        compose web view appears inside its own `UIWindow`. This causes the above coordinate conversion logic to fail,
+        since we attempt to map a rect given to us in screen coordinates from the compose web view's window scene's
+        coordinate space, instead of the screen's coordinate space.
+
+        We fix this by using `-convertRect:fromCoordinateSpace:` instead, and explicitly pass in
+        `self.window.screen.coordinateSpace`.
+
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        * UIProcess/API/ios/WKWebViewIOS.mm:
+        (-[WKWebView _updateScrollViewForTransaction:]):
+        (-[WKWebView _zoomToFocusRect:selectionRect:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):
+        (-[WKWebView _contentRectForUserInteraction]):
+        (-[WKWebView _updateVisibleContentRects]):
+        (-[WKWebView _keyboardChangedWithInfo:adjustScrollView:]):
+
+        Rename `_inputViewBounds` to `_inputViewBoundsInWindow` for clarity.
+
+        * UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:
+        * UIProcess/API/ios/WKWebViewTestingIOS.mm:
+        (-[WKWebView _inputViewBoundsInWindow]):
+        (-[WKWebView _inputViewBounds]): Deleted.
+
 2021-05-11  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h

@@ -158 +158 @@
     Optional<CGSize> _maximumUnobscuredSizeOverride;
     Optional<WebCore::FloatSize> _lastSentMaximumUnobscuredSize;
-    CGRect _inputViewBounds;
+    CGRect _inputViewBoundsInWindow;
 
     CGFloat _viewportMetaTagWidth;

trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm

@@ -808 +808 @@
     [_scrollView _setZoomEnabledInternal:layerTreeTransaction.allowsUserScaling()];
 
-    bool hasDockedInputView = !CGRectIsEmpty(_inputViewBounds);
+    bool hasDockedInputView = !CGRectIsEmpty(_inputViewBoundsInWindow);
     bool isZoomed = layerTreeTransaction.pageScaleFactor() > layerTreeTransaction.initialScaleFactor();
 
@@ -1243 +1243 @@
     CGRect unobscuredScrollViewRectInWebViewCoordinates = UIEdgeInsetsInsetRect([self bounds], _obscuredInsets);
     CGRect visibleScrollViewBoundsInWebViewCoordinates = CGRectIntersection(unobscuredScrollViewRectInWebViewCoordinates, [fullScreenView convertRect:[fullScreenView bounds] toView:self]);
-    CGRect formAssistantFrameInWebViewCoordinates = [window convertRect:_inputViewBounds toView:self];
+    CGRect formAssistantFrameInWebViewCoordinates = [window convertRect:_inputViewBoundsInWindow toView:self];
     CGRect intersectionBetweenScrollViewAndFormAssistant = CGRectIntersection(visibleScrollViewBoundsInWebViewCoordinates, formAssistantFrameInWebViewCoordinates);
     CGSize visibleSize = visibleScrollViewBoundsInWebViewCoordinates.size;
@@ -1882 +1882 @@
     // FIXME: handle split keyboard.
     UIEdgeInsets obscuredInsets = _obscuredInsets;
-    obscuredInsets.bottom = std::max(_obscuredInsets.bottom, _inputViewBounds.size.height);
+    obscuredInsets.bottom = std::max(_obscuredInsets.bottom, _inputViewBoundsInWindow.size.height);
     CGRect unobscuredRect = UIEdgeInsetsInsetRect(self.bounds, obscuredInsets);
     return [self convertRect:unobscuredRect toView:self._currentContentView];
@@ -2142 +2142 @@
         obscuredInsets:_obscuredInsets
         unobscuredSafeAreaInsets:[self _computedUnobscuredSafeAreaInset]
-        inputViewBounds:_inputViewBounds
+        inputViewBounds:_inputViewBoundsInWindow
         scale:scaleFactor minimumScale:[_scrollView minimumZoomScale]
         viewStability:viewStability
@@ -2299 +2299 @@
         return;
 
-    auto previousInputViewBounds = _inputViewBounds;
+    auto previousInputViewBounds = _inputViewBoundsInWindow;
     BOOL selectionWasVisible = self._selectionRectIsFullyVisibleAndNonEmpty;
 
-    // The keyboard rect is always in screen coordinates. In the view services case the window does not
-    // have the interface orientation rotation transformation; its host does. So, it makes no sense to
-    // clip the keyboard rect against its screen.
-    if ([[self window] _isHostedInAnotherProcess])
-        _inputViewBounds = [self.window convertRect:[endFrameValue CGRectValue] fromWindow:nil];
-    else
-        _inputViewBounds = [self.window convertRect:CGRectIntersection([endFrameValue CGRectValue], self.window.screen.bounds) fromWindow:nil];
-
-    if ([[UIPeripheralHost sharedInstance] isUndocked])
-        _inputViewBounds = CGRectZero;
+    _inputViewBoundsInWindow = ([&] {
+        if (UIPeripheralHost.sharedInstance.isUndocked)
+            return CGRectZero;
+
+        auto keyboardFrameInScreen = endFrameValue.CGRectValue;
+        // The keyboard rect is always in screen coordinates. In the view services case the window does not
+        // have the interface orientation rotation transformation; its host does. So, it makes no sense to
+        // clip the keyboard rect against its screen.
+        if (!self.window._isHostedInAnotherProcess)
+            keyboardFrameInScreen = CGRectIntersection(keyboardFrameInScreen, self.window.screen.bounds);
+
+        return [self.window convertRect:keyboardFrameInScreen fromCoordinateSpace:self.window.screen.coordinateSpace];
+    })();
 
     if (adjustScrollView) {
@@ -2325 +2328 @@
     }
 
-    if (selectionWasVisible && [_contentView _hasFocusedElement] && !CGRectIsEmpty(previousInputViewBounds) && !CGRectIsEmpty(_inputViewBounds) && !CGRectEqualToRect(previousInputViewBounds, _inputViewBounds))
+    if (selectionWasVisible && [_contentView _hasFocusedElement] && !CGRectIsEmpty(previousInputViewBounds) && !CGRectIsEmpty(_inputViewBoundsInWindow) && !CGRectEqualToRect(previousInputViewBounds, _inputViewBoundsInWindow))
         [self _scrollToAndRevealSelectionIfNeeded];
 

trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h

@@ -40 +40 @@
 @property (nonatomic, readonly) NSString *formInputLabel;
 @property (nonatomic, readonly) NSArray<NSValue *> *_uiTextSelectionRects;
-@property (nonatomic, readonly) CGRect _inputViewBounds;
+@property (nonatomic, readonly) CGRect _inputViewBoundsInWindow;
 @property (nonatomic, readonly) NSString *_scrollingTreeAsText;
 @property (nonatomic, readonly) NSNumber *_stableStateOverride;

trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm

@@ -204 +204 @@
 }
 
-- (CGRect)_inputViewBounds
-{
-    return _inputViewBounds;
+- (CGRect)_inputViewBoundsInWindow
+{
+    return _inputViewBoundsInWindow;
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2021-05-11  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Mail compose web view doesn't scroll to reveal the selection in certain configurations
+        https://bugs.webkit.org/show_bug.cgi?id=225675
+        rdar://77095886
+
+        Reviewed by Tim Horton.
+
+        Rename some testing SPI. See WebKit/ChangeLog for more information.
+
+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
+        (WTR::UIScriptControllerIOS::inputViewBounds const):
+
 2021-05-11  Commit Queue  <commit-queue@webkit.org>
 

trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

@@ -850 +850 @@
 JSObjectRef UIScriptControllerIOS::inputViewBounds() const
 {
-    return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:toNSDictionary(webView()._inputViewBounds) inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
+    return JSValueToObject(m_context->jsContext(), [JSValue valueWithObject:toNSDictionary(webView()._inputViewBoundsInWindow) inContext:[JSContext contextWithJSGlobalContextRef:m_context->jsContext()]].JSValueRef, nullptr);
 }