New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 850169 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

contain:size doesn't fully work on <button>

Project Member Reported by dholb...@gmail.com, Jun 6 2018

Issue description

Chrome Version: Version 68.0.3440.7 (Official Build) dev (64-bit)
OS: Ubuntu 18.04

What steps will reproduce the problem?
(1) Visit https://jsfiddle.net/mz2Lf4wj/
(2) Compare the adjacent "2" and "3" lines.

What is the expected result?
Line 2 should look the same as Line 3 (within the button section, as well as within the inline-block section), aside from the fact that the word "INSIDE" will be painted on line 3.  In other words -- quoting the spec: "When laying out [an element with contain:size], it must be treated as having no contents." https://drafts.csswg.org/css-contain/#containment-size

What happens instead?
 - For line 3 under "button", contain:size seems to only make the button's *height* collapse. But the button's width and baseline are still determined based on its contents.

The bottom half (the "inline-block" section) is a semi-reference case for the top half -- it demonstrates (in Chrome) how size containment is *supposed* to work. In that part, my expectations are met. But in the top half, for <button>, they are not met.
 

Comment 1 by dholb...@gmail.com, Jun 6 2018

Summary: contain:size doesn't fully work on <button> (was: contain:size doens't fully work on <button>)

Comment 2 by dholb...@gmail.com, Jun 6 2018

Description: Show this description
Cc: r...@igalia.com

Comment 4 by dholb...@gmail.com, Jun 7 2018

Description: Show this description

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

Status: Available (was: Untriaged)

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

Owner: r...@igalia.com
Status: Started (was: Available)
The problem is not only related to buttons, but flexboxes have the same issue
(as internally a button is a flexbox).

Also grids have a similar issue, despite they use a different code.
Project Member

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

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

commit 2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Tue Jun 12 01:37:48 2018

[css-contain] Size containment should ignore contents in flexbox/grid

The spec is pretty clear
(https://drafts.csswg.org/css-contain/#containment-size):
  "1. When laying out the containing element,
  it must be treated as having no contents."

The patch modifies ComputeIntrinsicLogicalWidths() and
FirstLineBoxBaseline() in LayoutFlexibleBox and LayoutGrid,
so the contents are ignored if the flex or grid container
has "contain: size".
Buttons are implemented as flexboxes internally,
so they get fixed too thanks to this change.

BUG= 850169 
TEST=external/wpt/css/css-contain/contain-size-button-001.html
TEST=external/wpt/css/css-contain/contain-size-flexbox-001.html
TEST=external/wpt/css/css-contain/contain-size-grid-001.html

Change-Id: Ia08c2c7b31126911cb4f0d1fc70aa444c22af8e3
Reviewed-on: https://chromium-review.googlesource.com/1093854
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#566228}
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-button-001.html
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-flexbox-001.html
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/contain-size-grid-001.html
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/reference/contain-size-button-001-ref.html
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/reference/contain-size-flexbox-001-ref.html
[add] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/WebKit/LayoutTests/external/wpt/css/css-contain/reference/contain-size-grid-001-ref.html
[modify] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/blink/renderer/core/layout/layout_flexible_box.cc
[modify] https://crrev.com/2d2551f9932f4407d1dcee7eb16cc7a9d8ff936c/third_party/blink/renderer/core/layout/layout_grid.cc

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

Status: Fixed (was: Started)
This should be fixed now.

Sign in to add a comment