New issue
Advanced search Search tips

Issue 907709 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 26
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[css-contain] contain:layout isn't suppressing baseline for some elements

Project Member Reported by dholb...@gmail.com, Nov 22

Issue description

Chrome Version: 72.0.3610.2 (Official Build) dev (64-bit)
OS: (e.g. Win10, MacOS 10.12, etc...)

What steps will reproduce the problem?
(1) load testcase: https://jsfiddle.net/1dau69bn/

What is the expected result?
The teal border-bottom edges should all be aligned.
(The boxes should suppress their "real" baselines, since they have 'contain:layout', and they should instead synthesize a baseline from their border-bottom edge, and they should do baseline alignment using that synthesized baseline.)

What happens instead?
The boxes for "block", "fieldset", and "multi col" are too low -- they're aligning with their contents' actual baselines.  They're not honoring the spec text about 'contain:layout'.

Firefox Nightly (with about:config pref layout.css.contain.enabled = true) gives the "expected result" here, thanks to :rego's work in https://bugzilla.mozilla.org/show_bug.cgi?id=1491235 .  Note also that I'll be adding a version of the above jsfiddle as a web platform test (indirectly, via mozilla's auto-synchronized w3c-css/submitted directory) over in https://bugzilla.mozilla.org/show_bug.cgi?id=1507663


Spec quote for "contain:layout" behavior:
====
7. For the purpose of the vertical-align property, or any other property whose effects need to relate the position of the containing element’s baseline to something other than its descendants, the containing element is treated as having no baseline.
====
https://drafts.csswg.org/css-contain/#containment-layout

Spec quote for what "treated as having no baseline" means in this context (for flex items):
===
If the item does not have a baseline in the necessary axis, then one is synthesized from the flex item’s border box.
===
https://drafts.csswg.org/css-flexbox-1/#valdef-align-items-baseline
 
Owner: r...@igalia.com
Status: Assigned (was: Untriaged)
Summary: [css-contain] contain:layout isn't suppressing baseline for some elements (was: contain:layout isn't suppressing baseline for some elements in Chrome)
Thanks for the bug report dholbert@.

The very same issue happens with a grid container instead of a flexbox.
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 26

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

commit 9e7dbfacbe70a63e1e0344a511a4ad3eb0905336
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Mon Nov 26 13:08:11 2018

[css-contain] Fix baseline for elements with contain: layout

Layout containment elements are treated as having no baseline,
when that's applied to table cells, flex or grid items
that are baseline aligned that means that they should use
their synthesized baseline.

The patch adds simple check in LayoutBlockFlow::FirstLineBoxBaseline()
similar to what we have in other methods. If the element has
layout containment it has no baseline (it returns -1).
The same thing is done in LayoutNGMixin<Base>::FragmentBaseline()
for LayoutNG.

Two tests from the WPT suite has been modified in order to avoid
baseline alignment as they are not checking that feature
of layout containment but something else.

BUG= 907709 
TEST=external/wpt/css/css-contain/contain-layout-baseline-002.html
TEST=external/wpt/css/css-contain/contain-layout-baseline-003.html
TEST=external/wpt/css/css-contain/contain-layout-baseline-004.html

Change-Id: Id393787dd24bf3e34aaf73476e446fb9d0a4e593
Reviewed-on: https://chromium-review.googlesource.com/c/1348892
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610833}
[modify] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/renderer/core/layout/layout_block_flow.cc
[modify] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/renderer/core/layout/ng/layout_ng_mixin.cc
[add] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/contain-layout-baseline-002.html
[add] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/contain-layout-baseline-003.html
[add] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/contain-layout-baseline-004.html
[modify] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/contain-layout-cell-001.html
[modify] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/contain-layout-cell-002.html
[add] https://crrev.com/9e7dbfacbe70a63e1e0344a511a4ad3eb0905336/third_party/blink/web_tests/external/wpt/css/css-contain/reference/contain-layout-baseline-004-ref.html

Status: Fixed (was: Assigned)

Sign in to add a comment