New issue
Advanced search Search tips

Issue 868102 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner:
Closed: Sep 5
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

[css-contain] Paint containment shouldn't create a stacking context for elements it doesn't apply

Project Member Reported by r...@igalia.com, Jul 26

Issue description


Check the attached example, it shows the difference between a regular div element vs a ruby-text element.

Paint containment applies on the regular for that reason it creates a stacking context.
However it doesn't apply on ruby-text elements, however it still creates the stacking context.

The problem is the code at ComputedStyle::UpdateIsStackingContext()
is using directly "ContainsPaint()" which only checks that the element has "contain: paint".
We should use LayoutObject::ShouldApplyPaintContainment() instead which is the one in charge of checking if paint containment should apply or not to the element.

 
paint-containment-bug-stacking-context.html
509 bytes View Download
Cc: r...@igalia.com
Owner: ----
Status: Available (was: Assigned)
I was taking a look to this as I thought it could be a simple fix,
but it's not that simple for me. :-/

This is a CL of what I tried but doesn't work, I'm uploading it
as the tests in the CL should be valid anyway:
https://chromium-review.googlesource.com/c/chromium/src/+/1158365

As you can see in that patch I'm removing the ContainsPaint() call
in ComputedStyle::UpdateIsStackingContext().
And I'm adding new code in LayoutTreeBuilderForElement::CreateLayoutObject()
to set IsStackingContext to true if the element has paint containment.
But it seems that's too late and the stacking context is not created.

The problem is that we cannot determine if the element creates or not
a stacking context without having the LayoutObject. For example an element
with "display: inline" won't have paint containment unless it's blockified
(for example if it's a flex or grid item) in that case paint containment
would apply to it. 

I don't know a lot about all this stacking context stuff,
so if anyone has ideas or suggestions about how to fix this
(or know the people with knowledge in this area) please tell me.
If someone wants to take this and fix it by themselves that's also fine with me.
Project Member

Comment 3 by bugdroid1@chromium.org, 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

The tests are now in WPT so I'm abandoning my CL.

I'll be on holidays until September starting next week,
so I don't think I'll have time to fix these before holidays.
Owner: masonfreed@chromium.org
Status: Assigned (was: Available)
Owner: chrishtr@chromium.org
Owner: futhark@chromium.org
Rune would you mind giving Rego some help with this? Then he can be unblocked
also on  crbug.com/870157 .
Labels: -Pri-3 Pri-2
Status: Started (was: Assigned)
The test-case uses ruby-text which is a display-inside value in css3-display which we don't support, so it will not pass unless we start implementing that.

They're defined in https://drafts.csswg.org/css-ruby/#ruby-display as well.

The ruby elements in the HTML spec are styled in the UA sheet with these values and Edge and Firefox supports them. We probably should too.

Cc: futhark@chromium.org
Owner: r...@igalia.com
Status: WontFix (was: Started)
Ok, I didn't realize about that. :-(

So I've reported a different bug for that particular topic: issue #880802.

Regarding this issue, I've created a different test using "display: inline" in WPT,
which works as expected in Chromium:
https://github.com/web-platform-tests/wpt/pull/12847

I couldn't find any failing case so far, so I guess current code is good enough
and we can close this issue.

Sign in to add a comment