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

Issue 671010 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Use other robhogan account instead.
Closed: Dec 13
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 3
Type: Bug



Sign in to add a comment

Should % height cell children be allowed to overflow?

Project Member Reported by robho...@gmail.com, Dec 4 2016

Issue description

https://chromium.googlesource.com/chromium/src/+/8cbbe0ffc3c5d96b3b912ef931c3cb29625e7643/third_party/WebKit/LayoutTests/fast/table/auto-with-percent-height.html

In this test shouldn't we let each <div> get 100% height of the cell and thus overflow? As they would if they were inside a div with a fixed height?

I think the answer is 'no' - but I need to understand the reason so I can retain the behaviour while fixing something else. At the moment Blink only treats them as auto because it doesn't let anything with overflow visible or hidden to calculate its percent height - it just makes them size as auto.

Francois - help!

!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.0//EN">
<html>
<head>
<style>
.table {
  background-color: purple;
  color: white;
  height: 100%;
}
.outer {
  border: 3px solid red;
  margin: 1em;
}
</style>
</head>
<body>
<table class=outer>
  <tr><td><div class=table>Div One</div><div class=table>Div Two</div>
</table>
 

Comment 1 by robho...@gmail.com, Dec 4 2016

Answered my own question: it's covered by https://drafts.csswg.org/css2/visudet.html#the-height-property
It is indeed true that percentage-heights do not apply if the parent block is auto-sized. I will make sure this is clarified directly in the tables spec, right now there is no mention to that fact.

For tables is it a bit trickier though, because cells cannot have their own height and inherit their height from their parent row (except in Firefox, which is a bug they are tracking). 

As a result, whether the parent is considered to have an height should rely on the row, and sadly this is not what seems to be interoperable.

https://jsfiddle.net/La2rbjjo/1/
https://jsfiddle.net/La2rbjjo/2/
https://jsfiddle.net/La2rbjjo/3/

Here is Edge behavior:
- A cell is considered non-auto-sized 
-- if it has a height specified in pixels OR
-- if the parent table height is specified in pixels OR 
-- if the parent table height is specified in percent (and this percentage is not ignored as auto due to the auto-sized parent)

Here is Chrome behavior:
- A cell is considered non-auto-sized 
-- if it has a height specified (in pixels or percent, even if the percent is ignored) OR
-- if the parent table height is specified in pixels OR 
-- if the parent table height is specified in percent (and this percentage is not ignored as auto due to the auto-sized parent)

I think the right behavior would be the following one:
- A cell is considered non-auto-sized 
-- if it or any of its table ancestor has a height specified in pixels or percent (if that percent value is not ignored due to the table/table-parent being auto-sized)

I have no idea whether this right behavior is web-compatible, maybe we should add a use counter to see if the case where the height is specified on row and a child of a cell is percent-sized is common. I don't expect so because it doesn't work, but the web is old...

Comment 3 by robho...@gmail.com, Dec 5 2016

Wow, thanks for the detailed feedback Francois.

Rather than try UseCounters I think we can just plump for the right behaviour and then move towards it. 

eae - what do you think?

Comment 4 by tkent@chromium.org, Dec 9 2016

Components: Blink>Layout
Project Member

Comment 5 by bugdroid1@chromium.org, Dec 14 2016

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

commit 6c6315ec5c5ca31be742ebbe13cfecb43baae21a
Author: robhogan <robhogan@gmail.com>
Date: Wed Dec 14 22:30:42 2016

Percent height border-box content should get correct height in percent height cells

"For the purpose of calculating [the minimum height of a row],
descendants of table cells whose height depends on percentages
of their parent cell's height are considered to have an auto
height if they have overflow set to visible or hidden or if
they are replaced elements, and a 0px height if they have not."

This CL does two things re this rule:

- It ensures we respect it for replaced elements, including ones that aren't
  LayoutReplaced or isReplaced() objects. This is covered by the table-percent-
  height-* tests.
- It ensures we always obey it for hidden/visible overflow elements.

The CL also does two other things:

- If the cell doesn't have a specified height then treat it as auto for the
  purposes of calculating percent heights of its children. See bug
  671010 for the discussion that leads to this approach - soon to be specified
  we hope.
  Note that this results in the new behaviour of the form control elements in
  table-percent-height-* tests: they size as they would if they were in auto-
  sized <div>. Again, see  bug 671010 .
  We introduce a 'regression' in the behaviour of radio/select elements when
  percent-sized inside a cell that has no specified height - they now behave
  the same as radio/select elements when inside a div with auto height, they
  get a zero height. This is covered specifically in input-radio-height-inside-auto-container.html.
  It can also be seen in the updates to table-percent-height.html.

- If we've computed the height of a cell's child using the height made available
  by the cell, then be sure to respect content-/border-sizing of the child. We
  were just assuming that children were always content-sized. These are covered
  by the percent-height-border-box-content-* tests and are the main fall-out
  in 669867.

BUG= 669867 ,  671010 

Review-Url: https://codereview.chromium.org/2535173006
Cr-Commit-Position: refs/heads/master@{#438652}

[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/replaced/input-radio-height-inside-auto-container-expected.html
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/replaced/input-radio-height-inside-auto-container.html
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/replaced/table-percent-height-text-controls.html
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/replaced/table-percent-height.html
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-2-expected.txt
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-2.html
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-3-expected.txt
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-3.html
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-expected.html
[add] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell.html
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/003-expected.png
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/003-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-2-expected.png
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-2-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-4-expected.png
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-4-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/tables/mozilla/bugs/bug30692-expected.png
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/linux/tables/mozilla/bugs/bug30692-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/win/fast/replaced/table-percent-height-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/LayoutTests/platform/win/fast/replaced/table-percent-height-text-controls-expected.txt
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/6c6315ec5c5ca31be742ebbe13cfecb43baae21a/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp

Project Member

Comment 6 by bugdroid1@chromium.org, Dec 22 2016

Labels: merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/58eb404753d8ee142e02b9695a84c2c6c24f88ea

commit 58eb404753d8ee142e02b9695a84c2c6c24f88ea
Author: Robert Hogan <robhogan@gmail.com>
Date: Thu Dec 22 13:36:51 2016

Percent height border-box content should get correct height in percent height cells

"For the purpose of calculating [the minimum height of a row],
descendants of table cells whose height depends on percentages
of their parent cell's height are considered to have an auto
height if they have overflow set to visible or hidden or if
they are replaced elements, and a 0px height if they have not."

This CL does two things re this rule:

- It ensures we respect it for replaced elements, including ones that aren't
  LayoutReplaced or isReplaced() objects. This is covered by the table-percent-
  height-* tests.
- It ensures we always obey it for hidden/visible overflow elements.

The CL also does two other things:

- If the cell doesn't have a specified height then treat it as auto for the
  purposes of calculating percent heights of its children. See bug
  671010 for the discussion that leads to this approach - soon to be specified
  we hope.
  Note that this results in the new behaviour of the form control elements in
  table-percent-height-* tests: they size as they would if they were in auto-
  sized <div>. Again, see  bug 671010 .
  We introduce a 'regression' in the behaviour of radio/select elements when
  percent-sized inside a cell that has no specified height - they now behave
  the same as radio/select elements when inside a div with auto height, they
  get a zero height. This is covered specifically in input-radio-height-inside-auto-container.html.
  It can also be seen in the updates to table-percent-height.html.

- If we've computed the height of a cell's child using the height made available
  by the cell, then be sure to respect content-/border-sizing of the child. We
  were just assuming that children were always content-sized. These are covered
  by the percent-height-border-box-content-* tests and are the main fall-out
  in 669867.

BUG= 669867 ,  671010 

Review-Url: https://codereview.chromium.org/2535173006
Cr-Commit-Position: refs/heads/master@{#438652}
(cherry picked from commit 6c6315ec5c5ca31be742ebbe13cfecb43baae21a)

Review-Url: https://codereview.chromium.org/2599043002 .
Cr-Commit-Position: refs/branch-heads/2924@{#596}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/TestExpectations
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/replaced/input-radio-height-inside-auto-container-expected.html
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/replaced/input-radio-height-inside-auto-container.html
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/replaced/table-percent-height-text-controls.html
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/replaced/table-percent-height.html
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-2-expected.txt
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-2.html
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-3-expected.txt
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-3.html
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell-expected.html
[add] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/fast/table/percent-height-border-box-content-in-cell.html
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/fast/table/003-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-2-expected.png
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-2-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-4-expected.png
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/fast/table/split-table-section-before-anonymous-block-4-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/linux/tables/mozilla/bugs/bug30692-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/win/fast/replaced/table-percent-height-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/LayoutTests/platform/win/fast/replaced/table-percent-height-text-controls-expected.txt
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/Source/core/layout/LayoutBox.cpp
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/Source/core/layout/LayoutBox.h
[modify] https://crrev.com/58eb404753d8ee142e02b9695a84c2c6c24f88ea/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp

FYI, I just updated the spec to clarify the behavior. What do you think of the text?

<<
Once the final size of the table and the rows has been determined, the content of the table-cells must also go through a second layout pass, where, if appropriate, percentage-based heights are this time resolved against their parent cell used height.

It is appropriate to resolve percentage heights on direct children of table cells if the cell is considered to have its height specified explicitly or the element is absolutely positioned, see <a href="http://www.w3.org/TR/CSS2/visudet.html#the-height-property">CSS 2</a>.

It is further clarified that a cell is considered to have its height specified explicitly if the computed height of the cell or any of its table ancestors is a length or percentage. 
>>
I think so. Would you mind adding some text to your jsfiddle cases in the spec and here that state the expected behaviour? I *think* we're passing them on Chrome but it's easy to miss a trick!
Status: Fixed (was: Assigned)

Sign in to add a comment