Attachment 373290 Details for Bug 199353 – Patch

[patch] Patch

bug-199353-20190701174057.patch (text/plain), 12.42 KB, created by Andy Estes on 2019-07-01 17:40:58 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2019-07-01 17:40:58 PDT

Size: 12.42 KB

patch

obsolete

>Subversion Revision: 247018
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index fa3ba850519823c13a7cdaa9766d52c083cf2bde..0f5b6a7501c6fce6787377734ed7ab15e99af431 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,32 @@
>+2019-07-01  Andy Estes  <aestes@apple.com>
>+
>+        [iOS] Do not disable scrolling when WKScrollView subviews intersect the content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=199353
>+        <rdar://problem/50822146>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        As of r245006, we disable scrolling in WKScrollView when the <body> element is styled with
>+        'overflow: hidden'. The overflowed content is still present in the WKContentView but is
>+        clipped by the scroll view.
>+
>+        WebKit exposes its scroll view as API (-scrollView), and some clients position their own
>+        subviews in the scroll view's content inset areas. When scrolling is disabled, these
>+        subviews can become unreachable.
>+
>+        This patch implements a heuristic that enables scrolling when subviews intersect the scroll
>+        view's left, right, or bottom content inset areas. The top inset area is excluded from this
>+        heuristic because they are visible in the common case where scroll offsets are at 0 and
>+        merely shrink the height of the visible content view.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _didCommitLayerTree:]):
>+        * UIProcess/ios/WKScrollView.h:
>+        * UIProcess/ios/WKScrollView.mm:
>+        (-[WKScrollView _currentTopLeftRubberbandAmount]):
>+        (-[WKScrollView _hasContentInRelevantInsetAreas]):
>+        (-[WKScrollView layoutSubviews]):
>+
> 2019-07-01  Eric Carlson  <eric.carlson@apple.com>
> 
>         [iOS] Exiting from fullscreen scrolls to top of page
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 921fb01a0e3486eab5b3b152d28d7a8adbbb3b56..c34b18787ea96443e84e289654bd1b3ce7873e6e 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -2026,7 +2026,7 @@ - (void)_didCommitLayerTree:(const WebKit::RemoteLayerTreeTransaction&)layerTree
> #if ENABLE(ASYNC_SCROLLING)
>     bool hasDockedInputView = !CGRectIsEmpty(_inputViewBounds);
>     bool isZoomed = layerTreeTransaction.pageScaleFactor() > layerTreeTransaction.initialScaleFactor();
>-    [_scrollView _setScrollEnabledInternal:_page->scrollingCoordinatorProxy()->hasScrollableMainFrame() || hasDockedInputView || isZoomed];
>+    [_scrollView _setScrollEnabledInternal:_page->scrollingCoordinatorProxy()->hasScrollableMainFrame() || hasDockedInputView || isZoomed || [_scrollView _hasContentInRelevantInsetAreas]];
> #endif
>     if (!layerTreeTransaction.scaleWasSetByUIProcess() && ![_scrollView isZooming] && ![_scrollView isZoomBouncing] && ![_scrollView _isAnimatingZoom] && [_scrollView zoomScale] != layerTreeTransaction.pageScaleFactor()) {
>         LOG_WITH_STREAM(VisibleRects, stream << " updating scroll view with pageScaleFactor " << layerTreeTransaction.pageScaleFactor());
>diff --git a/Source/WebKit/UIProcess/ios/WKScrollView.h b/Source/WebKit/UIProcess/ios/WKScrollView.h
>index 409086404f55b3d63a5fd5739bbdab9d5988bf90..33623d3b89e78fc194b610a6f0c4fc70d7948620 100644
>--- a/Source/WebKit/UIProcess/ios/WKScrollView.h
>+++ b/Source/WebKit/UIProcess/ios/WKScrollView.h
>@@ -42,6 +42,8 @@
> - (void)_setContentInsetAdjustmentBehaviorInternal:(UIScrollViewContentInsetAdjustmentBehavior)insetAdjustmentBehavior;
> #endif
> 
>+@property (nonatomic, readonly) BOOL _hasContentInRelevantInsetAreas;
>+
> @end
> 
> #endif // PLATFORM(IOS_FAMILY)
>diff --git a/Source/WebKit/UIProcess/ios/WKScrollView.mm b/Source/WebKit/UIProcess/ios/WKScrollView.mm
>index 67c63b5fdcfea30f492311daafb7e132dae803fd..39600e6a12ecca5df935d14100f42957096636f2 100644
>--- a/Source/WebKit/UIProcess/ios/WKScrollView.mm
>+++ b/Source/WebKit/UIProcess/ios/WKScrollView.mm
>@@ -136,6 +136,8 @@ @implementation WKScrollView {
>     BOOL _zoomEnabledInternal;
> }
> 
>+@synthesize _hasContentInRelevantInsetAreas=_hasContentInRelevantInsetAreas;
>+
> - (id)initWithFrame:(CGRect)frame
> {
>     self = [super initWithFrame:frame];
>@@ -302,7 +304,7 @@ - (CGSize)_currentTopLeftRubberbandAmount
> 
>     if (contentOffset.y < -edgeInsets.top)
>         rubberbandAmount.height = std::min<CGFloat>(contentOffset.y + edgeInsets.top, 0);
>-    
>+
>     return rubberbandAmount;
> }
> 
>@@ -397,6 +399,26 @@ - (void)_updateZoomability
>     [super setZoomEnabled:(_zoomEnabledByClient && _zoomEnabledInternal)];
> }
> 
>+- (void)layoutSubviews
>+{
>+    [super layoutSubviews];
>+
>+    UIEdgeInsets contentInsets = self.contentInset;
>+    UIEdgeInsets contentRectOutsets = { 0, -contentInsets.left, -contentInsets.bottom, -contentInsets.right };
>+    CGRect contentRect = { CGPointMake(0, 0), self.contentSize };
>+    CGRect contentRectWithRelevantInsetAreas = UIEdgeInsetsInsetRect(contentRect, contentRectOutsets);
>+
>+    _hasContentInRelevantInsetAreas = NO;
>+    if (CGRectIsEmpty(contentRectWithRelevantInsetAreas))
>+        return;
>+
>+    CGRect boundingRect = CGRectNull;
>+    for (UIView *view in self.subviews)
>+        boundingRect = CGRectUnion(boundingRect, CGRectIntersection(contentRectWithRelevantInsetAreas, view.frame));
>+
>+    _hasContentInRelevantInsetAreas = !CGRectIsEmpty(boundingRect) && !CGRectContainsRect(contentRect, boundingRect);
>+}
>+
> #if PLATFORM(WATCHOS)
> 
> - (void)addGestureRecognizer:(UIGestureRecognizer *)gestureRecognizer
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index bdf9bb358040f6727b85d23e80f6e062f654fef5..11873beca8afa264b0f4a392619f407a9544f624 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-07-01  Andy Estes  <aestes@apple.com>
>+
>+        [iOS] Do not disable scrolling when WKScrollView subviews intersect the content inset areas
>+        https://bugs.webkit.org/show_bug.cgi?id=199353
>+        <rdar://problem/50822146>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm:
>+        (-[ContentSizeObserver initWithScrollView:changeHandler:]):
>+        (-[ContentSizeObserver observeValueForKeyPath:ofObject:change:context:]):
>+        (-[ContentSizeObserver dealloc]):
>+        (TestWebKitAPI::TEST):
>+
> 2019-07-01  Philippe Normand  <pnormand@igalia.com>
> 
>         Unreviewed, GTK a11y tests fix after r246958
>diff --git a/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm b/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>index 8e9f41bc0c3d59949afe44f3e3577bc199b94f5a..e1de1670f9108fd7c64d3d6760d2eed1ecbb028a 100644
>--- a/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>+++ b/Tools/TestWebKitAPI/Tests/ios/ScrollViewInsetTests.mm
>@@ -71,6 +71,44 @@ - (void)webViewWebContentProcessDidTerminate:(WKWebView *)webView
> }
> @end
> 
>+static NSString * const contentSizeKeyPath = @"contentSize";
>+
>+@interface ContentSizeObserver : NSObject
>+- (instancetype)init NS_UNAVAILABLE;
>+- (instancetype)initWithScrollView:(UIScrollView *)scrollView changeHandler:(void(^)())changeHandler;
>+@end
>+
>+@implementation ContentSizeObserver {
>+    void (^_changeHandler)();
>+    UIScrollView *_scrollView;
>+}
>+
>+- (instancetype)initWithScrollView:(UIScrollView *)scrollView changeHandler:(void(^)())changeHandler
>+{
>+    if (!(self = [super init]))
>+        return nil;
>+
>+    _changeHandler = [changeHandler copy];
>+    _scrollView = [scrollView retain];
>+    [_scrollView addObserver:self forKeyPath:contentSizeKeyPath options:NSKeyValueObservingOptionInitial context:nullptr];
>+    return self;
>+}
>+
>+- (void)observeValueForKeyPath:(NSString *)keyPath ofObject:(id)object change:(NSDictionary<NSKeyValueChangeKey, id> *)change context:(void*)context
>+{
>+    _changeHandler();
>+}
>+
>+- (void)dealloc
>+{
>+    [_scrollView removeObserver:self forKeyPath:contentSizeKeyPath context:nullptr];
>+    [_changeHandler release];
>+    [_scrollView release];
>+    [super dealloc];
>+}
>+
>+@end
>+
> namespace TestWebKitAPI {
> 
> static const CGFloat viewHeight = 500;
>@@ -206,6 +244,88 @@ TEST(ScrollViewInsetTests, RestoreInitialContentOffsetAfterCrashWithAsyncPolicyD
>     EXPECT_EQ(-400, initialContentOffset.y);
> }
> 
>+static NSString * const overflowHiddenDocumentMarkup = @"<!DOCTYPE html><meta name='viewport' content='width=device-width, initial-scale=1'><body style='width: 100%; height: 5000px; overflow: hidden;'>";
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithContentInset)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewOverlapping)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, 0, 320, 100)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewOutsideScrollableArea)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, -101, 320, 50)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewAbove)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, -50, 320, 50)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(NO, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewBelow)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(0, 0, 320, 50)]);
>+    auto observer = adoptNS([[ContentSizeObserver alloc] initWithScrollView:[webView scrollView] changeHandler:^{
>+        CGRect frame = [otherView frame];
>+        frame.origin.y = [webView scrollView].contentSize.height - 25;
>+        [otherView setFrame:frame];
>+    }]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(50, 0, 50, 0);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewBefore)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(-50, 0, 50, viewHeight)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(0, 50, 0, 50);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
>+TEST(ScrollViewInsetTests, OverflowHiddenBodyWithScrollViewSubviewAfter)
>+{
>+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 320, viewHeight)]);
>+    auto otherView = adoptNS([[UIView alloc] initWithFrame:CGRectMake(295, 0, 50, viewHeight)]);
>+    [[webView scrollView] addSubview:otherView.get()];
>+    [webView scrollView].contentInset = UIEdgeInsetsMake(0, 50, 0, 50);
>+    [webView synchronouslyLoadHTMLString:overflowHiddenDocumentMarkup];
>+    [webView waitForNextPresentationUpdate];
>+    EXPECT_EQ(YES, [webView scrollView].scrollEnabled);
>+}
>+
> } // namespace TestWebKitAPI
> 
> #endif // PLATFORM(IOS_FAMILY)

Actions: View | Formatted Diff | Diff

Attachments on bug 199353: 373216 | 373290