[css-contain] Make contain:layout a stacking context |
||||
Issue descriptionThe CSSWG just resolved that contain:layout should be a stacking context. https://github.com/w3c/csswg-drafts/issues/2942 This requires an intent to ship and research into compat. Assigning to myself while I find an owner. Gecko bug: https://bugzilla.mozilla.org/show_bug.cgi?id=1472919
,
Aug 2
@rego: are you able to own this bug then?
,
Aug 2
I'm afraid I'd need some extra help, like for bug #868102 . I was trying to fix that one unsuccessfully (check the last comment on the bug with more details). If someone provides me further information I could work on this one too.
,
Aug 2
,
Aug 6
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dbc976dff2f1d80d5244de1efa91c4fdd5815066 commit dbc976dff2f1d80d5244de1efa91c4fdd5815066 Author: Manuel Rego Casasnovas <rego@igalia.com> Date: Mon Aug 06 15:29:29 2018 [css-contain] Link failures in WPT tests to the related bugs Two of the failures are related to stacking contexts: * external/wpt/css/css-contain/contain-paint-021.html * external/wpt/css/css-contain/contain-layout-016.html The other one was a mistake that has been already fixed in WPT (see https://github.com/web-platform-tests/wpt/pull/12314): * external/wpt/css/css-contain/contain-paint-001.html TBR=eae@chromium.org BUG= 870811 , 868102 , 870157 Change-Id: I60e89decd2f939d1d9a850a1b8b556d31658e872 Reviewed-on: https://chromium-review.googlesource.com/1163673 Reviewed-by: Manuel Rego <rego@igalia.com> Commit-Queue: Manuel Rego <rego@igalia.com> Cr-Commit-Position: refs/heads/master@{#580878} [modify] https://crrev.com/dbc976dff2f1d80d5244de1efa91c4fdd5815066/third_party/WebKit/LayoutTests/TestExpectations
,
Sep 5
So after realizing that we don't support any ruby-specific display values (issue #880802) I guess we could fix this just doing what I said won't be enough in comment #1 (http://ix.io/1m1l/diff). Now a question for chrishtr@ about this comment: > This requires an intent to ship and research into compat. Do you feel we need to do this for such a small change? We've been fixing different spec bugs related to css-contain (some that changed behavior similar to this one) in the past months without any of this. Current implementation has several issues and don't follow the spec in different parts. Should we do intents and research for similar fixes too? In the case we want to do this, I guess first step would be to add a use counter and then start to collect data to try to clarify the possible impact on current websites. Should we follow a different approach?
,
Sep 5
Cool! As for compat, hmm. Can you give an example of one successfully made without intent to ship or compat research?
,
Sep 6
These are a summary of the changes I've been doing on css-contain to make Chromium implementation closer to the spec, however I'm not sure if they're more or less common/important than this scencario: * #843329: Here we disabled layout, paint and size containment for certain elements to follow the spec (like inlines, internal table and ruby elements, ...). * #850169: Size containment should ignore contents in flexbox/grid. * #843320: Layout containment overflowing contents as ink overflow. This one was trickier as DevTools (#851422) and news.google.com (#856131) got problems. DevTools was promtly fixed, however it seems news.google.com still have the issue. * #785212: Layout containment abspos and fixed descendants. This is the one that got reverted as it broke YouTube mobile. Here we added a use counter https://www.chromestatus.com/metrics/feature/timeline/popularity/2463 and notified YouTube team that changed their code. Once the numbers were lower this was landed again.
,
Sep 6
Ok go ahead and make the change then, hopefully it will stick.
,
Sep 7
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f01f396190da97d7218ee2f5d76b6b637bf3ced2 commit f01f396190da97d7218ee2f5d76b6b637bf3ced2 Author: Manuel Rego Casasnovas <rego@igalia.com> Date: Fri Sep 07 22:28:21 2018 [css-contain] Make contain:layout a stacking context The CSSWG has recently resolved that layout containment should create a stacking context like paint containment (see issue https://github.com/w3c/csswg-drafts/issues/2942). This is the spec text about this (https://drafts.csswg.org/css-contain/#containment-layout): "5. The element creates a stacking context." The patch is just a simple change in ComputedStyle::UpdateIsStackingContext() to add the new condition. Apart from that TestExpectations file is updated as we're now passing some tests from WPT. We're also failing one test, but for an unrelated issue (see bug #880802). BUG= 870157 TEST=external/wpt/css/css-contain/contain-layout-016.html TEST=external/wpt/css/css-contain/contain-layout-018.html TEST=external/wpt/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-layout-stacking-context-001.html Change-Id: Icdc102692801534364adf3fc0929192b886cdbc4 Reviewed-on: https://chromium-review.googlesource.com/1211922 Commit-Queue: Manuel Rego <rego@igalia.com> Reviewed-by: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Chris Harrelson <chrishtr@chromium.org> Reviewed-by: Emil A Eklund <eae@chromium.org> Cr-Commit-Position: refs/heads/master@{#589689} [modify] https://crrev.com/f01f396190da97d7218ee2f5d76b6b637bf3ced2/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/f01f396190da97d7218ee2f5d76b6b637bf3ced2/third_party/blink/renderer/core/style/computed_style.cc [modify] https://crrev.com/f01f396190da97d7218ee2f5d76b6b637bf3ced2/third_party/blink/renderer/core/style/computed_style_test.cc
,
Sep 10
|
||||
►
Sign in to add a comment |
||||
Comment 1 by r...@igalia.com
, Aug 2