-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Liquid Glass] [iOS] Blank gap above top fixed header when rubber-banding against top of page #47140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
EWS run on previous version of this PR (hash b417c26) |
EWS run on previous version of this PR (hash 0f876be) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to write a layout test for the (color extension in the) rubberband case?
It's a bit tricky since it's not part of the page or anything... but I can try! Edit: managed to put together an API test! |
EWS run on current version of this PR (hash 6463fab) |
Thanks for the review! |
…ding against top of page https://bugs.webkit.org/show_bug.cgi?id=294933 rdar://154233194 Reviewed by Abrar Rahman Protyasha. Make an adjustment to how we compute `-_obscuredInsetsForFixedColorExtension` on iOS, such that we expand the height of the top fixed color to span the area above the content while rubber-banding. * Source/WebCore/WebCore.xcodeproj/project.pbxproj: * Source/WebCore/page/PageColorSampler.h: * Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm: (-[WKWebView _obscuredInsetsForFixedColorExtension]): There are a few conditions where we avoid performing this extension: 1. If the scroll view is not scrolled past the top `adjustedContentInset`, we're not rubber- banding, so there's no gap to close in the first place. 2. If there is no predominant color on the top, then there is either no detected fixed header near the top of the page, or the fixed header near the top of the page has a backdrop filter. In either case we just fall back to the background color, which is what would show up in the gap anyways without extending the height of the color extension view. 3. If the sampled page top color is different than the sampled top fixed/sticky color, then there's no need to extend the height of the color extension view because the color extension view isn't adjacent to the top of the page. This can occur in the case where the header is sticky, or changes between static and fixed depending on the page's scroll position. 4. If the under-page background color is the same as the sampled top fixed color, then there's no need to extend the height of the color extension view since the result would be visually similar anyways. If none of the conditions above are met, we expand the height of the color extension view. * Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h: * Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm: (-[WKWebView _colorExtensionViewForTesting:]): Add a testing-only helper to retrieve color extension views on each side. * Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj: * Tools/TestWebKitAPI/Tests/WebKitCocoa/SampledPageTopColor.mm: (TEST(SampledPageTopColor, TopColorExtensionWhenRubberBanding)): * Tools/TestWebKitAPI/Tests/WebKitCocoa/top-fixed-element.html: Added. Add an API test to exercise this new behavior, by checking the height of fixed color extension views before and after scrolling past the top scroll extent. Canonical link: https://commits.webkit.org/296602@main
6463fab
to
f1caad5
Compare
Committed 296602@main (f1caad5): https://commits.webkit.org/296602@main Reviewed commits have been landed. Closing PR #47140 and removing active labels. |
f1caad5
6463fab