CSS table-cell will not inherit height properly in Chrome 63
Reported by
mcoz...@gmail.com,
Jan 2 2018
|
|||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/63.0.3239.84 Safari/537.36 Steps to reproduce the problem: 1. Make a div with display:table; width:100%; height:100% 2. Make two nested divs with display:table-cell; width:50%; height:100%; vertical-align:middle 3. Add paragraph text to first cell 4. Add block element/div to second cell, with height:100%; What is the expected behavior? The div in the second cell should grow with the text content of the first cell. What went wrong? The cell will not inherit the proper height of the table. A fixed height of the table is now required to make the block element fill the space. Image/div grows appropriately in Firefox and IE. Image used to grow appropriately in Chrome prior to V.63 Did this work before? Yes Chrome version: 63.0.3239.84 Channel: n/a OS Version: 6.1 (Windows 7, Windows Server 2008 R2) Flash Version: Code has not changed on sites afflicted with this problem so something about the old CSS is no longer valid in Chrome 63.
,
Jan 3 2018
mcozart@ Thanks for the issue. Able to reproduce this issue on Windows 10, Mac OS 10.12.6 and Ubuntu 14.04 on the latest Canary 65.0.3310.0 and Stable 63.0.3239.108 by following the steps mentioned in the original comment. Bisect Information: ==================== Good Build: 63.0.3236.0 Bad Build : 63.0.3238.0 On executing the per-revision bisect script, below is the changelog URL. Changelog URL: --------------- https://chromium.googlesource.com/chromium/src/+log/20893ab9a294053d7ff10a4a17cb148aeb51545a..65d9417940fca2d9edc013f02dbc5ba3b7cccc3c From the above Changelog URL, suspecting the below change for this issue. Reviewed-on: https://chromium-review.googlesource.com/700520 robhogan@ Can you please check if this issue is related to your change, else help us in assigning to the right owner. Adding ReleaseBlock-Stable as this is a recent Regression. Please feel to remove the same if this is not applicable. Thanks...
,
Jan 3 2018
Thank you, lucco. I am really trying to avoid a JS workaround as I have a template with this kind of layout across many, many domains that would need updating. Please let me know if I can provide any more information in an effort to get this resolved.
,
Jan 3 2018
,
Jan 3 2018
This stems from changes discussed in issue 687551 and issue 762330. Our intention was to match the spec (https://www.w3.org/TR/css-tables-3/#computing-the-table-height) and as far as I can tell from the test case supplied we are matching the spec. See https://bugs.chromium.org/p/chromium/issues/detail?id=762330#c9 and https://bugs.chromium.org/p/chromium/issues/detail?id=687551#c36. In a nutshell we are treating the table cell as though it is auto. So for example if you remove the 'height:100%' from the table cell you will see it has the same rendering. This is for the reasons discussed at the links above. That said, I would have expected our behaviour to match Edge here, since that is the other browser following the spec as closely as we are. I'm cc'ing the spec editor in the hope that he can confirm my understanding. (Sorry to drag you into yet another table cell height discussion Francois!) Maybe the difference has to do with browser-specific treatment of the image inside the cell. Can't rule out that I've misunderstood something either.
,
Jan 3 2018
Thanks Rob, Over to David to comment on our current behavior.
,
Jan 3 2018
Hi Rob, I'm sorry for it not being the cleanest example. I just want to make sure table-cells are not going to need to inherit fixed heights going forward. That defeats the purpose of using display:table and display:table-cell for equal height, responsive columns. Eager to hear more insight. Thanks.
,
Jan 4 2018
Not sure if anyone looked at Edge yet. Edge 41.16299.15.0 matches IE and Firefox. This might be obvious, but we can't change behavior that was interoperable with all the other browsers, even if our new behavior matches the current iteration of the spec. The spec will probably need more refinement for this case. (I learned this principle the hard way, trying to tackle the same bug in chrome 50.) If we don't hear really good news* from Francois within 24 hours we'll have to revert and merge to 64 beta. And it looks like the commit that caused this regression was fixing a regression from an earlier change, so we'll probably have to revert that too. * "Really good news": Edge is very soon going to match chrome's 63+ behavior.
,
Jan 4 2018
The Firefox screenshots I attached displayed the same for Edge, which was very surprising to me. Chrome is the only odd-ball not inheriting the height as expected. Thanks for keeping this moving!
,
Jan 7 2018
> 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 CSS 2.
>
> 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 the condition chain works fine here until you get to "table{height:100%}" which should "behave as auto" per https://drafts.csswg.org/css-sizing-3/#behave-auto but apparently that fact is ignored in all browsers but Chrome.
The spec text doesn't mention the "behave as auto" behavior explicitly and based on what I see, the interoperable behavior seems to be to ignore it for the table, but take it into account for the cell. It's a bit messy, and I agree it would require an additional line in the spec to clarify that point.
,
Jan 9 2018
Thanks for clearing that up Francois. I'll revert the change so that we match the other browsers.
,
Jan 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e7701a83830e5e68640af9dbb5932c73cf74ef3 commit 9e7701a83830e5e68640af9dbb5932c73cf74ef3 Author: Robert Hogan <robhogan@gmail.com> Date: Tue Jan 09 23:06:18 2018 Revert "Treat percent height cells with nothing to resolve against as auto" This reverts commit 65d9417940fca2d9edc013f02dbc5ba3b7cccc3c. Reason for revert: Per crbug.com/798478 the behaviour introduced with this patch was correct per the spec but turns out to be not interoperable. So revert for now and check interoperability again when spec is clarified. Original change's description: > Treat percent height cells with nothing to resolve against as auto > > tables/mozilla/bugs/ bug113424 .html covers the quirky behaviour, > in non-quirk mode we should continue to respect the rule that a > percent-height cell with nothing to resolve against is auto. > > Bug: 762330 > Change-Id: I8c7d97d23c75b7eb56a3d93637444d0ccada8d9b > Reviewed-on: https://chromium-review.googlesource.com/700520 > Commit-Queue: Emil A Eklund <eae@chromium.org> > Reviewed-by: Emil A Eklund <eae@chromium.org> > Cr-Commit-Position: refs/heads/master@{#507427} TBR=robhogan@gmail.com,eae@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 762330, 798478 Change-Id: I07e0b9cfd8f0884f316ee98b51f75fa22b360229 Reviewed-on: https://chromium-review.googlesource.com/858017 Commit-Queue: Robert Hogan <robhogan@gmail.com> Reviewed-by: Emil A Eklund <eae@chromium.org> Reviewed-by: Robert Hogan <robhogan@gmail.com> Cr-Commit-Position: refs/heads/master@{#528156} [modify] https://crrev.com/9e7701a83830e5e68640af9dbb5932c73cf74ef3/third_party/WebKit/LayoutTests/fast/table/percent-height-content-in-percent-height-cell-and-percent-height-table.html [modify] https://crrev.com/9e7701a83830e5e68640af9dbb5932c73cf74ef3/third_party/WebKit/Source/core/layout/LayoutTableSection.cpp
,
Jan 11 2018
Thanks for the revert. Is this something that needs to be targeted for M64? Since it's not as critical, my recommendation is to wait until M65.
,
Jan 16 2018
We're not planning any further M63 releases. Please reply to comment #14 for M64.
,
Jan 16 2018
,
Jan 16 2018
This bug requires manual review: We are only 6 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jan 17 2018
Will this be a safe revert? Why is this critical?
,
Jan 17 2018
We should not merge to beta given how close we are to the stable release. But that's just the lesser of two evils. We're in a rough spot here because the reverted CL (65d9417940fca2d9edc013f02dbc5ba3b7cccc3c) fixed a different regression. So the timeline of the page reported in issue 762330 will be: worked since antiquity regressed worked now regressed again in at least 65 The page reported here will have been broken in 63 and 64.
,
Jan 18 2018
Per comment#19, seems like we should not go ahead with the merge. Rejecting merge to M64. Let's target this for M65. If you think otherwise, please re-open with more clarifications.
,
Jan 22 2018
Moving to M65.
,
Jan 24 2018
I updated the spec based on your compat findings. https://github.com/w3c/csswg-drafts/commit/5b62f81c6ad732f1a0860414faa5950236682bd5
,
Jan 29 2018
Thanks Francois. |
|||||||||||
►
Sign in to add a comment |
|||||||||||
Comment 1 by lucco...@gmail.com
, Jan 3 2018Same problem here. For now I had to do a JS workaround so after the page finishes loading I set the height of the right TD to be the same as the table. $( document ).ready(function() { $('.rightTD').height($('.table').height()); }