Skip to content

[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

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

whsieh
Copy link
Member

@whsieh whsieh commented Jun 24, 2025

f1caad5

[Liquid Glass] [iOS] Blank gap above top fixed header when rubber-banding 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

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ⏳ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@whsieh whsieh requested a review from cdumez as a code owner June 24, 2025 22:27
@whsieh whsieh self-assigned this Jun 24, 2025
@whsieh whsieh added the New Bugs Unclassified bugs are placed in this component until the correct component can be determined. label Jun 24, 2025
Copy link
Member

@aprotyas aprotyas left a 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?

@whsieh
Copy link
Member Author

whsieh commented Jun 24, 2025

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!

@whsieh whsieh added the merge-queue Applied to send a pull request to merge-queue label Jun 25, 2025
@whsieh
Copy link
Member Author

whsieh commented Jun 25, 2025

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
@webkit-commit-queue
Copy link
Collaborator

Committed 296602@main (f1caad5): https://commits.webkit.org/296602@main

Reviewed commits have been landed. Closing PR #47140 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit f1caad5 into WebKit:main Jun 25, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants