1560420 - WPT test contain-size-flexbox-002.html / contain-size-scrollbars-002.html fail in browser with overlay scrollbars (e.g. both Firefox & Chrome on Android)

Status: NEW

(Core :: Layout: Flexbox, defect, P3)

Description

[Daniel Holbert [:dholbert]]

Per bug 1560268, the WPT test contain-size-flexbox-002.html reliably fails on Android.

Reftest-analyzer link:
https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/es9VvBJ4SCOdpkmHLvsamA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

The testcase has two zero-width boxes, vertically stacked, with no space between them (which ends up looking like "|"), and the reference case looks the same except it's got space between the two boxes (so it looks like ":").

On Desktop, things look like the testcase (two boxes snugly stacked), except the boxes have some width, which I think come from the scrollbar sizes (since the files have overflow:scroll content). On Android, this content looks very different due to having overlay scrollbars which don't occupy any space - hence the zero-width scrollbars there.

I'm filing this bug to look into the test failure and fix the impl or test (whichever is broken).

Comment 1

[Daniel Holbert [:dholbert]]

Live test/reference links:
https://hg.mozilla.org/mozilla-central/raw-file/tip/testing/web-platform/tests/css/css-contain/contain-size-flexbox-002.html
https://hg.mozilla.org/mozilla-central/raw-file/tip/testing/web-platform/tests/css/css-contain/reference/contain-size-flexbox-002-ref.html

I can make Firefox and Chrome both "fail" the reftest if I swap out overflow:scroll in favor of overflow:hidden in both of those ^^ files. And the failure looks just like the failure on Android ("|" vs ":", roughly).

So, it seems the test is incorrectly (and inadvertently) assuming that overflow:scroll will produce scrollbars that occupy a nonzero amount of space -- and on platforms where scrollbars do not occupy space, the test fails due to this alignment issue.

Biesi, you wrote this test -- perhaps you could take a look? If you don't get to it, I might take a crack at a fix and see if you'd review. :)

Comment 2

[Daniel Holbert [:dholbert]]

Attachment: modified copy of WPT, for demonstration ("fails" in Firefox and Chrome on Desktop)

Here's a copy of the testcase where I've:

• replaced "overflow:scroll" with "overflow:hidden" to emulate the Android scrollbars-don't-take-up-space behavior
• added borders around the outer div, for visualization purposes.

This shows the issue -- the outer flex container here is much shorter than the outer block container, because the flex container shrinkwraps the tiny scrollable child, whereas the block container won't get shorter than the height of a line.

Comment 3

[Daniel Holbert [:dholbert]]

So, bottom-line: the testcase is assuming that its first div is at least 1 line-height tall (because in the reference case, the corresponding div is display:block and contains inline content, and so will be at least 1 line-height tall).

And on Android (and in the attached modified testcase), this assumption doesn't hold up -- the testcase's first div is much shorter than that, because there are no scrollbars, and it's a flex container and hence is happy to shrinkwrap the smaller height of its inner div. So, the first div has a different height in the testcase vs. reference case, which means the second outer div ends up being positioned at a different point.

One possible solution: just remove the second div altogether (it's kind of redundant -- it's a reference-that-is-included-as-part-of-the-testcase, as I understand it).

Alternative solution: don't stack these divs vertically; instead, float them horizontally, or put them in another outermost horizontal flex container. (I don't love this solution, because it seems like it's adding complexity.)

biesi, what do you think? Would you be OK with removing the second div here? (and I guess the textual description, which necessarily assumes the presence of a reference-div-inside-of-the-testcase)

Comment 4

[Daniel Holbert [:dholbert]]

Third possible solution: in the reference case, make the first inner div be display:block rather than inline-block, so that its outer div (also a block) can shrinkwrap its height (as it does in the testcase) rather than being clamped to no-shorter-than-a-line-height.

I don't know offhand if that avoids testing what the testcase was aiming to test, though.

Comment 5

[Daniel Holbert [:dholbert]]

BTW, the existing version of this test fails in Chrome Beta 76 on Android right now (i.e. the links in comment 1 don't match - the first looks like "|" and the second looks like ":") It also fails in Firefox and Chrome responsive-design-mode (emulating a phone viewport).

Comment 6

[Emilio Cobos Álvarez [:emilio]]

There's a similar test (contain-size-scrollbars-002.html), which suffers from the same. Some quote from the Matrix chat:

(notably, this contain-size-scrollbars-002.html test fails in the same way on Chrome as it does on patched-Firefox-for-android, if I use overflow:hidden on it instead of overflow:scroll)
so the test definitely has buggy expectations, and our rendering is consistent with how Chrome handles an approximately-similar scenario
if you use the same bug number (I think you were going to), please add a comment to the pointed-at bug, and perhaps adjust the summary as well.
(to mention the name of the other test at least, and any other context you feel like including)

Comment 7

[Daniel Holbert [:dholbert]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #6)

There's a similar test (contain-size-scrollbars-002.html), which suffers from the same

FWIW, I see we've got that one as "pass or fail" on all platforms as of bug 1365806 / https://hg.mozilla.org/mozilla-central/rev/012f25fede5c#l4.6 ), though it looks like wpt.fyi shows us as passing that one. (Maybe we're just lucky enough to be running under the right circumstances on the wpt.fyi test environment, though?)

Also: wpt.fyi shows us as failing the original testcase that I mentioned here (contain-size-flexbox-002.html) but I think it's for a different reason than what's described in this bug (which was android-specific, i.e. overlay-scrollbar-specific); I'll spin that off as a new bug.

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit BugBot documentation.