WebKit Bugzilla
Attachment 359442 Details for
Bug 193494
: [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch for landing
bug-193494-20190117193640.patch (text/plain), 39.74 KB, created by
Wenson Hsieh
on 2019-01-17 19:36:41 PST
(
hide
)
Description:
Patch for landing
Filename:
MIME Type:
Creator:
Wenson Hsieh
Created:
2019-01-17 19:36:41 PST
Size:
39.74 KB
patch
obsolete
>Subversion Revision: 240102 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index 2dd9d50fe636288195f458eb51848519ec65a993..63a3af885851f9650a4754a4fe15f7df1ca923b8 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,43 @@ >+2019-01-17 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas >+ https://bugs.webkit.org/show_bug.cgi?id=193494 >+ <rdar://problem/46859627> >+ >+ Reviewed by Simon Fraser and Tim Horton. >+ >+ When computing the content offset to scroll to when revealing a given rect in content coordinates, we currently >+ just use the unobscured content rect. As a result, when scrolling to reveal a rect, we'll clamp the final scroll >+ position such that only content is visible. For example, when asked to reveal the rect `(0, 0, 1, 1)`, we adjust >+ the scroll position to be the origin. >+ >+ However, consider the case where a client (e.g. Mail on iOS) has added a content inset to the web view's scroll >+ view. If we're asked to reveal a rect that is outside the content area but within a content inset, we will still >+ end up clamping the scroll position to the unobscured rect. This manifests in a bug where selecting text and >+ autoscrolling in iOS Mail compose while the scroll view is scrolled all the way to the top to reveal the To/Cc/ >+ Subject fields causes the content offset to jump to the origin, rather than staying at (0, -topContentInset). >+ >+ To fix this, we teach `RenderLayer::scrollRectToVisible` about content insets that are visible. Rather than use >+ the content rects as-is, expand to encompass visible content insets as well. This ensures that revealing a >+ position which is already visible won't cause us to scroll away the content inset area and only show the >+ unobscured rect. >+ >+ Tests: editing/selection/ios/autoscroll-with-top-content-inset.html >+ fast/scrolling/ios/scroll-into-view-with-top-content-inset.html >+ >+ * page/FrameView.cpp: >+ (WebCore::FrameView::unobscuredContentRectExpandedByContentInsets const): >+ >+ Introduce a helper method that expands the unobscured content rect to include surrounding content insets. >+ >+ * page/FrameView.h: >+ * page/Page.h: >+ (WebCore::Page::contentInsets const): >+ (WebCore::Page::setContentInsets): >+ * rendering/RenderLayer.cpp: >+ (WebCore::RenderLayer::scrollRectToVisible): >+ (WebCore::RenderLayer::getRectToExpose const): >+ > 2019-01-16 Justin Fan <justin_fan@apple.com> > > [WebGPU] Update vertex-buffer-triangle-strip.html to actually use vertex buffer >diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog >index e492f8af27f8e1849826495a30f68393b87ff200..9561f9f31f0ded6cd814fb5ded88e4078011f3e8 100644 >--- a/Source/WebKit/ChangeLog >+++ b/Source/WebKit/ChangeLog >@@ -1,3 +1,48 @@ >+2019-01-17 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas >+ https://bugs.webkit.org/show_bug.cgi?id=193494 >+ <rdar://problem/46859627> >+ >+ Reviewed by Simon Fraser and Tim Horton. >+ >+ Adds `contentInsets` to `VisibleContentRectUpdateInfo`. This keeps track of the visible content insets >+ surrounding the unobscured content rect. See WebCore ChangeLog for more details. >+ >+ * Shared/VisibleContentRectUpdateInfo.cpp: >+ (WebKit::VisibleContentRectUpdateInfo::encode const): >+ (WebKit::VisibleContentRectUpdateInfo::decode): >+ (WebKit::operator<<): >+ * Shared/VisibleContentRectUpdateInfo.h: >+ (WebKit::VisibleContentRectUpdateInfo::VisibleContentRectUpdateInfo): >+ (WebKit::VisibleContentRectUpdateInfo::contentInsets const): >+ (WebKit::operator==): >+ * UIProcess/API/Cocoa/WKWebView.mm: >+ (-[WKWebView _computedObscuredInset]): >+ (-[WKWebView _computedContentInset]): >+ (-[WKWebView _computedUnobscuredSafeAreaInset]): >+ >+ We don't care about source compatibility with iOS 10 and below anymore, so we should change these >= iOS 11 >+ target checks to simply `PLATFORM(IOS)`. >+ >+ (-[WKWebView _updateVisibleContentRects]): >+ >+ Compute the visible content insets on all sides of the unobscured content rect. These insets are scaled to >+ content coordinates. >+ >+ * UIProcess/ios/WKContentView.h: >+ * UIProcess/ios/WKContentView.mm: >+ (floatBoxExtent): >+ >+ Add a helper to convert `UIEdgeInsets` to `WebCore::FloatBoxExtent`, and use it in a few places below. >+ >+ (-[WKContentView didUpdateVisibleRect:unobscuredRect:contentInsets:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]): >+ (-[WKContentView didUpdateVisibleRect:unobscuredRect:unobscuredRectInScrollViewCoordinates:obscuredInsets:unobscuredSafeAreaInsets:inputViewBounds:scale:minimumScale:inStableState:isChangingObscuredInsetsInteractively:enclosedInScrollableAncestorView:]): Deleted. >+ * WebProcess/WebPage/ios/WebPageIOS.mm: >+ (WebKit::WebPage::updateVisibleContentRects): >+ >+ Update the Page's content insets. >+ > 2019-01-16 Youenn Fablet <youenn@apple.com> > > Add a new SPI for controlling getUserMedia >diff --git a/Source/WebCore/page/FrameView.cpp b/Source/WebCore/page/FrameView.cpp >index 8dd924d797c1673bd07cc84268bb48be155d9487..06698156db4b5c210ee00868d1dc47fa8c07dc41 100644 >--- a/Source/WebCore/page/FrameView.cpp >+++ b/Source/WebCore/page/FrameView.cpp >@@ -2014,6 +2014,14 @@ void FrameView::viewportContentsChanged() > #endif > } > >+IntRect FrameView::unobscuredContentRectExpandedByContentInsets() const >+{ >+ FloatRect unobscuredContentRect = this->unobscuredContentRect(); >+ if (auto* page = frame().page()) >+ unobscuredContentRect.expand(page->contentInsets()); >+ return IntRect(unobscuredContentRect); >+} >+ > bool FrameView::fixedElementsLayoutRelativeToFrame() const > { > return frame().settings().fixedElementsLayoutRelativeToFrame(); >diff --git a/Source/WebCore/page/FrameView.h b/Source/WebCore/page/FrameView.h >index 44298462742d52dcd7c698d4c9c84b9bce997a7f..0c781ee62a3637f37ee67bac1e8361781fb955af 100644 >--- a/Source/WebCore/page/FrameView.h >+++ b/Source/WebCore/page/FrameView.h >@@ -326,6 +326,8 @@ public: > // Static function can be called from another thread. > WEBCORE_EXPORT static LayoutRect rectForViewportConstrainedObjects(const LayoutRect& visibleContentRect, const LayoutSize& totalContentsSize, float frameScaleFactor, bool fixedElementsLayoutRelativeToFrame, ScrollBehaviorForFixedElements); > #endif >+ >+ IntRect unobscuredContentRectExpandedByContentInsets() const; > > bool fixedElementsLayoutRelativeToFrame() const; > >diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h >index 522be986c1bb14f97a28a8a5ddaad8c29352ee96..b06531f9329c41866f91e0bc18ec9295693a0101 100644 >--- a/Source/WebCore/page/Page.h >+++ b/Source/WebCore/page/Page.h >@@ -347,6 +347,9 @@ public: > const FloatBoxExtent& obscuredInsets() const { return m_obscuredInsets; } > void setObscuredInsets(const FloatBoxExtent& obscuredInsets) { m_obscuredInsets = obscuredInsets; } > >+ const FloatBoxExtent& contentInsets() const { return m_contentInsets; } >+ void setContentInsets(const FloatBoxExtent& insets) { m_contentInsets = insets; } >+ > const FloatBoxExtent& unobscuredSafeAreaInsets() const { return m_unobscuredSafeAreaInsets; } > WEBCORE_EXPORT void setUnobscuredSafeAreaInsets(const FloatBoxExtent&); > >@@ -791,6 +794,7 @@ private: > > float m_topContentInset { 0 }; > FloatBoxExtent m_obscuredInsets; >+ FloatBoxExtent m_contentInsets; > FloatBoxExtent m_unobscuredSafeAreaInsets; > FloatBoxExtent m_fullscreenInsets; > Seconds m_fullscreenAutoHideDuration { 0_s }; >diff --git a/Source/WebCore/rendering/RenderLayer.cpp b/Source/WebCore/rendering/RenderLayer.cpp >index 6c2a423b6776fc990f0a7e0f340661bccd0f237d..2f2817e0dde975dd3de9ff4b9299addffe3d1f90 100644 >--- a/Source/WebCore/rendering/RenderLayer.cpp >+++ b/Source/WebCore/rendering/RenderLayer.cpp >@@ -2551,7 +2551,7 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid > #if !PLATFORM(IOS_FAMILY) > LayoutRect viewRect = frameView.visibleContentRect(); > #else >- LayoutRect viewRect = frameView.unobscuredContentRect(); >+ LayoutRect viewRect = frameView.unobscuredContentRectExpandedByContentInsets(); > #endif > // Move the target rect into "scrollView contents" coordinates. > LayoutRect targetRect = absoluteRect; >@@ -2591,7 +2591,7 @@ void RenderLayer::updateCompositingLayersAfterScroll() > } > } > >-LayoutRect RenderLayer::getRectToExpose(const LayoutRect &visibleRect, const LayoutRect &exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY) const >+LayoutRect RenderLayer::getRectToExpose(const LayoutRect& visibleRect, const LayoutRect& exposeRect, bool insideFixed, const ScrollAlignment& alignX, const ScrollAlignment& alignY) const > { > FrameView& frameView = renderer().view().frameView(); > if (renderer().isRenderView() && insideFixed) { >diff --git a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp >index 2ee8d96c4ea0cc0d34b22160fcdc10a09e26043e..e9a883d28bfe6671cd2856446174114d3750e169 100644 >--- a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp >+++ b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.cpp >@@ -37,6 +37,7 @@ void VisibleContentRectUpdateInfo::encode(IPC::Encoder& encoder) const > { > encoder << m_exposedContentRect; > encoder << m_unobscuredContentRect; >+ encoder << m_contentInsets; > encoder << m_unobscuredContentRectRespectingInputViewBounds; > encoder << m_unobscuredRectInScrollViewCoordinates; > encoder << m_customFixedPositionRect; >@@ -61,6 +62,8 @@ bool VisibleContentRectUpdateInfo::decode(IPC::Decoder& decoder, VisibleContentR > return false; > if (!decoder.decode(result.m_unobscuredContentRect)) > return false; >+ if (!decoder.decode(result.m_contentInsets)) >+ return false; > if (!decoder.decode(result.m_unobscuredContentRectRespectingInputViewBounds)) > return false; > if (!decoder.decode(result.m_unobscuredRectInScrollViewCoordinates)) >@@ -114,6 +117,7 @@ TextStream& operator<<(TextStream& ts, const VisibleContentRectUpdateInfo& info) > > ts.dumpProperty("exposedContentRect", info.exposedContentRect()); > ts.dumpProperty("unobscuredContentRect", info.unobscuredContentRect()); >+ ts.dumpProperty("contentInsets", info.contentInsets()); > ts.dumpProperty("unobscuredContentRectRespectingInputViewBounds", info.unobscuredContentRectRespectingInputViewBounds()); > ts.dumpProperty("unobscuredRectInScrollViewCoordinates", info.unobscuredRectInScrollViewCoordinates()); > ts.dumpProperty("customFixedPositionRect", info.customFixedPositionRect()); >diff --git a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h >index 91c1822a74acdef890fe561205fd1ba87d533ec4..814ca54c21d3f5335672e1ce23b5f6cb8d5f8eac 100644 >--- a/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h >+++ b/Source/WebKit/Shared/VisibleContentRectUpdateInfo.h >@@ -45,9 +45,10 @@ class VisibleContentRectUpdateInfo { > public: > VisibleContentRectUpdateInfo() = default; > >- VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& customFixedPositionRect, const WebCore::FloatBoxExtent& obscuredInsets, const WebCore::FloatBoxExtent& unobscuredSafeAreaInsets, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView, MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId) >+ VisibleContentRectUpdateInfo(const WebCore::FloatRect& exposedContentRect, const WebCore::FloatRect& unobscuredContentRect, const WebCore::FloatBoxExtent& contentInsets, const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates, const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds, const WebCore::FloatRect& customFixedPositionRect, const WebCore::FloatBoxExtent& obscuredInsets, const WebCore::FloatBoxExtent& unobscuredSafeAreaInsets, double scale, bool inStableState, bool isFirstUpdateForNewViewSize, bool isChangingObscuredInsetsInteractively, bool allowShrinkToFit, bool enclosedInScrollableAncestorView, MonotonicTime timestamp, double horizontalVelocity, double verticalVelocity, double scaleChangeRate, uint64_t lastLayerTreeTransactionId) > : m_exposedContentRect(exposedContentRect) > , m_unobscuredContentRect(unobscuredContentRect) >+ , m_contentInsets(contentInsets) > , m_unobscuredContentRectRespectingInputViewBounds(unobscuredContentRectRespectingInputViewBounds) > , m_unobscuredRectInScrollViewCoordinates(unobscuredRectInScrollViewCoordinates) > , m_customFixedPositionRect(customFixedPositionRect) >@@ -69,6 +70,7 @@ public: > > const WebCore::FloatRect& exposedContentRect() const { return m_exposedContentRect; } > const WebCore::FloatRect& unobscuredContentRect() const { return m_unobscuredContentRect; } >+ const WebCore::FloatBoxExtent& contentInsets() const { return m_contentInsets; } > const WebCore::FloatRect& unobscuredRectInScrollViewCoordinates() const { return m_unobscuredRectInScrollViewCoordinates; } > const WebCore::FloatRect& unobscuredContentRectRespectingInputViewBounds() const { return m_unobscuredContentRectRespectingInputViewBounds; } > const WebCore::FloatRect& customFixedPositionRect() const { return m_customFixedPositionRect; } >@@ -97,6 +99,7 @@ public: > private: > WebCore::FloatRect m_exposedContentRect; > WebCore::FloatRect m_unobscuredContentRect; >+ WebCore::FloatBoxExtent m_contentInsets; > WebCore::FloatRect m_unobscuredContentRectRespectingInputViewBounds; > WebCore::FloatRect m_unobscuredRectInScrollViewCoordinates; > WebCore::FloatRect m_customFixedPositionRect; // When visual viewports are enabled, this is the layout viewport. >@@ -121,6 +124,7 @@ inline bool operator==(const VisibleContentRectUpdateInfo& a, const VisibleConte > return a.scale() == b.scale() > && a.exposedContentRect() == b.exposedContentRect() > && a.unobscuredContentRect() == b.unobscuredContentRect() >+ && a.contentInsets() == b.contentInsets() > && a.unobscuredContentRectRespectingInputViewBounds() == b.unobscuredContentRectRespectingInputViewBounds() > && a.customFixedPositionRect() == b.customFixedPositionRect() > && a.obscuredInsets() == b.obscuredInsets() >diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >index 4a441b0cdd8a6d7d15d8f1a1f3ae05fc8b61cb56..ae0e250b40f7eebf76929c2df487e554d474bfa6 100644 >--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm >@@ -1734,7 +1734,7 @@ - (UIEdgeInsets)_computedObscuredInset > if (_haveSetObscuredInsets) > return _obscuredInsets; > >-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000 >+#if PLATFORM(IOS) > if (self._safeAreaShouldAffectObscuredInsets) > return UIEdgeInsetsAdd(UIEdgeInsetsZero, self._scrollViewSystemContentInset, self._effectiveObscuredInsetEdgesAffectedBySafeArea); > #endif >@@ -1749,7 +1749,7 @@ - (UIEdgeInsets)_computedContentInset > > UIEdgeInsets insets = [_scrollView contentInset]; > >-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000 >+#if PLATFORM(IOS) > if (self._safeAreaShouldAffectObscuredInsets) > insets = UIEdgeInsetsAdd(insets, self._scrollViewSystemContentInset, self._effectiveObscuredInsetEdgesAffectedBySafeArea); > #endif >@@ -1762,7 +1762,7 @@ - (UIEdgeInsets)_computedUnobscuredSafeAreaInset > if (_haveSetUnobscuredSafeAreaInsets) > return _unobscuredSafeAreaInsets; > >-#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000 >+#if PLATFORM(IOS) > if (!self._safeAreaShouldAffectObscuredInsets) > return self.safeAreaInsets; > #endif >@@ -2975,6 +2975,26 @@ - (void)_updateVisibleContentRects > CGRect unobscuredRectInContentCoordinates = _frozenUnobscuredContentRect ? _frozenUnobscuredContentRect.value() : [self convertRect:unobscuredRect toView:_contentView.get()]; > unobscuredRectInContentCoordinates = CGRectIntersection(unobscuredRectInContentCoordinates, [self _contentBoundsExtendedForRubberbandingWithScale:scaleFactor]); > >+ // The following logic computes the extent to which the bottom, top, left and right content insets are visible. >+ auto scrollViewInsets = [_scrollView contentInset]; >+ auto scrollViewBounds = [_scrollView bounds]; >+ auto scrollViewContentSize = [_scrollView contentSize]; >+ auto scrollViewOriginIncludingInset = UIEdgeInsetsInsetRect(scrollViewBounds, computedContentInsetUnadjustedForKeyboard).origin; >+ auto maximumVerticalScrollExtentWithoutRevealingBottomContentInset = scrollViewContentSize.height - CGRectGetHeight(scrollViewBounds); >+ auto maximumHorizontalScrollExtentWithoutRevealingRightContentInset = scrollViewContentSize.width - CGRectGetWidth(scrollViewBounds); >+ auto contentInsets = UIEdgeInsetsZero; >+ if (scrollViewInsets.left > 0 && scrollViewOriginIncludingInset.x < 0) >+ contentInsets.left = std::min(-scrollViewOriginIncludingInset.x, scrollViewInsets.left) / scaleFactor; >+ >+ if (scrollViewInsets.top > 0 && scrollViewOriginIncludingInset.y < 0) >+ contentInsets.top = std::min(-scrollViewOriginIncludingInset.y, scrollViewInsets.top) / scaleFactor; >+ >+ if (scrollViewInsets.right > 0 && scrollViewOriginIncludingInset.x > maximumHorizontalScrollExtentWithoutRevealingRightContentInset) >+ contentInsets.right = std::min(scrollViewOriginIncludingInset.x - maximumHorizontalScrollExtentWithoutRevealingRightContentInset, scrollViewInsets.right) / scaleFactor; >+ >+ if (scrollViewInsets.bottom > 0 && scrollViewOriginIncludingInset.y > maximumVerticalScrollExtentWithoutRevealingBottomContentInset) >+ contentInsets.bottom = std::min(scrollViewOriginIncludingInset.y - maximumVerticalScrollExtentWithoutRevealingBottomContentInset, scrollViewInsets.bottom) / scaleFactor; >+ > #if ENABLE(CSS_SCROLL_SNAP) && ENABLE(ASYNC_SCROLLING) > if (inStableState) { > WebKit::RemoteScrollingCoordinatorProxy* coordinator = _page->scrollingCoordinatorProxy(); >@@ -2994,6 +3014,7 @@ - (void)_updateVisibleContentRects > > [_contentView didUpdateVisibleRect:visibleRectInContentCoordinates > unobscuredRect:unobscuredRectInContentCoordinates >+ contentInsets:contentInsets > unobscuredRectInScrollViewCoordinates:unobscuredRect > obscuredInsets:_obscuredInsets > unobscuredSafeAreaInsets:[self _computedUnobscuredSafeAreaInset] >diff --git a/Source/WebKit/UIProcess/ios/WKContentView.h b/Source/WebKit/UIProcess/ios/WKContentView.h >index a0607e3d1abc75289817991a4b2e61ef8292c169..522aca19d02b089688cc823233c9e01e683ebc56 100644 >--- a/Source/WebKit/UIProcess/ios/WKContentView.h >+++ b/Source/WebKit/UIProcess/ios/WKContentView.h >@@ -71,6 +71,7 @@ class WebProcessPool; > > - (void)didUpdateVisibleRect:(CGRect)visibleRect > unobscuredRect:(CGRect)unobscuredRect >+ contentInsets:(UIEdgeInsets)contentInsets > unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates > obscuredInsets:(UIEdgeInsets)obscuredInsets > unobscuredSafeAreaInsets:(UIEdgeInsets)unobscuredSafeAreaInsets >diff --git a/Source/WebKit/UIProcess/ios/WKContentView.mm b/Source/WebKit/UIProcess/ios/WKContentView.mm >index ced85e8c9918a7b567167d97ae88db60fafaf3a5..304c23f401bff648e83286d23f3db5e5e4225508 100644 >--- a/Source/WebKit/UIProcess/ios/WKContentView.mm >+++ b/Source/WebKit/UIProcess/ios/WKContentView.mm >@@ -365,6 +365,11 @@ - (void)_didExitStableState > [_textSelectionAssistant deactivateSelection]; > } > >+static WebCore::FloatBoxExtent floatBoxExtent(UIEdgeInsets insets) >+{ >+ return WebCore::FloatBoxExtent(insets.top, insets.right, insets.bottom, insets.left); >+} >+ > - (CGRect)_computeUnobscuredContentRectRespectingInputViewBounds:(CGRect)unobscuredContentRect inputViewBounds:(CGRect)inputViewBounds > { > // The input view bounds are in window coordinates, but the unobscured rect is in content coordinates. Account for this by converting input view bounds to content coordinates. >@@ -376,6 +381,7 @@ - (CGRect)_computeUnobscuredContentRectRespectingInputViewBounds:(CGRect)unobscu > > - (void)didUpdateVisibleRect:(CGRect)visibleContentRect > unobscuredRect:(CGRect)unobscuredContentRect >+ contentInsets:(UIEdgeInsets)contentInsets > unobscuredRectInScrollViewCoordinates:(CGRect)unobscuredRectInScrollViewCoordinates > obscuredInsets:(UIEdgeInsets)obscuredInsets > unobscuredSafeAreaInsets:(UIEdgeInsets)unobscuredSafeAreaInsets >@@ -404,11 +410,12 @@ - (void)didUpdateVisibleRect:(CGRect)visibleContentRect > WebKit::VisibleContentRectUpdateInfo visibleContentRectUpdateInfo( > visibleContentRect, > unobscuredContentRect, >+ floatBoxExtent(contentInsets), > unobscuredRectInScrollViewCoordinates, > unobscuredContentRectRespectingInputViewBounds, > fixedPositionRectForLayout, >- WebCore::FloatBoxExtent(obscuredInsets.top, obscuredInsets.right, obscuredInsets.bottom, obscuredInsets.left), >- WebCore::FloatBoxExtent(unobscuredSafeAreaInsets.top, unobscuredSafeAreaInsets.right, unobscuredSafeAreaInsets.bottom, unobscuredSafeAreaInsets.left), >+ floatBoxExtent(obscuredInsets), >+ floatBoxExtent(unobscuredSafeAreaInsets), > zoomScale, > isStableState, > _sizeChangedSinceLastVisibleContentRectUpdate, >diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >index 06696c065cc7748c1cbde00545e37364ea694e53..9b09a0afcbb2aa86700f7da2b54ea97875cb0cea 100644 >--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm >@@ -3016,6 +3016,7 @@ void WebPage::updateVisibleContentRects(const VisibleContentRectUpdateInfo& visi > viewportConfigurationChanged(); > > frameView.setUnobscuredContentSize(visibleContentRectUpdateInfo.unobscuredContentRect().size()); >+ m_page->setContentInsets(visibleContentRectUpdateInfo.contentInsets()); > m_page->setObscuredInsets(visibleContentRectUpdateInfo.obscuredInsets()); > m_page->setUnobscuredSafeAreaInsets(visibleContentRectUpdateInfo.unobscuredSafeAreaInsets()); > m_page->setEnclosedInScrollableAncestorView(visibleContentRectUpdateInfo.enclosedInScrollableAncestorView()); >diff --git a/Tools/ChangeLog b/Tools/ChangeLog >index 077d393294b53bee78a3c494e8655d76d98290d8..18edb5f98eb86cd76f8e45185203b02abec3f83a 100644 >--- a/Tools/ChangeLog >+++ b/Tools/ChangeLog >@@ -1,3 +1,35 @@ >+2019-01-17 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas >+ https://bugs.webkit.org/show_bug.cgi?id=193494 >+ <rdar://problem/46859627> >+ >+ Reviewed by Simon Fraser and Tim Horton. >+ >+ Add a new test option to add a top content inset to the test runner's WKWebView's scroll view, and automatically >+ scroll to reveal this top content inset area when beginning the test (i.e., scroll to (0, -topContentInset)). >+ >+ * DumpRenderTree/ios/UIScriptControllerIOS.mm: >+ (WTR::UIScriptController::contentOffsetX const): >+ (WTR::UIScriptController::contentOffsetY const): >+ * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl: >+ * TestRunnerShared/UIScriptContext/UIScriptController.cpp: >+ (WTR::UIScriptController::contentOffsetX const): >+ (WTR::UIScriptController::contentOffsetY const): >+ * TestRunnerShared/UIScriptContext/UIScriptController.h: >+ >+ Also add new UIScriptController methods to ask for the content offset of the platform scroll view. >+ >+ * WebKitTestRunner/TestController.cpp: >+ (WTR::updateTestOptionsFromTestHeader): >+ * WebKitTestRunner/TestOptions.h: >+ (WTR::TestOptions::hasSameInitializationOptions const): >+ * WebKitTestRunner/ios/TestControllerIOS.mm: >+ (WTR::TestController::platformResetStateToConsistentValues): >+ * WebKitTestRunner/ios/UIScriptControllerIOS.mm: >+ (WTR::UIScriptController::contentOffsetX const): >+ (WTR::UIScriptController::contentOffsetY const): >+ > 2019-01-16 Youenn Fablet <youenn@apple.com> > > Add a new SPI for controlling getUserMedia >diff --git a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm >index 398034a4c20e39acaef269ce94589484824c2b96..8a1125f4ef9198452669e0419c589a8902e8a670 100644 >--- a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm >+++ b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm >@@ -211,6 +211,16 @@ static CGPoint contentOffsetBoundedInValidRange(UIScrollView *scrollView, CGPoin > return contentOffset; > } > >+double UIScriptController::contentOffsetX() const >+{ >+ return [gWebScrollView contentOffset].x; >+} >+ >+double UIScriptController::contentOffsetY() const >+{ >+ return [gWebScrollView contentOffset].y; >+} >+ > void UIScriptController::scrollToOffset(long x, long y) > { > [gWebScrollView setContentOffset:contentOffsetBoundedInValidRange(gWebScrollView, CGPointMake(x, y)) animated:YES]; >diff --git a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl >index 7d204e85726a86d3fa8ec2c38b9778c13b706623..5424ba8a8cfbe4ad814c0dabe6117337eedf967c 100644 >--- a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl >+++ b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl >@@ -231,6 +231,9 @@ interface UIScriptController { > > void resignFirstResponder(); > >+ readonly attribute double contentOffsetX; >+ readonly attribute double contentOffsetY; >+ > void scrollToOffset(long x, long y); // Initiate an animated scroll in the UI process. > attribute object didEndScrollingCallback; > >diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp >index c9587c42e9f03efa8994d54d39019e7678ec6b78..a5baa06cd533200599f3af56893e375f6e0233c1 100644 >--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp >+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp >@@ -336,6 +336,16 @@ JSRetainPtr<JSStringRef> UIScriptController::formInputLabel() const > return nullptr; > } > >+double UIScriptController::contentOffsetX() const >+{ >+ return 0; >+} >+ >+double UIScriptController::contentOffsetY() const >+{ >+ return 0; >+} >+ > void UIScriptController::scrollToOffset(long x, long y) > { > } >diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h >index a6482c9e7940a9e5745fb026406408d0b4e3458e..d3fb1cbb09250a8279dfcb28bbf8c9027f7021cb 100644 >--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h >+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h >@@ -113,6 +113,9 @@ public: > JSObjectRef contentsOfUserInterfaceItem(JSStringRef) const; > void overridePreference(JSStringRef preference, JSStringRef value); > >+ double contentOffsetX() const; >+ double contentOffsetY() const; >+ > void scrollToOffset(long x, long y); > > void immediateScrollToOffset(long x, long y); >diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp >index 7c88e0027f3d750be61567232a881adcdfb4dda8..926177ef2c7729b48a3bc037a41a1b6f2c4382d3 100644 >--- a/Tools/WebKitTestRunner/TestController.cpp >+++ b/Tools/WebKitTestRunner/TestController.cpp >@@ -1266,6 +1266,8 @@ static void updateTestOptionsFromTestHeader(TestOptions& testOptions, const std: > testOptions.editable = parseBooleanTestHeaderValue(value); > else if (key == "enableUndoManagerAPI") > testOptions.enableUndoManagerAPI = parseBooleanTestHeaderValue(value); >+ else if (key == "contentInset.top") >+ testOptions.contentInsetTop = std::stod(value); > pairStart = pairEnd + 1; > } > } >diff --git a/Tools/WebKitTestRunner/TestOptions.h b/Tools/WebKitTestRunner/TestOptions.h >index 10cd4e4c34571b0b1b8a02cb6dd479c67fec7340..614ce7b5818597ea9a4813adc7b2e1fc46a46079 100644 >--- a/Tools/WebKitTestRunner/TestOptions.h >+++ b/Tools/WebKitTestRunner/TestOptions.h >@@ -69,6 +69,8 @@ struct TestOptions { > bool editable { false }; > bool enableUndoManagerAPI { false }; > >+ double contentInsetTop { 0 }; >+ > float deviceScaleFactor { 1 }; > Vector<String> overrideLanguages; > std::string applicationManifest; >@@ -113,7 +115,8 @@ struct TestOptions { > || shouldIgnoreMetaViewport != options.shouldIgnoreMetaViewport > || enableEditableImages != options.enableEditableImages > || editable != options.editable >- || enableUndoManagerAPI != options.enableUndoManagerAPI) >+ || enableUndoManagerAPI != options.enableUndoManagerAPI >+ || contentInsetTop != options.contentInsetTop) > return false; > > if (experimentalFeatures != options.experimentalFeatures) >diff --git a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm >index 4a1080cad49df10dc1b4446705c076c1e0afbb4d..855d2d5bc700340474c4271cc7b7aaa951020043 100644 >--- a/Tools/WebKitTestRunner/ios/TestControllerIOS.mm >+++ b/Tools/WebKitTestRunner/ios/TestControllerIOS.mm >@@ -132,7 +132,8 @@ void TestController::platformResetStateToConsistentValues(const TestOptions& opt > UIScrollView *scrollView = webView.scrollView; > [scrollView _removeAllAnimations:YES]; > [scrollView setZoomScale:1 animated:NO]; >- [scrollView setContentOffset:CGPointZero]; >+ scrollView.contentInset = UIEdgeInsetsMake(options.contentInsetTop, 0, 0, 0); >+ scrollView.contentOffset = CGPointMake(0, -options.contentInsetTop); > > if (webView.interactingWithFormControl) > shouldRestoreFirstResponder = [webView resignFirstResponder]; >diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm >index 9e20477b1e868267f49c4a0092b0ad58c3d49186..4aa94290578748cf81bc5355076dfa27a8a81c5d 100644 >--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm >+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm >@@ -487,6 +487,18 @@ static CGPoint contentOffsetBoundedInValidRange(UIScrollView *scrollView, CGPoin > return contentOffset; > } > >+double UIScriptController::contentOffsetX() const >+{ >+ TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView(); >+ return webView.scrollView.contentOffset.x; >+} >+ >+double UIScriptController::contentOffsetY() const >+{ >+ TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView(); >+ return webView.scrollView.contentOffset.y; >+} >+ > void UIScriptController::scrollToOffset(long x, long y) > { > TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView(); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index e856254f6b5005505f8a1dd3c3c961e87901d7c1..738272c3fc9e3e95049e6c9a370340d688a578ec 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,27 @@ >+2019-01-17 Wenson Hsieh <wenson_hsieh@apple.com> >+ >+ [iOS] Content offset jumps erratically when autoscrolling near scroll view content inset areas >+ https://bugs.webkit.org/show_bug.cgi?id=193494 >+ <rdar://problem/46859627> >+ >+ Reviewed by Simon Fraser and Tim Horton. >+ >+ * editing/selection/ios/autoscroll-with-top-content-inset-expected.txt: Added. >+ * editing/selection/ios/autoscroll-with-top-content-inset.html: Added. >+ >+ Add a new test to verify that moving the selection by autoscrolling near the top content inset area does not >+ cause the scroll view's content offset to jump. >+ >+ * fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt: Added. >+ * fast/scrolling/ios/scroll-into-view-with-top-content-inset.html: Added. >+ >+ Add a new test to verify that programmatically scrolling an element that's already visible into view does not >+ scroll away the scroll view's content inset. >+ >+ * resources/ui-helper.js: >+ (window.UIHelper.contentOffset): >+ (window.UIHelper): >+ > 2019-01-16 Alicia Boya GarcÃa <aboya@igalia.com> > > Unreviewed GTK and WPE test gardening. >diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..3ee5cff85a82615cd35321e10fd0beeb24c74db2 >--- /dev/null >+++ b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-expected.txt >@@ -0,0 +1,13 @@ >+Select me and drag up. >+ >+Verifies that triggering autoscroll near the top of a web view with a top content inset does not cause the scroll view's content offset to jump to 0. This test requires WebKitTestRunner. To verify manually, load this page in a web view that has a scroll view top content inset, and scroll such that the full top content inset area is visible. Check that starting a text selection loupe gesture near the top of the top of the document and dragging upwards does not thrash the scroll view's content offset. >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS finalContentOffset.x is 0 >+PASS verticalMovementDuringDrag < 1 is true >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html >new file mode 100644 >index 0000000000000000000000000000000000000000..f4337ef0f41d03294f9a843a6e2eccb25e6efc85 >--- /dev/null >+++ b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset.html >@@ -0,0 +1,51 @@ >+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true, contentInset.top=100 ] --> >+<html> >+<meta name="viewport" content="width=device-width, initial-scale=1"> >+<head> >+ <script src="../../../resources/basic-gestures.js"></script> >+ <script src="../../../resources/ui-helper.js"></script> >+ <script src="../../../resources/js-test.js"></script> >+ <style> >+ body { >+ margin: 0; >+ box-sizing: border-box; >+ border: red 1px solid; >+ } >+ >+ #text { >+ font-size: 30px; >+ } >+ >+ #console { >+ overflow: scroll; >+ height: 100px; >+ } >+ </style> >+ <script> >+ jsTestIsAsync = true; >+ >+ async function run() >+ { >+ await UIHelper.activateAndWaitForInputSessionAt(110, 40); >+ originalContentOffset = await UIHelper.contentOffset(); >+ await longPressAndHoldAtPoint(110, 40); >+ await touchAndDragFromPointToPoint(110, 40, 210, 40); >+ await liftUpAtPoint(210, 40); >+ finalContentOffset = await UIHelper.contentOffset(); >+ verticalMovementDuringDrag = Math.abs(finalContentOffset.y - originalContentOffset.y); >+ >+ shouldBe("finalContentOffset.x", "0"); >+ shouldBeTrue("verticalMovementDuringDrag < 1"); >+ finishJSTest(); >+ } >+ </script> >+</head> >+<body contenteditable onload="run()"> >+ <p id="text"><strong>Select me and drag up.</strong></p> >+ <p id="description"></p> >+ <p id="console"></p> >+</body> >+<script> >+ description("Verifies that triggering autoscroll near the top of a web view with a top content inset does not cause the scroll view's content offset to jump to 0. This test requires WebKitTestRunner. To verify manually, load this page in a web view that has a scroll view top content inset, and scroll such that the full top content inset area is visible. Check that starting a text selection loupe gesture near the top of the top of the document and dragging upwards does not thrash the scroll view's content offset."); >+</script> >+</html> >diff --git a/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..f7f479c8583cc6763ade6f7a28c451b90b245c46 >--- /dev/null >+++ b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset-expected.txt >@@ -0,0 +1,13 @@ >+Verifies that Element.scrollIntoView() does not scroll away the top content inset if the element is already visible. This test requires WebKitTestRunner. >+ >+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". >+ >+ >+PASS originalContentOffset.x is 0 >+PASS originalContentOffset.y is -100 >+PASS finalContentOffset.x is 0 >+PASS finalContentOffset.y is -100 >+PASS successfullyParsed is true >+ >+TEST COMPLETE >+ >diff --git a/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html >new file mode 100644 >index 0000000000000000000000000000000000000000..fd77ef6f5130b3bba75ab08f6b7ab14e86cd9e39 >--- /dev/null >+++ b/LayoutTests/fast/scrolling/ios/scroll-into-view-with-top-content-inset.html >@@ -0,0 +1,47 @@ >+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true, contentInset.top=100 ] --> >+<html> >+<meta name="viewport" content="width=device-width, initial-scale=1"> >+<head> >+ <script src="../../../resources/ui-helper.js"></script> >+ <script src="../../../resources/js-test.js"></script> >+ <style> >+ body { >+ margin: 0; >+ border: red 1px solid; >+ } >+ >+ #target { >+ position: absolute; >+ width: 4px; >+ height: 4px; >+ top: 0px; >+ left: 0px; >+ background-color: silver; >+ } >+ </style> >+ <script> >+ jsTestIsAsync = true; >+ >+ async function run() >+ { >+ originalContentOffset = await UIHelper.contentOffset(); >+ target.scrollIntoView({ block: "nearest", inline: "nearest" }); >+ finalContentOffset = await UIHelper.contentOffset(); >+ >+ shouldBe("originalContentOffset.x", "0"); >+ shouldBe("originalContentOffset.y", "-100"); >+ shouldBe("finalContentOffset.x", "0"); >+ shouldBe("finalContentOffset.y", "-100"); >+ finishJSTest(); >+ } >+ </script> >+</head> >+<body contenteditable onload="run()"> >+ <div id="target"></div> >+ <p id="description"></p> >+ <p id="console"></p> >+</body> >+<script> >+ description("Verifies that Element.scrollIntoView() does not scroll away the top content inset if the element is already visible. This test requires WebKitTestRunner."); >+</script> >+</html> >diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js >index 2f569aacc55c45c7025ce216a70345729224bc6e..e65f031d640d321f8aa098c3cf3a3e0e46e47309 100644 >--- a/LayoutTests/resources/ui-helper.js >+++ b/LayoutTests/resources/ui-helper.js >@@ -504,4 +504,16 @@ window.UIHelper = class UIHelper { > const escapedIdentifier = identifier.replace(/`/g, "\\`"); > return new Promise(resolve => testRunner.runUIScript(`uiController.setKeyboardInputModeIdentifier(\`${escapedIdentifier}\`)`, resolve)); > } >+ >+ static contentOffset() >+ { >+ if (!this.isIOS()) >+ return Promise.resolve(); >+ >+ const uiScript = "JSON.stringify([uiController.contentOffsetX, uiController.contentOffsetY])"; >+ return new Promise(resolve => testRunner.runUIScript(uiScript, result => { >+ const [offsetX, offsetY] = JSON.parse(result) >+ resolve({ x: offsetX, y: offsetY }); >+ })); >+ } > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 193494
:
359379
|
359437
|
359441
| 359442