Skip to content
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

[container-queries] Stop forcing layout containment but force an independent formatting context #31376

Conversation

karlcow
Copy link
Member

@karlcow karlcow commented Jul 29, 2024

9c47b8d

[container-queries] Stop forcing layout containment but force an independent formatting context
https://bugs.webkit.org/show_bug.cgi?id=277122
rdar://132549134

Reviewed by NOBODY (OOPS!).

container-type does not force layout containment, but does force
an independent formatting context by CSS WG resolution on
w3c/csswg-drafts#10544

This takes into account the latest import for container queries
WPT tests formatting
web-platform-tests/wpt@27c89c6

* LayoutTests/TestExpectations:
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::firstLineBaseline const):
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::establishesIndependentFormattingContext const):
(WebCore::RenderElement::hasEligibleContainmentForSizeQuery const):
* Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp:
(WebCore::StyleRareNonInheritedData::usedContain const):

Co-authored-by: Tim Nguyen <ntim@apple.com>

9c47b8d

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-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
✅ 🛠 tv
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@karlcow karlcow self-assigned this Jul 29, 2024
@karlcow karlcow added the CSS Cascading Style Sheets implementation label Jul 29, 2024
@karlcow
Copy link
Member Author

karlcow commented Jul 29, 2024

Let' start fresh.

@karlcow karlcow requested a review from nt1m July 29, 2024 06:23
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 29, 2024
@nt1m
Copy link
Member

nt1m commented Jul 29, 2024

I think I know what's causing most of these failures:

This needs to be removed: https://searchfox.org/wubkat/rev/b36cbce69fddb7da33823f316bd8ead5bebee970/Source/WebCore/rendering/RenderElement.cpp#2507-2509

@nt1m nt1m removed the merging-blocked Applied to prevent a change from being merged label Jul 29, 2024
@nt1m nt1m changed the title container-type does not force layout containment, but does force an independent formatting context [container-queries] Stop forcing layout containment but force an independent formatting context Jul 29, 2024
@nt1m nt1m force-pushed the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch from a361626 to a694dd8 Compare July 29, 2024 08:00
@nt1m nt1m self-assigned this Jul 29, 2024
@nt1m nt1m force-pushed the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch from a694dd8 to 5202a1e Compare July 29, 2024 08:02
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 29, 2024
@karlcow karlcow removed the merging-blocked Applied to prevent a change from being merged label Jul 30, 2024
@karlcow karlcow changed the title [container-queries] Stop forcing layout containment but force an independent formatting context [ Jul 30, 2024
@karlcow karlcow force-pushed the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch from 5202a1e to 3204996 Compare July 30, 2024 08:04
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 30, 2024
@karlcow karlcow changed the title [ [container-queries] Stop forcing layout containment but force an independent formatting context Jul 30, 2024
@karlcow karlcow removed the request for review from nikolaszimmermann July 30, 2024 09:54
@karlcow
Copy link
Member Author

karlcow commented Jul 30, 2024

houla… what happened.

@karlcow karlcow removed the merging-blocked Applied to prevent a change from being merged label Jul 30, 2024
@karlcow karlcow force-pushed the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch from d10b0b7 to 66f5a44 Compare July 30, 2024 10:01
@karlcow
Copy link
Member Author

karlcow commented Jul 30, 2024

So the tests for table have been fixed, except table-cell tests which still regress. So still 8 FAIL instead of the 16 FAIL initially.
http://wpt.live/css/css-conditional/container-queries/nested-query-containers.html

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 30, 2024
…pendent formatting context

https://bugs.webkit.org/show_bug.cgi?id=277122
rdar://132549134

Reviewed by NOBODY (OOPS!).

container-type does not force layout containment, but does force
an independent formatting context by CSS WG resolution on
w3c/csswg-drafts#10544

This takes into account the latest import for container queries
WPT tests formatting
web-platform-tests/wpt@27c89c6

* LayoutTests/TestExpectations:
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::firstLineBaseline const):
* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::establishesIndependentFormattingContext const):
(WebCore::RenderElement::hasEligibleContainmentForSizeQuery const):
* Source/WebCore/rendering/style/StyleRareNonInheritedData.cpp:
(WebCore::StyleRareNonInheritedData::usedContain const):

Co-authored-by: Tim Nguyen <ntim@apple.com>
@karlcow karlcow removed the merging-blocked Applied to prevent a change from being merged label Jul 31, 2024
@karlcow karlcow force-pushed the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch from 66f5a44 to 9c47b8d Compare July 31, 2024 02:53
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jul 31, 2024
@bfgeek
Copy link

bfgeek commented Sep 27, 2024

@karlcow @nt1m - Any change you got to investigate the failure? The change in Chromium looks like its sticky - so if possible it'd be great if WebKit can follow soon.

Thanks,
Ian

@karlcow
Copy link
Member Author

karlcow commented Oct 1, 2024

@bfgeek
@alanbaradlay asked me if he could take over 🎉
This should be more effective.

@bfgeek
Copy link

bfgeek commented Oct 3, 2024

@karlcow @alanbaradlay - The zero table-cell + baseline isn't particularly important to the test (IMO). If you like we can just change that test (to use vertical-align:middle instead), which then would unblock this change (basically verbatim).

Thougths?

@alanbaradlay
Copy link
Contributor

@karlcow @alanbaradlay - The zero table-cell + baseline isn't particularly important to the test (IMO). If you like we can just change that test (to use vertical-align:middle instead), which then would unblock this change (basically verbatim).

Thougths?

I agree that it's not super important. I could also just skip the test temporarily until after I address the baseline bug. Let's see if I can get to it in the next couple of days, if not, I'll land this one way or another.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 3, 2024
As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
aarongable pushed a commit to chromium/chromium that referenced this pull request Oct 3, 2024
As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363827}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 3, 2024
As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363827}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Oct 3, 2024
As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363827}
@nt1m nt1m closed this Oct 4, 2024
@karlcow
Copy link
Member Author

karlcow commented Oct 7, 2024

This is fixed by e0a7770

@karlcow karlcow deleted the eng/container-type-does-not-force-layout-containment-but-does-force-an-independent-formatting-context branch October 7, 2024 01:03
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 10, 2024
…ith extreme baselines., a=testonly

Automatic update from web-platform-tests
[flex] Fix line cross-size calculation with extreme baselines.

As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: David Grogan <dgrogan@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1363827}

--

wpt-commits: a860f3bee0f0c23461759e009149d36377c894ad
wpt-pr: 48453
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 15, 2024
…ith extreme baselines., a=testonly

Automatic update from web-platform-tests
[flex] Fix line cross-size calculation with extreme baselines.

As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: David Grogan <dgroganchromium.org>
Cr-Commit-Position: refs/heads/main{#1363827}

--

wpt-commits: a860f3bee0f0c23461759e009149d36377c894ad
wpt-pr: 48453

UltraBlame original commit: b4bd4104b815edf9525ea3f33cfd841a7149942a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 15, 2024
…ith extreme baselines., a=testonly

Automatic update from web-platform-tests
[flex] Fix line cross-size calculation with extreme baselines.

As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: David Grogan <dgroganchromium.org>
Cr-Commit-Position: refs/heads/main{#1363827}

--

wpt-commits: a860f3bee0f0c23461759e009149d36377c894ad
wpt-pr: 48453

UltraBlame original commit: b4bd4104b815edf9525ea3f33cfd841a7149942a
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 15, 2024
…ith extreme baselines., a=testonly

Automatic update from web-platform-tests
[flex] Fix line cross-size calculation with extreme baselines.

As pointed out in:
WebKit/WebKit#31376 (comment)

Blink/WebKit were doing something incorrect when a baseline was
outside the border-box of a flex-item.

This was because we were initializing max_major_descent and
max_minor_descent to zero, instead of LayoutUnit::Min(), (similar
to FlexLine::max_major_ascent_ FlexLine::max_minor_ascent_).

This only occurs with a single item in a flex-line.

Change-Id: I4f32eb912a07247d16231451ba63e7c7a75a8b23
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5901280
Commit-Queue: Ian Kilpatrick <ikilpatrickchromium.org>
Reviewed-by: David Grogan <dgroganchromium.org>
Cr-Commit-Position: refs/heads/main{#1363827}

--

wpt-commits: a860f3bee0f0c23461759e009149d36377c894ad
wpt-pr: 48453

UltraBlame original commit: b4bd4104b815edf9525ea3f33cfd841a7149942a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CSS Cascading Style Sheets implementation merging-blocked Applied to prevent a change from being merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants