Skip to content

Commit

Permalink
Entering a To: address moves the viewpoint to the bottom right corner…
Browse files Browse the repository at this point in the history
… of the email

https://bugs.webkit.org/show_bug.cgi?id=245297
rdar://96703879

Reviewed by Tim Horton.

On iPad in the Mail app, when composing a message whose contents are taller
than the viewport size, and when the compose window is narrow enough such that
the contact picker appears as a modal when entering a contact in the `To:`
field, dismissing the contact picker causes the message to erroneously scroll
to the bottom right of the message.

This is because in this case, there is a large top inset on the web view's
scroll view. This causes the UIProcess to erroneously create a `CGRectNull`
and sends it to the web process in a visible content rect update. Because a
`CGRectNull` has an origin of `(INT_MAX, INT_MAX)`, the web view attempts to
scroll to that position.

This PR fixes this by using `FloatRect`s intersection behavior instead of using
`CGRectIntersection`, so that the resulting rect's origin stays the same instead
of becoming `CGRectNull` in the case the two rects don't intersect.

A change also had to be made to
`-[WKWebView _scrollToContentScrollPosition:scrollOrigin:animated:]` so that the
view scrolls to its original position with the inset, instead of `0`.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _scrollToContentScrollPosition:scrollOrigin:animated:]):
(-[WKWebView _updateVisibleContentRects]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/RestoreScrollPosition.mm:
(-[RestoreScrollPositionWithLargeContentInsetWebView safeAreaInsets]):
(TestWebKitAPI::TEST):

Canonical link: https://commits.webkit.org/254640@main
  • Loading branch information
rr-codes authored and hortont424 committed Sep 19, 2022
1 parent cc81559 commit c5a7221
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
16 changes: 12 additions & 4 deletions Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm
Expand Up @@ -1287,9 +1287,17 @@ - (void)_scrollToContentScrollPosition:(WebCore::FloatPoint)scrollPosition scrol

[_scrollView _stopScrollingAndZoomingAnimations];

if (!CGPointEqualToPoint(contentOffsetInScrollViewCoordinates, [_scrollView contentOffset]))
CGPoint scrollViewContentOffset = [_scrollView contentOffset];

if (!CGPointEqualToPoint(contentOffsetInScrollViewCoordinates, scrollViewContentOffset)) {
if (WTF::areEssentiallyEqual<float>(scrollPosition.x(), 0) && scrollViewContentOffset.x < 0)
contentOffsetInScrollViewCoordinates.x = scrollViewContentOffset.x;

if (WTF::areEssentiallyEqual<float>(scrollPosition.y(), 0) && scrollViewContentOffset.y < 0)
contentOffsetInScrollViewCoordinates.y = scrollViewContentOffset.y;

[_scrollView setContentOffset:contentOffsetInScrollViewCoordinates animated:animated];
else {
} else {
// If we haven't changed anything, there would not be any VisibleContentRect update sent to the content.
// The WebProcess would keep the invalid contentOffset as its scroll position.
// To synchronize the WebProcess with what is on screen, we send the VisibleContentRect again.
Expand Down Expand Up @@ -2480,8 +2488,8 @@ - (void)_updateVisibleContentRects

CGFloat scaleFactor = contentZoomScale(self);
CGRect unobscuredRect = UIEdgeInsetsInsetRect(self.bounds, computedContentInsetUnadjustedForKeyboard);
CGRect unobscuredRectInContentCoordinates = _perProcessState.frozenUnobscuredContentRect ? _perProcessState.frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()];
unobscuredRectInContentCoordinates = CGRectIntersection(unobscuredRectInContentCoordinates, [self _contentBoundsExtendedForRubberbandingWithScale:scaleFactor]);
WebCore::FloatRect unobscuredRectInContentCoordinates = WebCore::FloatRect(_perProcessState.frozenUnobscuredContentRect ? _perProcessState.frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()]);
unobscuredRectInContentCoordinates.intersect([self _contentBoundsExtendedForRubberbandingWithScale:scaleFactor]);

auto contentInsets = [self currentlyVisibleContentInsetsWithScale:scaleFactor obscuredInsets:computedContentInsetUnadjustedForKeyboard];

Expand Down
36 changes: 36 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/RestoreScrollPosition.mm
Expand Up @@ -32,10 +32,46 @@
#import <WebKit/WKWebViewPrivate.h>
#import <WebKit/_WKProcessPoolConfiguration.h>

#if PLATFORM(IOS_FAMILY)
@interface RestoreScrollPositionWithLargeContentInsetWebView : TestWKWebView
@end

@implementation RestoreScrollPositionWithLargeContentInsetWebView
- (UIEdgeInsets)safeAreaInsets
{
return UIEdgeInsetsMake(141, 0, 0, 0);
}
@end
#endif

namespace TestWebKitAPI {

#if PLATFORM(IOS_FAMILY)

TEST(RestoreScrollPositionTests, RestoreScrollPositionWithLargeContentInset)
{
auto topInset = 1165;

auto webView = adoptNS([[RestoreScrollPositionWithLargeContentInsetWebView alloc] initWithFrame:CGRectMake(0, 0, 375, 1024)]);

[webView synchronouslyLoadTestPageNamed:@"simple-tall"];

auto insets = UIEdgeInsetsMake(topInset, 0, 0, 0);
[webView scrollView].contentInset = insets;
[webView _setObscuredInsets:UIEdgeInsetsMake(141, 0, 0, 0)];
[webView scrollView].contentOffset = CGPointMake(0, -topInset);

[webView waitForNextPresentationUpdate];
[webView _overrideLayoutParametersWithMinimumLayoutSize:CGSizeMake(375, 727) maximumUnobscuredSizeOverride:CGSizeMake(375, 727)];

[webView scrollView].contentInset = UIEdgeInsetsMake(1024, 0, 0, 0);
[webView waitForNextPresentationUpdate];

CGPoint contentOffsetAfterScrolling = [webView scrollView].contentOffset;

EXPECT_EQ(-topInset, contentOffsetAfterScrolling.y);
}

TEST(RestoreScrollPositionTests, RestoreScrollPositionDuringResize)
{
auto processPoolConfiguration = adoptNS([[_WKProcessPoolConfiguration alloc] init]);
Expand Down

0 comments on commit c5a7221

Please sign in to comment.