Account for popover/dialog styles without overlay property [chromium/src : main]

0 views

Skip to first unread message

Joey Arhar (Gerrit)

unread,

Jan 22, 2026, 2:24:40 PM (22 hours ago) Jan 22

to Menard, Alexis, Daniil Sakhapov, Rune Lillesveen, Chromium LUCI CQ, AyeAye, apavlo...@chromium.org, blink-rev...@chromium.org, blink-re...@chromium.org, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org

Attention needed from Rune Lillesveen

Joey Arhar added 1 comment

Commit Message

File-level comment, Patchset 1:

Joey Arhar . resolved

Hey yall, I realized that there are a bunch of tests unexpectedly failing after I made this patch to remove the overlay property: https://chromium-review.googlesource.com/c/chromium/src/+/7133219
You can see what the failures look like: https://chromium-layout-test-archives.storage.googleapis.com/results.html?json=chromium/try/linux-rel/2493697/headless_shell_wpt_tests%20%28with%20patch%29/full_results_jsonp.js
It looks like some tests which are supposed to have scroll markers don't have them anymore, and some tests which weren't supposed to have a scrollbar now have a scrollbar. Did I miss something in my patch to put overlay behind a flag?

Rune Lillesveen

My guess is that it's a side-effect of this one now being IsEmpty() more often now:
https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/css/element_rule_collector.cc;l=1386-1388
Adding sakhapov@ for what the proper condition for creating a ::scroll-marker is.

Daniil Sakhapov

This is probably the same reason as - https://issues.chromium.org/issues/464010034, will fix.

Joey Arhar

Thanks for the patch! It looks like there are still some more tests failing

Rune Lillesveen

Thanks for the patch! It looks like there are still some more tests failing
That's probably expected. There's a second CL needed which hasn't landed yet.

Joey Arhar

Thanks for the second patch!
I still see 29 tests failing, including one scroll marker test: https://ci.chromium.org/ui/p/chromium/builders/try/linux-rel/2510728/test-results

Daniil Sakhapov

The last scroll marker test is on my list.
The other tests are unknown to me, but given the (probably) common root issue I'll try to fix them as well.
Stay tuned for the next CLs! 😊

Joey Arhar

Thank you Daniil!

Joey Arhar

From offline discussion, it looks like the remaining failures like missing scrollbars are due to other issues which are different from the scroll marker ones.
I tried to debug one of the test failures, menu-elements-default-style.html, which looks like the anchor positioned element is positioned in the top left corner instead of under its anchor. However, when I open the test in chrome or content_shell, the top layer element is positioned correctly - the issue only happens when running it as a test, and it didn't help to add a double rAF before taking the screenshot.
I'm not sure what's wrong with this or all the other failures, so now I'm looking at other ways to get the performance gain: https://chromium-review.googlesource.com/c/chromium/src/+/7277167

Joey Arhar

Nevermind, I'm starting to make more progress :)

Joey Arhar

I've fixed a bunch of them with the latest patch set, but I'll have another patch to continue working on the remaining ones

Open in Gerrit

Related details

Attention is currently required from:

Rune Lillesveen

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Rune Lillesveen (Gerrit)

unread,

5:11 AM (8 hours ago) 5:11 AM

Attention needed from Joey Arhar

Rune Lillesveen added 1 comment

File third_party/blink/renderer/core/html/html_element.h

Line 394, Patchset 7 (Latest): bool SupportsTopLayerStyle() const override;

Rune Lillesveen . unresolved

What does SupportsTopLayerStyle() mean? Documentation?

Open in Gerrit

Related details

Attention is currently required from:

Joey Arhar

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

No-Unresolved-Comments

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

0 new messages