Skip to content

Fix broken z-ordering on pages with "position: sticky" #46930

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix broken z-ordering on pages with "position: sticky" #46930

wants to merge 1 commit into from

Conversation

kalenikaliaksandr
Copy link

@kalenikaliaksandr kalenikaliaksandr commented Jun 18, 2025 •
edited by webkit-early-warning-system
Loading

0b5336e

Fix broken z-ordering on pages with "position: sticky"

https://bugs.webkit.org/show_bug.cgi?id=238127

Reviewed by NOBODY (OOPS!).

Sticky elements could move around without recomputing overlap extent,
which leads to incorrect z-ordering when a sticky element moves onto an
element that would otherwise be promoted to have a separate layer for
indirect "overlap" reason. The bug is fixed using the same approach
applied in 5b652c3 to "position: fixed" elements: expanding overlap
extent to cover all possible sticky positions.

* LayoutTests/compositing/layer-creation/sticky-overlap-extent-expected.txt: Added.
* LayoutTests/compositing/layer-creation/sticky-overlap-extent.html: Added.
* Source/WebCore/page/scrolling/ScrollingConstraints.cpp:
(WebCore::StickyPositionViewportConstraints::computeStickyExtent const):
* Source/WebCore/page/scrolling/ScrollingConstraints.h:
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::stickyScrollableAreaBoundsInflatedForScrolling):
(WebCore::RenderLayerCompositor::computeExtent const):

44f0532

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp loading 🛠 jsc-armv7
✅ 🛠 tv-sim loading 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 18, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 0b5336e)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792 Ahmad-S792 added the CSS Cascading Style Sheets implementation label Jun 18, 2025
@Ahmad-S792 Ahmad-S792 requested review from smfr and mattwoodrow June 18, 2025 22:42
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 18, 2025
@kalenikaliaksandr kalenikaliaksandr marked this pull request as draft June 19, 2025 02:58
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 9bd39bc)

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
⏳ 🛠 🧪 jsc ⏳ 🧪 api-ios   🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 vision ⏳ 🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ⏳ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 20, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 4032537)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 4a793be)

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
✅ 🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 21, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 29090bd)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 22, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 2220211)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 22, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 0e2a654)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@kalenikaliaksandr
Copy link
Author

I removed a test because I ran out of ideas for how to make one that isn’t flaky across all runners. the closest one I got that fails only on mac-wk1 by returning empty string from layerTreeAsText(), which is supposedly means the layers tree hasn't been built yet:

<!DOCTYPE html>

<html>
    <head>
        <style>
            .sticky {
                position: sticky;
                top: 0;
                width: 100px;
                height: 100px;
                background: green;
            }
            .box {
                position: relative;
                height: 200px;
                height: 200px;
                background: blue;
            }
        </style>
        <script>
            if (window.testRunner) {
                testRunner.dumpAsText();
            }

            function doTest() {
                if (window.testRunner) {
                    document.getElementById('result').innerText = internals.layerTreeAsText(document);
                }
            }
            window.addEventListener('load', doTest, false);
        </script>
    </head>
    <body>
        <div class="container">
            <div class="sticky"></div>
            <div class="box">This element should be composited</div>
        </div>
        <pre id="result"></pre>
    </body>
</html>

expectation:

This element should be composited
(GraphicsLayer
  (anchor 0.00 0.00)
  (bounds 800.00 600.00)
  (children 1
    (GraphicsLayer
      (bounds 800.00 600.00)
      (contentsOpaque 1)
      (children 2
        (GraphicsLayer
          (position 8.00 8.00)
          (preserves3D 1)
          (children 1
            (GraphicsLayer
              (bounds 100.00 100.00)
              (contentsOpaque 1)
            )
          )
        )
        (GraphicsLayer
          (position 8.00 108.00)
          (bounds 784.00 200.00)
          (contentsOpaque 1)
          (drawsContent 1)
        )
      )
    )
  )
)

I also tried another approach by doing a ref-test but it ended up being even more fragile, because it has to force layout tree built, then do scrolling in such way that overlap testing is not performed (window.scrollTo() seem to force layer tree update) and only then allow capturing of screenshot.

@kalenikaliaksandr kalenikaliaksandr marked this pull request as ready for review June 22, 2025 02:46
@mattwoodrow
Copy link
Contributor

I removed a test because I ran out of ideas for how to make one that isn’t flaky across all runners. the closest one I got that fails only on mac-wk1 by returning empty string from layerTreeAsText(), which is supposedly means the layers tree hasn't been built yet:

It looks like we just don't get compositing for sticky in this case with WK1, and without any composited layers, the dump returns empty.
I think it'd be fine to just add wk1 specific expectations (either -expected.txt in platform/wk1, or a [ Failure ] annotation in platform/wk1/TestExpectations).

I'll wait to see if Simon can review the actual code change, he's much more versed in the sticky code than I am.

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash f5081a0)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 28, 2025 •
edited
Loading

EWS run on previous version of this PR (hash d2f5616)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@Ahmad-S792 Ahmad-S792 removed the merging-blocked Applied to prevent a change from being merged label Jun 28, 2025
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 28, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 29, 2025 •
edited
Loading

EWS run on previous version of this PR (hash b8e8439)

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
loading 🧪 webkitpy   🧪 ios-wk2-wpt   🧪 mac-wk1 ⏳ 🛠 wpe-cairo
  🧪 api-ios   🧪 mac-wk2   🛠 gtk
loading 🛠 vision   🧪 mac-AS-debug-wk2   🧪 gtk-wk2
✅ 🛠 vision-sim   🧪 mac-wk2-stress   🧪 api-gtk
  🧪 vision-wk2   🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@kalenikaliaksandr
Fix broken z-ordering on pages with "position: sticky"
44f0532 
https://bugs.webkit.org/show_bug.cgi?id=238127

Reviewed by NOBODY (OOPS!).

Sticky elements could move around without recomputing overlap extent,
which leads to incorrect z-ordering when a sticky element moves onto an
element that would otherwise be promoted to be composited for indirect
"overlap" reason. The bug is fixed using the same approach applied in
5b652c3 to "position: fixed" elements: expanding overlap extent to cover
all possible sticky positions.

Bug 238127 mentioned CSS grids, but the issue turned out to be more
general and at least applies to all sticky elements whose containing
block is the root element.

* LayoutTests/compositing/layer-creation/sticky-overlap-extent-expected.txt: Added.
* LayoutTests/compositing/layer-creation/sticky-overlap-extent.html: Added.
* LayoutTests/platform/mac-wk1/TestExpectations:
* Source/WebCore/page/scrolling/ScrollingConstraints.cpp:
(WebCore::StickyPositionViewportConstraints::computeStickyExtent const):
* Source/WebCore/page/scrolling/ScrollingConstraints.h:
* Source/WebCore/rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeExtent const):
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 29, 2025 •
edited
Loading

EWS run on current version of this PR (hash 44f0532)

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
✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2   🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp loading 🛠 jsc-armv7
✅ 🛠 tv-sim loading 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cdumez cdumez Awaiting requested review from cdumez cdumez is a code owner

@smfr smfr Awaiting requested review from smfr

@mattwoodrow mattwoodrow Awaiting requested review from mattwoodrow

Assignees

@kalenikaliaksandr kalenikaliaksandr

Labels
CSS Cascading Style Sheets implementation merging-blocked Applied to prevent a change from being merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kalenikaliaksandr @webkit-early-warning-system @mattwoodrow @Ahmad-S792 @webkit-ews-buildbot