[LayoutNG] Incorrect cell heights |
|||
Issue descriptionMay need more categorizations, but following test failures are about incorrect cell heights. # Incorrect cell height: not quirks. fast/css-intrinsic-dimensions/height-positioned.html # Incorrect cell height: can WontFix if quirks only? fast/block/basic/quirk-percent-height-table-cell.html tables/mozilla/bugs/ bug101674 .html tables/mozilla/bugs/ bug131020 -2.html tables/mozilla/bugs/ bug137388 -3.html tables/mozilla/bugs/bug149275-1.html tables/mozilla/bugs/bug149275-2.html tables/mozilla/bugs/ bug23235 .html tables/mozilla/bugs/ bug50695 -2.html tables/mozilla/bugs/ bug55527 .html Maybe this one is different cause? From David's comment: > tables/mozilla/bugs/ bug137388 -3.html > Looks like we're too eager to apply the %height-of-indefinite-height-is-auto rule for this iframe
,
Aug 1
IIUC you said you'll look into them, but if any of these turned out to be non-tables, or you're too busy, let's discuss how to get them down.
,
Aug 22
When these 4 are set to standards mode, ng matches legacy, so I suppose they are lower priority: tables/mozilla/bugs/ bug101674 .html tables/mozilla/bugs/ bug137388 -3.html tables/mozilla/bugs/ bug50695 -2.html tables/mozilla/bugs/ bug23235 .html (with NG having different kerning or word spacing) These have % height and only sizes are wrong: tables/mozilla/bugs/ bug131020 -2.html tables/mozilla/bugs/bug149275-1.html fast/css-intrinsic-dimensions/height-positioned.html These have % height and both size and position are wrong: fast/block/basic/quirk-percent-height-table-cell.html tables/mozilla/bugs/bug149275-2.html no % height, no clue where height difference is coming from, possibly text sizing? tables/mozilla/bugs/ bug55527 .html
,
Aug 22
Notes from tables/mozilla/bugs/bug149275-1.html FF matches NG, which makes td>div auto height so that the Line # entries are shown. Edge matches legacy, which gives td>div 0 height and Line # entries are NOT shown. Reducing test case for legacy (chrome stable): #dvFundos overflow: overflow:hidden -- NG behavior (auto height) overflow:visible -- NG behavior overflow:scroll -- legacy behavior (0 height) overflow:auto -- legacy behavior #dvFundos overflow-x: overflow-x:hidden -- legacy behavior (switched from above!) overflow-x:visible -- NG behavior overflow-x:scroll -- legacy behavior overflow-x:auto -- legacy behavior Reduction at http://jsfiddle.net/dgrogan/761mud58/1/ <table height='100%' border=1> <tr> <td> <div id=dvFundos style='height:100%;overflow-x:hidden;border: 5px dotted lightblue;'> some text </div> </td> </tr> </table> To make Edge/Legacy show the text, which matches NG and FF:<br> - Remove %height from table<br> - Change overflow-x:hidden to overflow-x:visible or removing it altogether<br> - Remove %height from #dvFundos
,
Aug 22
Notes from tables/mozilla/bugs/ bug131020 -2.html NG/Edge/FF agree on the dimensions of the blue box; legacy makes it too short. NG and FF paint the blue box in the same place, aligning its bottom border with that of the table. Legacy and Edge center it vertically. This is probably an improvement over legacy and should be rebased. http://jsfiddle.net/dgrogan/6wuj3cv8/2/
,
Aug 22
fast/css-intrinsic-dimensions/height-positioned.html Edge/FF look nothing like Legacy/NG. MDN reports they don't implement height:fill-available. caniuse agrees. Given that I don't have another browser to verify NG or legacy as more correct, and this isn't a table (but does look like one!), I'm going to punt.
,
Aug 22
bug149275-2.html Two things going on. (1) When resizing %height descendants of table cells, legacy apparently treated the resulting height as border box height, not content height. NG correctly does this as content height, to the extent that it should non-auto, which I am unclear on. Firefox treats the %height as auto, presumably because the parent has auto height. There's been on-and-off discussion of this issue over the last few years but I'm not caught up on where it stands now. (2) The content of the cell is not honoring vertical-align:middle in NG, but I do not know why. http://jsfiddle.net/dgrogan/kz0bq6rv/7/
,
Aug 23
bug55527 .html is due to a rounding issue with calculating the height of the cell using 1ex as top and bottom margins, but only when running layout tests. When running content_shell without --run-web-tests, the results are identical between legacy and NG.
,
Aug 23
To add to comment 8, when --run-web-tests is specified, the row height in NG changes, legacy is the same. row height is usually 36, but with --run-web-tests --enable-blink-features=LayoutNG, row height is 37.
,
Oct 13
,
Dec 11
It happens because of fractional block-side padding on table cells. See attachment. LayoutNG treats table cell padding as any other padding, i.e. with sub-pixel support. Legacy, on the other hand, rounds block-side padding down (but apparently not inline-side padding). Fix here: https://chromium-review.googlesource.com/c/chromium/src/+/1372632
,
Dec 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e865d9cff6864f415556dd8db58cd3f575f975d4 commit e865d9cff6864f415556dd8db58cd3f575f975d4 Author: Morten Stenshorne <mstensho@chromium.org> Date: Mon Dec 17 10:27:39 2018 [LayoutNG] Floor table cell padding on the block sides. This matches legacy layout, which generally doesn't support subpixels for table layout. Bug: 869869 Change-Id: Ib0d9c61cee27568388b3bc5fef81bf173ce8f77f Reviewed-on: https://chromium-review.googlesource.com/c/1372632 Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Cr-Commit-Position: refs/heads/master@{#617076} [modify] https://crrev.com/e865d9cff6864f415556dd8db58cd3f575f975d4/third_party/blink/renderer/core/layout/ng/ng_length_utils.cc [modify] https://crrev.com/e865d9cff6864f415556dd8db58cd3f575f975d4/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG |
|||
►
Sign in to add a comment |
|||
Comment 1 by kojii@chromium.org
, Aug 1