New issue
Advanced search Search tips

Issue 869873 link

Starred by 1 user
Status: Fixed
Owner:
dgro...@chromium.org
Closed: Oct 12
Cc:
mstensho@chromium.org
kojii@chromium.org
dgro...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 869867



Sign in to add a comment

[LayoutNG] Tables has spaces before/after for unknown reasons

Project Member Reported by kojii@chromium.org, Aug 1 
From our discussions:

fast/table/dynamic-descendant-percentage-height.html

The tables and parts have the same sizes in legacy vs NG. But it's like they're painted out of place. Using devtools and selecting <table> in the Elements tab shows an overlay offset from the location on screen.

tables/mozilla/bugs/ bug137388 -2.html

The cell in the top table is getting a wrong height for some reason
 
tables/mozilla/bugs/ bug14159 -1.html

The tables heights in legacy are taller. NG looks more correct. I don't know why legacy is getting a huge cell height and filling it with whitespace. (Both are very different from FF because of an issue Morten and I discussed a few weeks ago -- https://crbug.com/860084)

Comment 1 by kojii@chromium.org, Aug 1

Blocking: 869867
Cc: mstensho@chromium.org
Owner: dgagnon@chromium.org
Status: Assigned (was: Available)
As you commented, fast/borders/border-image-border-radius.html is done.

fast/table/dynamic-descendant-percentage-height.html is really strange, Gecko has the same space, but Edge does not. I can't really explain why the space between the tables exist.

Again, allow me to assign to you, but if this should split further, need someone else's help, or any, happy to discuss further.

Comment 2 by dgro...@chromium.org, Aug 1

Owner: dgro...@chromium.org

Comment 3 by mstensho@chromium.org, Aug 2 

Note that the test doesn't have to be dynamic, though. You can just put the child directly in the markup, and it will still fail.

Comment 4 by dgro...@chromium.org, Aug 28 

Ian has fast/table/dynamic-descendant-percentage-height.html passing in his WIP patch https://chromium-review.googlesource.com/c/chromium/src/+/1183199

Comment 5 by dgro...@chromium.org, Aug 29 

tables/mozilla/bugs/ bug14159 -1.html renders the same in NG standards, legacy standards, and legacy quirks. Only NG quirks renders differently. The space is actually in the cell, not between the tables. NG quirks puts the empty P block outside the cell for some reason.
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 31 

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

commit 156b7d30031eefc5fd91d0889c9bd7c6447de5d7
Author: David Grogan <dgrogan@chromium.org>
Date: Fri Aug 31 01:15:54 2018

[LayoutNG] NG renders this quirks test better than legacy

Change baselines to NG results. Legacy renders this test identically in
quirks mode and standards mode. FF and NG change the rendering between
the modes but match each other in both.

I didn't look into why the test is supposed to change in quirks vs
standards. NG and Firefox draw the empty P block below the cell in
quirks mode.

Bug:  869873 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ice54cce0fed3d8c260297ac0b8c9f5a8099086c4
Reviewed-on: https://chromium-review.googlesource.com/1196033
Commit-Queue: David Grogan <dgrogan@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587895}
[modify] https://crrev.com/156b7d30031eefc5fd91d0889c9bd7c6447de5d7/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/156b7d30031eefc5fd91d0889c9bd7c6447de5d7/third_party/WebKit/LayoutTests/TestExpectations
[modify] https://crrev.com/156b7d30031eefc5fd91d0889c9bd7c6447de5d7/third_party/WebKit/LayoutTests/platform/linux/tables/mozilla/bugs/bug14159-1-expected.png
[modify] https://crrev.com/156b7d30031eefc5fd91d0889c9bd7c6447de5d7/third_party/WebKit/LayoutTests/platform/mac/tables/mozilla/bugs/bug14159-1-expected.png
[modify] https://crrev.com/156b7d30031eefc5fd91d0889c9bd7c6447de5d7/third_party/WebKit/LayoutTests/platform/win/tables/mozilla/bugs/bug14159-1-expected.png

Comment 7 by dgro...@chromium.org, Oct 12

Status: Fixed (was: Assigned)
Turns out we fixed these tests
►

Sign in to add a comment