2016963 - Grid reftest failures after bug 2013408 with layout.abspos.fragmentainer-aware-positioning.enabled=true

Status: RESOLVED FIXED

(Core :: Layout: Positioned, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

See this try run on current Nightly with pref on. We fail the following grid reftests:

• layout/reftests/css-grid/grid-fragmentation-002.html
• layout/reftests/css-grid/grid-fragmentation-dyn1-002.html
• layout/reftests/css-grid/grid-fragmentation-dyn3-002.html

Our rendering of these grid tests differs from Google Chrome's. However, we should investigate whether they expose a real bug, or whether our current rendering is correct and we simply need to update the reference files to reflect that.

This should block enabling the pref in bug 2002115.

Comment 1

[BugBot [:suhaib / :marco]]

Set release status flags based on info from the regressing bug 2013408

:dshin, since you are the author of the regressor, bug 2013408, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

layout.abspos.fragmentainer-aware-positioning.enabled is not yet enabled.

Comment 3

[David Shin[:dshin]]

Attachment: Reftest output to be used with Reftest Analyzer

Reftest output attached for pref on vs. off, for layout/reftests/css-grid/grid-fragmentation-002.html. Main differences occur for the second column container, as well as the third last column container. I believe that as far as abspos fragmentation goes, we're more correct, i.e. Lime abspos element should not be only showing up on the first part, which is what the reference expects.

There are other issues with this test. In fact, I don't think the grid is being fragmented correctly - Grid in the second column container should be across 3 columns, because height-wise, we have: 10px top padding + 5px top border + 50px row + 50px row + 12px gap + 5px bottom border = 132px, which requires 3 64px tall columns to cover, but we are somehow managing to put everything within 2 columns.

Third last column container is strange as well - the grid container overflows the column container.

I could be missing something about the grid fragmentation behaviour here?

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Reduced testcase

This reduced testcase is from the third to last column container in layout/reftests/css-grid/grid-fragmentation-002.html

With Bug 2013408, the absolutely positioned element under the grid container correctly computes its unfragmented size. However, the current rendering is the result of two bugs:

1. We are currently unable to fragment tall grid rows (bug 1852515). Therefore, the first and second grid rows end up in column 1 and column 2, respectively, even if they are taller than the fragmentainer (column). In the reduced testcase, I added two grid items to make this apparent.
2. For absolutely positioned elements, we currently give them the available block-size up to the fragmentainer's end edge (the column end), not its absolutely containing block's padding end edge (bug 2010621). So in the reduced testcase, the green abspos element just laid out to the block-end edge of the column 1 and column 2, not to the block-end edge of the grid container.

TLDR: we are a step closer to the correct result, so I'll post a patch to adjust the reference file.

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-7)]

Attachment: Bug 2016963 - Adjust grid-fragmentation-002-ref.html to match the rendering with fragmentainer-aware positioning. r?#layout

See bug 2016963 comment 4 for a detailed analysis.

The absolute positioned element uses grid's padding edge as the containing
block [1].

The total height of the grid area (padding-edge) in the adjusted sub-tests is
112px (two 50px grid rows plus the gap 12px). The <span> has 1px border, and
that leaves 110px for the its content-box height.

[1] See the NOTE in https://drafts.csswg.org/css-grid-2/#abspos-items

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/668b228d3b57
https://hg.mozilla.org/integration/autoland/rev/82ca116c1730
Adjust grid-fragmentation-002-ref.html to match the rendering with fragmentainer-aware positioning. r=layout-reviewers,hiro

Comment 7

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/82ca116c1730