Skip to content

Commit

Permalink
[visionOS] Find in HTML note obscures matches when they are at the top
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=260982
rdar://109165939

Reviewed by Wenson Hsieh.

On visionOS, the find bar initially appears in the scroll view's inset area.
As the scroll view is scrolled, the find bar's frame is adjusted to ensure
it is always at the top of the currently visible rect. Both these behaviors are
implemented by UIKit.

When matches are found, scrolling is performed to make them visible. This is
achieved by using `-[WKWebView _scrollToRect:origin:minimumScrollDistance:]`.
However, that method always enforces a minimum content offset of (0, 0). This
is incompatible with the find bar's position, since for matches at the top of
the scroll view, a scroll to (0, 0) is forced, even when match is already visible.
The forced scroll to (0, 0) then ends up obscuring the match, as the find bar's
position is adjusted to keep it at the top.

Fix by accounting for content insets when determining the minimum content offset
for targeted scrolling.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _initialContentOffsetForScrollView]):
(constrainContentOffset):
(-[WKWebView _scrollToRect:origin:minimumScrollDistance:]):
* Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm:
(TEST):

Use a 500ms delay to ensure that scrolling did not occur. Waiting for two
presentation updates is unfortunately insufficient.

Canonical link: https://commits.webkit.org/267539@main
  • Loading branch information
pxlcoder committed Sep 1, 2023
1 parent 86bdc44 commit 2e3f11b
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 4 deletions.
12 changes: 8 additions & 4 deletions Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,7 @@ - (void)_videoControlsManagerDidChange

- (CGPoint)_initialContentOffsetForScrollView
{
// FIXME: Should this use -[_scrollView adjustedContentInset]?
auto combinedUnobscuredAndScrollViewInset = [self _computedContentInset];
return CGPointMake(-combinedUnobscuredAndScrollViewInset.left, -combinedUnobscuredAndScrollViewInset.top);
}
Expand Down Expand Up @@ -1327,10 +1328,10 @@ - (void)_zoomToRect:(WebCore::FloatRect)targetRect atScale:(double)scale origin:
[self _zoomToCenter:visibleRectAfterZoom.center() atScale:scale animated:animated honorScrollability:YES];
}

static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOffset, WebCore::FloatSize contentSize, WebCore::FloatSize unobscuredContentSize)
static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOffset, WebCore::FloatPoint minimumContentOffset, WebCore::FloatSize contentSize, WebCore::FloatSize unobscuredContentSize)
{
WebCore::FloatSize maximumContentOffset = contentSize - unobscuredContentSize;
return contentOffset.constrainedBetween(WebCore::FloatPoint(), WebCore::FloatPoint(maximumContentOffset));
return contentOffset.constrainedBetween(minimumContentOffset, WebCore::FloatPoint(maximumContentOffset));
}

- (void)_scrollToContentScrollPosition:(WebCore::FloatPoint)scrollPosition scrollOrigin:(WebCore::IntPoint)scrollOrigin animated:(BOOL)animated
Expand Down Expand Up @@ -1415,6 +1416,9 @@ - (BOOL)_scrollToRect:(WebCore::FloatRect)targetRect origin:(WebCore::FloatPoint
WebCore::FloatPoint unobscuredContentOffset = unobscuredContentRect.location();
WebCore::FloatSize contentSize([self._currentContentView bounds].size);

auto scrollViewInsets = [_scrollView adjustedContentInset];
WebCore::FloatPoint minimumContentOffset(-scrollViewInsets.left, -scrollViewInsets.top);

// Center the target rect in the scroll view.
// If the target doesn't fit in the scroll view, center on the gesture location instead.
WebCore::FloatPoint newUnobscuredContentOffset;
Expand All @@ -1426,14 +1430,14 @@ - (BOOL)_scrollToRect:(WebCore::FloatRect)targetRect origin:(WebCore::FloatPoint
newUnobscuredContentOffset.setY(targetRect.y() - (unobscuredContentRect.height() - targetRect.height()) / 2);
else
newUnobscuredContentOffset.setY(origin.y() - unobscuredContentRect.height() / 2);
newUnobscuredContentOffset = constrainContentOffset(newUnobscuredContentOffset, contentSize, unobscuredContentRect.size());
newUnobscuredContentOffset = constrainContentOffset(newUnobscuredContentOffset, minimumContentOffset, contentSize, unobscuredContentRect.size());

if (unobscuredContentOffset == newUnobscuredContentOffset) {
if (targetRect.width() > unobscuredContentRect.width())
newUnobscuredContentOffset.setX(origin.x() - unobscuredContentRect.width() / 2);
if (targetRect.height() > unobscuredContentRect.height())
newUnobscuredContentOffset.setY(origin.y() - unobscuredContentRect.height() / 2);
newUnobscuredContentOffset = constrainContentOffset(newUnobscuredContentOffset, contentSize, unobscuredContentRect.size());
newUnobscuredContentOffset = constrainContentOffset(newUnobscuredContentOffset, minimumContentOffset, contentSize, unobscuredContentRect.size());
}

WebCore::FloatSize scrollViewOffsetDelta = newUnobscuredContentOffset - unobscuredContentOffset;
Expand Down
14 changes: 14 additions & 0 deletions Tools/TestWebKitAPI/Tests/WebKitCocoa/FindInPage.mm
Original file line number Diff line number Diff line change
Expand Up @@ -792,6 +792,20 @@ static void testPerformTextSearchWithQueryStringInWebView(WKWebView *webView, NS
EXPECT_TRUE(CGPointEqualToPoint([webView scrollView].contentOffset, CGPointMake(0, 664)));
}

TEST(WebKit, ScrollToFoundRangeAtTopWithContentInsets)
{
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)]);
[webView scrollView].contentInset = UIEdgeInsetsMake(30, 0, 0, 0);
[webView synchronouslyLoadHTMLString:@"<meta name='viewport' content='width=device-width,initial-scale=1'><div contenteditable><p>Top</p><p style='margin-top: 800px'>Bottom</p></div>"];
[webView objectByEvaluatingJavaScript:@"let p = document.querySelector('p'); document.getSelection().setBaseAndExtent(p, 0, p, 1)"];

auto ranges = textRangesForQueryString(webView.get(), @"Top");
[webView scrollRangeToVisible:[ranges firstObject] inDocument:nil];

TestWebKitAPI::Util::runFor(500_ms);
EXPECT_TRUE(CGPointEqualToPoint([webView scrollView].contentOffset, CGPointMake(0, -[webView scrollView].contentInset.top)));
}

TEST(WebKit, CannotHaveMultipleFindOverlays)
{
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:CGRectMake(0, 0, 200, 200)]);
Expand Down

0 comments on commit 2e3f11b

Please sign in to comment.