New issue
Advanced search Search tips

Issue 843329 link

Starred by 2 users
Status: Fixed
Owner:
r...@igalia.com
Closed: Jun 2018
Cc:
flor...@rivoal.net
e...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[css-contain] Disable containment on different elements

Project Member Reported by r...@igalia.com, May 15 2018 

In the attached example you can see a <span> element with "contain: paint"
that gets it applied when it shouldn't as it's a non-atomic inline.

Text from the spec (https://drafts.csswg.org/css-contain/#containment-paint):
  ... if the element’s principal box is a non-atomic inline-level box,
  paint containment has no effect.
contain-paint-span.html
127 bytes View Download
contain-paint-span.png
10.7 KB View Download

Comment 1 by r...@igalia.com, May 16 2018

Summary: [css-contain] Disable containment on different elements (was: [css-contain] Paint containment shouldn't apply to non-atomic inline-level boxes)
This seems a more generic issue as paint, layout and size containment have similar restrictions:
* Internal table elements.
* Internal ruby elements.
* Non-atomic inline-level boxes.

Note that each type of containment has its own restrictions,
not all of them have the ones listed above.

This is the reason why some WPT tests are failing now like:
http://w3c-test.org/css/css-contain/contain-paint-002.html

I believe this could be a generic bug for all the changes that are required
in order to disable containment effects on the different elements.
Project Member

Comment 2 by bugdroid1@chromium.org, May 25 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c42d28a2c412eebb39c96e81d1a08191be7cb404

commit c42d28a2c412eebb39c96e81d1a08191be7cb404
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Fri May 25 11:38:24 2018

[css-contain] Disable paint containment in non-atomic inlines

Text from the spec
(https://drafts.csswg.org/css-contain/#containment-paint):
  "... if the element's principal box is a non-atomic inline-level box,
  paint containment has no effect."

The patch disables "contain: paint" in non-atomic inline-level boxes.
For that a new method LayoutObject::ShouldApplyPaintContainment()
is added, which is used instead of ComputeStyle::ContainsPaint()
all around the code.
It's worth to highlight the change in
ComputedStyle::CanContainFixedPositionObjects() as it doesn't know
if the element is inline or not, it cannot check
if we're in paint containment anymore.
Otherwise it'd make a containing block non-atomic inlines
with "contain: paint", the ones in which we're disabling
paint containment on this patch.

More changes on that new method would be needed to fulfill
the rest of requirements from the spec, as paint containment
shouldn't apply to internal table and ruby elements for example.

Also similar patches for other kind of containment like layout and size
are going to be required.

Regarding tests this patch makes us pass contain-paint-002.html.
On top of that a few extra tests have been added.

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-paint-011.html
TEST=external/wpt/css/css-contain/contain-paint-012.html
TEST=external/wpt/css/css-contain/contain-paint-013.html

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_layout_ng;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: I140fad061f5cda14e52451aa8f4bc70c9edede3e
Reviewed-on: https://chromium-review.googlesource.com/1071794
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561832}
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-011.html
[add] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-012.html
[add] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-013-ref.html
[add] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-013.html
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_inline.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/layout_table_box_component.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/ng/inline/ng_inline_item.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/layout/paint_containment_test.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/paint/paint_layer_clipper.cc
[modify] https://crrev.com/c42d28a2c412eebb39c96e81d1a08191be7cb404/third_party/blink/renderer/core/style/computed_style.h

Comment 3 by viswa.karala@chromium.org, May 28 2018

Labels: TE-Verified-M69 TE-Verified-69.0.3443.0
Able to reproduce the issue on chrome version 68.0.3440.0(build without fix)
Verified the fix on Mac 10.12.6, Windows-10 & Ubuntu 14.04 on Chrome version #69.0.3443.0 as per the comment#0
Attaching screenshot for reference.
Observed "Able to see excepted behaviour as shown in screenshot in comment#0"
Hence, the fix is working as expected.
Adding the verified label.

Thanks!
843329.png
92.3 KB View Download
Project Member

Comment 4 by bugdroid1@chromium.org, May 30 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/69a85361696cec163ed6cab1edc3f393720b6a2b

commit 69a85361696cec163ed6cab1edc3f393720b6a2b
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Wed May 30 09:33:09 2018

[css-grid] Disable paint containment for ruby-text elements

Text from the spec
(https://drafts.csswg.org/css-contain/#containment-paint):
  "... if the element is an internal ruby element ...
  paint containment has no effect."

The patch disables "contain: paint" for ruby-text elements.
We need to modify the reference file for contain-paint-008 test
because of an unrelated issue (see crbug.com/847743).

Other internal ruby elements like ruby-base, ruby-base-container,
and ruby-base-text-container are already inline elements
so they don't need extra checks.
The tests checking those contain-paint-005, contain-paint-006,
and contain-paint-007 do not pass due to a different issue
related to absolute positions and ruby (see crbug.com/847274).
TestExpectations is updated to point to the actual problem.

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-paint-008.html

Change-Id: I26814598a3b806b67264146386052cbdfff62847
Reviewed-on: https://chromium-review.googlesource.com/1077387
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#562776}
[modify] https://crrev.com/69a85361696cec163ed6cab1edc3f393720b6a2b/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/69a85361696cec163ed6cab1edc3f393720b6a2b/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/reference/contain-paint-008-ref.html
[modify] https://crrev.com/69a85361696cec163ed6cab1edc3f393720b6a2b/third_party/blink/renderer/core/layout/layout_object.h
Project Member

Comment 5 by bugdroid1@chromium.org, May 30 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c1aee266961732caac973b18dc6c6f87ae6b59ca

commit c1aee266961732caac973b18dc6c6f87ae6b59ca
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Wed May 30 21:24:56 2018

[css-contain] Disable paint containment on internal table elements

Text from the spec
(https://drafts.csswg.org/css-contain/#containment-paint):
  "... if the element is an internal table element other than
  'display: table-cell', ... paint containment has no effect."

The patch disables "contain: paint" for internal table elements
except table cells.

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-paint-014.html
TEST=external/wpt/css/css-contain/contain-paint-015.html
TEST=external/wpt/css/css-contain/contain-paint-016.html
TEST=external/wpt/css/css-contain/contain-paint-017.html
TEST=external/wpt/css/css-contain/contain-paint-018.html
TEST=external/wpt/css/css-contain/contain-paint-019.html

Change-Id: I975950ab72fe5cc5aa106fa5fdafd37c64dd6d85
Reviewed-on: https://chromium-review.googlesource.com/1078868
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#563000}
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-014.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-015.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-016.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-017.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-018.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-paint-019.html
[add] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/reference/contain-paint-014-ref.html
[modify] https://crrev.com/c1aee266961732caac973b18dc6c6f87ae6b59ca/third_party/blink/renderer/core/layout/layout_object.h
Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cd1672b012e2eef90d0d6c7e227200dbda98fff1

commit cd1672b012e2eef90d0d6c7e227200dbda98fff1
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Fri Jun 01 11:00:36 2018

[css-contain] Disable size containment for certain elements

Text from the spec
(https://drafts.csswg.org/css-contain/#containment-size):
  "... if the element is an internal table element, or if the element
  is an internal ruby element, or if the element’s principal box
  is a non-atomic inline-level box, size containment has no effect."

The patch disables "contain: size" for internal table elements,
internal ruby elements and non-atomic inline-level boxes.

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-size-005.html
TEST=external/wpt/css/css-contain/contain-size-006.html
TEST=external/wpt/css/css-contain/contain-size-007.html
TEST=external/wpt/css/css-contain/contain-size-008.html
TEST=external/wpt/css/css-contain/contain-size-009.html
TEST=external/wpt/css/css-contain/contain-size-010.html
TEST=external/wpt/css/css-contain/contain-size-011.html
TEST=external/wpt/css/css-contain/contain-size-012.html

Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ie3264b5c76eb04ec97064b3da8706d840924597f
Reviewed-on: https://chromium-review.googlesource.com/1080799
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#563586}
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-006.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-007.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-008.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-009.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-010.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-011.html
[add] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-012.html
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/layout_block.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/layout_object.h
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/ng/ng_block_layout_algorithm.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.cc
[modify] https://crrev.com/cd1672b012e2eef90d0d6c7e227200dbda98fff1/third_party/blink/renderer/core/layout/ng/ng_layout_input_node.h
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 5 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600

commit b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Tue Jun 05 20:09:12 2018

[css-contain] Disable layout containment for certain elements

Text from the spec
(https://drafts.csswg.org/css-contain/#containment-layout):
  "... if the element is an internal table element other than
  display: table-cell, or if the element is an internal ruby element,
  or if the element’s principal box is a non-atomic inline-level box,
  layout containment has no effect."

The patch disables "contain: layout" for internal table elements
but table cells, internal ruby elements and
non-atomic inline-level boxes.

Some of the new tests pass or fail incidentally due to  crbug.com/785212 ,
once that's fixed all the tests should be working as expected.

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-layout-008.html
TEST=external/wpt/css/css-contain/contain-layout-009.html
TEST=external/wpt/css/css-contain/contain-layout-010.html
TEST=external/wpt/css/css-contain/contain-layout-011.html
TEST=external/wpt/css/css-contain/contain-layout-012.html
TEST=external/wpt/css/css-contain/contain-layout-013.html
TEST=external/wpt/css/css-contain/contain-layout-014.html

Change-Id: I5f0893a9ce69dc8e76cf16acb1b8556bf0e02adf
Reviewed-on: https://chromium-review.googlesource.com/1087268
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#564631}
[modify] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-008.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-009.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-010.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-011.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-012.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-013.html
[add] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-layout-014.html
[modify] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/b9e5f3f7bc73cef7d939fbe92b79f37dfb3f6600/third_party/blink/renderer/core/layout/layout_object.h

Comment 8 by r...@igalia.com, Jun 5 2018

Owner: r...@igalia.com
Status: Fixed (was: Available)
With the last change this should be fixed now.
Project Member

Comment 9 by bugdroid1@chromium.org, Jun 14 2018 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1ae77fbaea90159a2545ee682d3a57010e13eb70

commit 1ae77fbaea90159a2545ee682d3a57010e13eb70
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Thu Jun 14 16:11:28 2018

[css-contain] Disable size containment for tables

The CSSWG resolved about it in the last meeting:
https://github.com/w3c/csswg-drafts/issues/2746

BUG= 843329 
TEST=external/wpt/css/css-contain/contain-size-012.html

Change-Id: Ibb037f9ab1b95bed03e46fdbeb0e68520ff741b4
Reviewed-on: https://chromium-review.googlesource.com/1100824
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#567289}
[modify] https://crrev.com/1ae77fbaea90159a2545ee682d3a57010e13eb70/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-012.html
[modify] https://crrev.com/1ae77fbaea90159a2545ee682d3a57010e13eb70/third_party/blink/renderer/core/layout/layout_object.h
►

Sign in to add a comment