New issue
Advanced search Search tips

Issue 869882 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner: ----
Closed: Jan 18
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task

Blocking:
issue 869867



Sign in to add a comment

[LayoutNG] Tables in vertical writing mode

Project Member Reported by kojii@chromium.org, Aug 1

Issue description

fast/table/height-percent-test-vertical.html
fast/table/table-display-types-vertical.html
fast/table/border-collapsing/004-vertical.html
html/details_summary/details-writing-mode-align-center.html
html/details_summary/details-writing-mode-align-left.html
html/details_summary/details-writing-mode-align-right.html
html/details_summary/details-writing-mode.html

Not sure if this is table or constraint space. Maybe vertical and orthogonal flows are separate issue? Let me start with single issue and we can split if needed.
 
Blocking: 869867
Owner: dgro...@chromium.org
Status: Started (was: Untriaged)
I reduced one of these down but may need help figuring out the best fix.
Let me know if I can help
Looks like these are table-cell + orthogonal too, maybe easier since they're focused tests.

 external/wpt/css/css-writing-modes/orthogonal-parent-shrink-to-fit-001q.html
 external/wpt/css/css-writing-modes/orthogonal-parent-shrink-to-fit-001r.html
 external/wpt/css/css-writing-modes/orthogonal-parent-shrink-to-fit-001s.html
 external/wpt/css/css-writing-modes/orthogonal-parent-shrink-to-fit-001t.html
Morten already filed a bug for the details_summary failures:  issue 855039 
Owner: ----
Status: Available (was: Started)
Summary: [LayoutNG] Tables in vertical writing mode (was: [LayoutNG] Tables + vertical / orthogonal flow failures)
Yeah, let's use  issue 855039  for cell children with orthogonal flow and this one for vertical tables.
notes to self

table-display-types-vertical.html is quirks and contains tables and table cells that establish vertical flows, but not at the same time

height-percent-test-vertical is quirks and contains tables that take part in vertical flow

fast/table/border-collapsing/004-vertical.html is quirks and contains tables that takes part in vertical flow

table-display-types-vertical.html only the <p display:table-cell> is wrong. None of legacy, NG, Edge or FF agree but NG is clearly broken and prob won't take too much to fix.
Owner: dgro...@chromium.org
Status: Started (was: Available)
height-percent-test-vertical.html

NG displays something different when window is resized?? Something to do with the body being overflow:hidden, and possibly the initial scroll position being different between legacy and NG. But the initial rendering of NG doesn't appear anywhere when scrolling is enabled.

In standards mode, NG matches legacy and FF-quirks and obviates the scrolling issue because the contents don't overflow the body.

My initial thought is WONTFIX except for the scrolling issue which may not be tables-specific.
fast/table/border-collapsing/004-vertical.html

also related to <body overflow:hidden> b/c removing that causes different scrollbars for NG v legacy. Removing quirks doesn't change outcome
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 11

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

commit 8a8f37334579c02154f2cbd1bb128bb53235d304
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Oct 11 20:24:42 2018

Fix FontOrientation when StyleAdjuster changes WritingMode

This patch fixes FontOrientation in ComputedStyle when
StyleAdjuster changes WritingMode.

FontOrientation is computed from WritingMode and
TextOrientation when constructing style through FontBuilder,
but it can be out-of-sync when WritingMode is changed without
using FontBuilder.

This patch re-synchronize FontOrientation in such cases.

Bug:  869882 
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Change-Id: Ie1eedcec9c96e381fc508871a8b714ab0f086081
Reviewed-on: https://chromium-review.googlesource.com/c/1275986
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#598913}
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/css/resolver/font_builder.cc
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/css/resolver/style_adjuster.cc
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/dom/document.cc
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/layout/layout_object.cc
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/style/computed_style.cc
[modify] https://crrev.com/8a8f37334579c02154f2cbd1bb128bb53235d304/third_party/blink/renderer/core/style/computed_style.h

Owner: ----
Status: Available (was: Started)
I retract the WONTFIX in comment 9; this issue also affects flexbox as shown in http://davidsgrogan.github.io/quirks_percent_ortho_ng_boundary.html. And there was no scrolling issue.
Project Member

Comment 13 by bugdroid1@chromium.org, Nov 26

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

commit 9aa0ccdf1e9f7de0e629ba4e0ec5e09853d815b4
Author: Morten Stenshorne <mstensho@chromium.org>
Date: Mon Nov 26 19:10:23 2018

Overridden containing block size trumps writing mode.

If someone has bothered to set a containing block size override, that
should be chosen in favor of special-code for orthogonal writing modes,
that reads out data from some ancestor layout object. LayoutNG sets an
override when entering legacy layout roots, since we shouldn't be
allowed to read out data from ancestor objects during layout when using
LayoutNG. We also need to take care when the child establishes an
orthogonal writing mode, in order to pick the correct dimension.

Bug:  869882 
Change-Id: I63997ebc10b77fba37527ed7a01ae406854d8f96
Cq-Include-Trybots: luci.chromium.try:linux_layout_tests_layout_ng
Reviewed-on: https://chromium-review.googlesource.com/c/1348055
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Koji Ishii <kojii@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610905}
[modify] https://crrev.com/9aa0ccdf1e9f7de0e629ba4e0ec5e09853d815b4/third_party/blink/renderer/core/layout/layout_box.cc
[modify] https://crrev.com/9aa0ccdf1e9f7de0e629ba4e0ec5e09853d815b4/third_party/blink/web_tests/FlagExpectations/enable-blink-features=LayoutNG

Comment 14 by mstensho@chromium.org, Jan 18 (4 days ago)

Status: Fixed (was: Available)
All of the listed tests have disappeared from the NG expectations file.

Sign in to add a comment