Title: [277352] trunk
Revision
277352
Author
[email protected]
Date
2021-05-11 20:46:21 -0700 (Tue, 11 May 2021)

Log 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):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (277351 => 277352)


--- trunk/Source/WebKit/ChangeLog	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Source/WebKit/ChangeLog	2021-05-12 03:46:21 UTC (rev 277352)
@@ -1,3 +1,42 @@
+2021-05-11  Wenson Hsieh  <[email protected]>
+
+        [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  <[email protected]>
 
         Crash in DisplayLink::incrementFullSpeedRequestClientCount()

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h (277351 => 277352)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h	2021-05-12 03:46:21 UTC (rev 277352)
@@ -157,7 +157,7 @@
     Optional<WebCore::FloatSize> _lastSentViewLayoutSize;
     Optional<CGSize> _maximumUnobscuredSizeOverride;
     Optional<WebCore::FloatSize> _lastSentMaximumUnobscuredSize;
-    CGRect _inputViewBounds;
+    CGRect _inputViewBoundsInWindow;
 
     CGFloat _viewportMetaTagWidth;
     BOOL _viewportMetaTagWidthWasExplicit;

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm (277351 => 277352)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm	2021-05-12 03:46:21 UTC (rev 277352)
@@ -807,7 +807,7 @@
     [_scrollView setMaximumZoomScale:layerTreeTransaction.maximumScaleFactor()];
     [_scrollView _setZoomEnabledInternal:layerTreeTransaction.allowsUserScaling()];
 
-    bool hasDockedInputView = !CGRectIsEmpty(_inputViewBounds);
+    bool hasDockedInputView = !CGRectIsEmpty(_inputViewBoundsInWindow);
     bool isZoomed = layerTreeTransaction.pageScaleFactor() > layerTreeTransaction.initialScaleFactor();
 
     bool scrollingNeededToRevealUI = false;
@@ -1242,7 +1242,7 @@
 
     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;
 
@@ -1881,7 +1881,7 @@
 {
     // 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];
 }
@@ -2141,7 +2141,7 @@
         unobscuredRectInScrollViewCoordinates:unobscuredRect
         obscuredInsets:_obscuredInsets
         unobscuredSafeAreaInsets:[self _computedUnobscuredSafeAreaInset]
-        inputViewBounds:_inputViewBounds
+        inputViewBounds:_inputViewBoundsInWindow
         scale:scaleFactor minimumScale:[_scrollView minimumZoomScale]
         viewStability:viewStability
         enclosedInScrollableAncestorView:scrollViewCanScroll([self _scroller])
@@ -2298,20 +2298,23 @@
     if (!endFrameValue)
         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];
+    _inputViewBoundsInWindow = ([&] {
+        if (UIPeripheralHost.sharedInstance.isUndocked)
+            return CGRectZero;
 
-    if ([[UIPeripheralHost sharedInstance] isUndocked])
-        _inputViewBounds = 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) {
         CGFloat bottomInsetBeforeAdjustment = [_scrollView contentInset].bottom;
         SetForScope<BOOL> insetAdjustmentGuard(_currentlyAdjustingScrollViewInsetsForKeyboard, YES);
@@ -2324,7 +2327,7 @@
             _totalScrollViewBottomInsetAdjustmentForKeyboard += bottomInsetAfterAdjustment - bottomInsetBeforeAdjustment;
     }
 
-    if (selectionWasVisible && [_contentView _hasFocusedElement] && !CGRectIsEmpty(previousInputViewBounds) && !CGRectIsEmpty(_inputViewBounds) && !CGRectEqualToRect(previousInputViewBounds, _inputViewBounds))
+    if (selectionWasVisible && [_contentView _hasFocusedElement] && !CGRectIsEmpty(previousInputViewBounds) && !CGRectIsEmpty(_inputViewBoundsInWindow) && !CGRectEqualToRect(previousInputViewBounds, _inputViewBoundsInWindow))
         [self _scrollToAndRevealSelectionIfNeeded];
 
     [self _scheduleVisibleContentRectUpdate];

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h (277351 => 277352)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h	2021-05-12 03:46:21 UTC (rev 277352)
@@ -39,7 +39,7 @@
 @property (nonatomic, readonly) NSString *selectFormPopoverTitle;
 @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;
 @property (nonatomic, readonly) CGRect _dragCaretRect;

Modified: trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm (277351 => 277352)


--- trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm	2021-05-12 03:46:21 UTC (rev 277352)
@@ -203,9 +203,9 @@
     return [_contentView formInputLabel];
 }
 
-- (CGRect)_inputViewBounds
+- (CGRect)_inputViewBoundsInWindow
 {
-    return _inputViewBounds;
+    return _inputViewBoundsInWindow;
 }
 
 - (NSArray<NSValue *> *)_uiTextSelectionRects

Modified: trunk/Tools/ChangeLog (277351 => 277352)


--- trunk/Tools/ChangeLog	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Tools/ChangeLog	2021-05-12 03:46:21 UTC (rev 277352)
@@ -1,3 +1,16 @@
+2021-05-11  Wenson Hsieh  <[email protected]>
+
+        [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  <[email protected]>
 
         Unreviewed, reverting r277319.

Modified: trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm (277351 => 277352)


--- trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2021-05-12 03:40:45 UTC (rev 277351)
+++ trunk/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm	2021-05-12 03:46:21 UTC (rev 277352)
@@ -849,7 +849,7 @@
 
 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);
 }
 
 JSRetainPtr<JSStringRef> UIScriptControllerIOS::scrollingTreeAsText() const
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to