Changeset 233905 in webkit

Timestamp:

Jul 17, 2018 8:50:12 PM (6 years ago)

Author:

timothy_horton@apple.com

Message:

REGRESSION (iOS 12): Can't scroll to the bottom of the page in WKWebView while keyboard is up on pages with viewport-fit=cover
​https://bugs.webkit.org/show_bug.cgi?id=187743
<rdar://problem/41651546>

Reviewed by Simon Fraser.

UIScrollView's _systemContentInset no longer includes keyboard insets
in apps linked on iOS 12+ when contentInsetAdjustmentBehavior is None.

We use contentInsetAdjustmentBehavior to control adjustment of other
sources of insets, but expect the keyboard inset to always be applied.

For now, barring a more comprehensive way to separate insets, re-add
the keyboard inset in cases where UIKit does not.

• Platform/spi/ios/UIKitSPI.h:

Move some IPI to the SPI header.

• UIProcess/API/Cocoa/WKWebView.mm:

(-[WKWebView _haveSetObscuredInsets]):

• UIProcess/API/Cocoa/WKWebViewInternal.h:

Make it possible for WKScrollView to check whether the client is overriding insets.

• UIProcess/Cocoa/VersionChecks.h:

Add a linkedOnOrAfter() version for this new UIScrollView behavior.

• UIProcess/ios/WKScrollView.mm:

(-[WKScrollView initWithFrame:]):
Force WKScrollView's scroll indicator to always respect insets. We always
want the scroll bars in a sensible place, even if the page sets
viewport-fit=cover.

(-[WKScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:]):
Store the bottom inset due to the keyboard.

(-[WKScrollView _systemContentInset]):
Add the bottom inset due to the keyboard to the systemContentInset
in all cases where UIKit does not. Also avoid adding it if the client takes
full control of the insets, because the only client that does (MobileSafari)
includes insets-due-to-keyboard in their custom insets.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/spi/ios/UIKitSPI.h (modified) (3 diffs)
• UIProcess/API/Cocoa/WKWebView.mm (modified) (2 diffs)
• UIProcess/API/Cocoa/WKWebViewInternal.h (modified) (1 diff)
• UIProcess/Cocoa/VersionChecks.h (modified) (1 diff)
• UIProcess/ios/WKScrollView.mm (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-07-17  Tim Horton  <timothy_horton@apple.com>
+
+        REGRESSION (iOS 12): Can't scroll to the bottom of the page in WKWebView while keyboard is up on pages with viewport-fit=cover
+        https://bugs.webkit.org/show_bug.cgi?id=187743
+        <rdar://problem/41651546>
+
+        Reviewed by Simon Fraser.
+
+        UIScrollView's _systemContentInset no longer includes keyboard insets
+        in apps linked on iOS 12+ when contentInsetAdjustmentBehavior is None.
+
+        We use contentInsetAdjustmentBehavior to control adjustment of other
+        sources of insets, but expect the keyboard inset to always be applied.
+
+        For now, barring a more comprehensive way to separate insets, re-add
+        the keyboard inset in cases where UIKit does not.
+
+        * Platform/spi/ios/UIKitSPI.h:
+        Move some IPI to the SPI header.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _haveSetObscuredInsets]):
+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
+        Make it possible for WKScrollView to check whether the client is overriding insets.
+
+        * UIProcess/Cocoa/VersionChecks.h:
+        Add a linkedOnOrAfter() version for this new UIScrollView behavior.
+
+        * UIProcess/ios/WKScrollView.mm:
+        (-[WKScrollView initWithFrame:]):
+        Force WKScrollView's scroll indicator to always respect insets. We always
+        want the scroll bars in a sensible place, even if the page sets
+        viewport-fit=cover.
+
+        (-[WKScrollView _adjustForAutomaticKeyboardInfo:animated:lastAdjustment:]):
+        Store the bottom inset due to the keyboard.
+
+        (-[WKScrollView _systemContentInset]):
+        Add the bottom inset due to the keyboard to the systemContentInset 
+        in all cases where UIKit does not. Also avoid adding it if the client takes
+        full control of the insets, because the only client that does (MobileSafari)
+        includes insets-due-to-keyboard in their custom insets.
+
 2018-07-17  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebKit/Platform/spi/ios/UIKitSPI.h

@@ -316 +316 @@
 @end
 
+typedef NS_ENUM(NSInteger, UIScrollViewIndicatorInsetAdjustmentBehavior) {
+    UIScrollViewIndicatorInsetAdjustmentAutomatic,
+    UIScrollViewIndicatorInsetAdjustmentAlways,
+    UIScrollViewIndicatorInsetAdjustmentNever
+};
+
 @interface UIScrollView ()
 - (void)_stopScrollingAndZoomingAnimations;
@@ -327 +333 @@
 @property (nonatomic, readonly) BOOL _isInterruptingDeceleration;
 @property (nonatomic, getter=_contentScrollInset, setter=_setContentScrollInset:) UIEdgeInsets contentScrollInset;
+@property (nonatomic, getter=_indicatorInsetAdjustmentBehavior, setter=_setIndicatorInsetAdjustmentBehavior:) UIScrollViewIndicatorInsetAdjustmentBehavior indicatorInsetAdjustmentBehavior;
 #if __IPHONE_OS_VERSION_MAX_ALLOWED >= 110000
 @property (nonatomic, readonly) UIEdgeInsets _systemContentInset;
@@ -977 +984 @@
 @end
 
+@interface UIScrollView (IPI)
+- (CGFloat)_rubberBandOffsetForOffset:(CGFloat)newOffset maxOffset:(CGFloat)maxOffset minOffset:(CGFloat)minOffset range:(CGFloat)range outside:(BOOL *)outside;
+- (void)_adjustForAutomaticKeyboardInfo:(NSDictionary *)info animated:(BOOL)animated lastAdjustment:(CGFloat*)lastAdjustment;
+- (BOOL)_isScrollingToTop;
+- (CGPoint)_animatedTargetOffset;
+@end
+
 @interface UIPeripheralHost (IPI)
 - (void)_beginIgnoringReloadInputViews;
 - (int)_endIgnoringReloadInputViews;
 - (void)forceReloadInputViews;
+- (CGFloat)getVerticalOverlapForView:(UIView *)view usingKeyboardInfo:(NSDictionary *)info;
 @end
 

trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

@@ -155 +155 @@
 #define RELEASE_LOG_IF_ALLOWED(...) RELEASE_LOG_IF(_page && _page->isAlwaysOnLoggingAllowed(), ViewState, __VA_ARGS__)
 
-@interface UIScrollView (UIScrollViewInternal)
-- (void)_adjustForAutomaticKeyboardInfo:(NSDictionary*)info animated:(BOOL)animated lastAdjustment:(CGFloat*)lastAdjustment;
-- (BOOL)_isScrollingToTop;
-- (CGPoint)_animatedTargetOffset;
-@end
-
-@interface UIPeripheralHost(UIKitInternal)
-- (CGFloat)getVerticalOverlapForView:(UIView *)view usingKeyboardInfo:(NSDictionary *)info;
-@end
-
 @interface UIView (UIViewInternal)
 - (UIViewController *)_viewControllerForAncestor;
@@ -3175 +3165 @@
     if ([uiDelegate respondsToSelector:@selector(_webView:didChangeSafeAreaShouldAffectObscuredInsets:)])
         [uiDelegate _webView:self didChangeSafeAreaShouldAffectObscuredInsets:avoidsUnsafeArea];
+}
+
+- (BOOL)_haveSetObscuredInsets
+{
+    return _haveSetObscuredInsets;
 }
 

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

@@ -148 +148 @@
 
 @property (nonatomic, readonly) BOOL _allowsDoubleTapGestures;
+@property (nonatomic, readonly) BOOL _haveSetObscuredInsets;
 @property (nonatomic, readonly) UIEdgeInsets _computedObscuredInset;
 @property (nonatomic, readonly) UIEdgeInsets _computedUnobscuredSafeAreaInset;

trunk/Source/WebKit/UIProcess/Cocoa/VersionChecks.h

@@ -40 +40 @@
     FirstThatDefaultsToPassiveTouchListenersOnDocument = DYLD_IOS_VERSION_11_3,
     FirstWhereScrollViewContentInsetsAreNotObscuringInsets = DYLD_IOS_VERSION_12_0,
+    FirstWhereUIScrollViewDoesNotApplyKeyboardInsetsUnconditionally = DYLD_IOS_VERSION_12_0,
 #elif PLATFORM(MAC)
     FirstWithNetworkCache = DYLD_MACOSX_VERSION_10_11,

trunk/Source/WebKit/UIProcess/ios/WKScrollView.mm

@@ -29 +29 @@
 #if PLATFORM(IOS)
 
+#import "UIKitSPI.h"
+#import "VersionChecks.h"
 #import "WKWebViewInternal.h"
 #import <pal/spi/cg/CoreGraphicsSPI.h>
@@ -38 +40 @@
 
 using namespace WebKit;
-
-@interface UIScrollView (UIScrollViewInternalHack)
-- (CGFloat)_rubberBandOffsetForOffset:(CGFloat)newOffset maxOffset:(CGFloat)maxOffset minOffset:(CGFloat)minOffset range:(CGFloat)range outside:(BOOL *)outside;
-@end
 
 @interface WKScrollViewDelegateForwarder : NSObject <UIScrollViewDelegate>
@@ -134 +132 @@
     BOOL _contentInsetAdjustmentBehaviorWasExternallyOverridden;
 #endif
+    CGFloat _keyboardBottomInsetAdjustment;
 }
 
@@ -145 +144 @@
     self.alwaysBounceVertical = YES;
     self.directionalLockEnabled = YES;
+    [self _setIndicatorInsetAdjustmentBehavior:UIScrollViewIndicatorInsetAdjustmentAlways];
 
 #if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
@@ -331 +331 @@
 }
 
+- (void)_adjustForAutomaticKeyboardInfo:(NSDictionary *)info animated:(BOOL)animated lastAdjustment:(CGFloat*)lastAdjustment
+{
+    [super _adjustForAutomaticKeyboardInfo:info animated:animated lastAdjustment:lastAdjustment];
+
+    _keyboardBottomInsetAdjustment = [[UIPeripheralHost sharedInstance] getVerticalOverlapForView:self usingKeyboardInfo:info];
+}
+
+- (UIEdgeInsets)_systemContentInset
+{
+    UIEdgeInsets systemContentInset = [super _systemContentInset];
+
+    // Internal clients who use setObscuredInsets include the keyboard height in their
+    // manually overriden insets, so we don't need to re-add it here.
+    if (_internalDelegate._haveSetObscuredInsets)
+        return systemContentInset;
+
+    // Match the inverse of the condition that UIScrollView uses to decide whether
+    // to include keyboard insets in the systemContentInset. We always want
+    // keyboard insets applied, even when web content has chosen to disable automatic
+    // safe area inset adjustment.
+    if (linkedOnOrAfter(SDKVersion::FirstWhereUIScrollViewDoesNotApplyKeyboardInsetsUnconditionally) && self.contentInsetAdjustmentBehavior == UIScrollViewContentInsetAdjustmentNever)
+        systemContentInset.bottom += _keyboardBottomInsetAdjustment;
+
+    return systemContentInset;
+}
+
 #if PLATFORM(WATCHOS)