Closed Bug 1626108 Opened 1 year ago Closed 1 month ago

New wpt failures from PR 22547 in /css/css-flexbox/negative-overflow.html

Categories

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

defect

Tracking

()

RESOLVED FIXED
89 Branch
Tracking Status
firefox89 --- fixed

People

(Reporter: mozilla.org, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [wpt], [wptsync upstream])

Attachments

(2 files)

The following tests have untriaged failures in the CI runs for wpt PR 22547:

Firefox-only failures

/css/css-flexbox/negative-overflow.html
.flexbox 10: FAIL
.flexbox 8: FAIL
.flexbox 9: FAIL

These updates will be on mozilla-central once bug 1626067 lands.

Note: this bug is for tracking fixing the issues and is not
owned by the wpt sync bot.

This bug is linked to the relevant tests by an annotation in
https://github.com/web-platform-tests/wpt-metadata. These annotations
can be edited using the wpt interop dashboard
https://jgraham.github.io/wptdash/

If this bug is split into multiple bugs, please also update the
annotations, otherwise we are unable to track which wpt issues are
already triaged. Resolving as duplicate or closing this issue should
be cause the bot to automatically update or remove the annotation.

Live version: https://wpt.live/css/css-flexbox/negative-overflow.html

For the first one (.flexbox 8: FAIL), the test has:

<div class="flexbox align-items-center vertical-padding" data-expected-scroll-height="90">
    <div></div>
</div>

...where the outer div has:

 overflow:auto;
 height:50px;
 padding-top:5px;
 padding-bottom: 10px;
 display:flex;
 align-items:center;

...and the inner div has height:100px.

This might have some relation to bug 748518 -- not sure. The 10px padding-bottom is suspect, given that we're off the test's expectation by 10px. Though I do see the scrollHeight changing after I make large adjustments to padding-bottom on the scrollable element here, so it's not as simple as padding-bottom being completely ignored (as I was initially guessing).

Anyway, triaging this as P3.

Priority: -- → P3
Summary: New wpt failures from PR 22547 → New wpt failures from PR 22547 in /css/css-flexbox/negative-overflow.html
Duplicate of this bug: 1626237
Duplicate of this bug: 1626561

Because this bug's Severity has not been changed from the default since it was filed, and it's Priority is P3 (Backlog,) indicating it has been triaged, the bug's Severity is being updated to S3 (normal.)

Severity: normal → S3

Nowadays, the only failures in this test are in "wrap-reverse" pieces, for .flexbox 5 and 11:

Fail .flexbox 5 assert_equals: <div class="flexbox wrap-reverse" data-expected-scroll-height="50"> <div></div> <div></div> </div> scrollHeight expected 50 but got 200

Fail .flexbox 11 assert_equals: <div class="flexbox wrap-reverse vertical-padding" data-expected-scroll-height="65"> <div></div> <div></div> </div> scrollHeight expected 65 but got 215

(Live link to test: https://wpt.live/css/css-flexbox/negative-overflow.html )

These parts of the test seem to expect that wrap-reverse does not reverse the scroll direction for the cross axis (even though the test expects that e.g. column-reverse does reverse the scroll direction for the main axis).

I think this is a bug in the test, not a bug in Firefox. (And a bug in the browsers that "pass" these subtests. I think the test was just inadvertently coded to match these browsers' behavior.)

TYLin already filed a Chrome bug on this: https://bugs.chromium.org/p/chromium/issues/detail?id=1114306

Note that this test is on the "compat2021" test set, so I'm marking this as blocking that metabug (though in this case we're failing due a bug in the test, not a bug in Firefox, as noted above).

Blocks: compat2021

I'll aim to fix this test's expectations tomorrow.

Flags: needinfo?(dholbert)
Attached file reduced testcase 1
Flags: needinfo?(dholbert)

The just-attached reduced testcase is from .flexbox 5. Firefox reports a scrollHeight of 200, which I believe is correct. Chrome reports a scrollHeight of 50. (which is what the WPT test currently incorrectly requires.)

This testcase wasn't accounting for the fact that "flex-wrap: wrap-reverse"
flips the direction in which overflow is reachable/scrollable, in a flex
container's cross axis (just as "flex-direction: column-reverse" does in the
main axis). The CSSWG resolved on this here:
https://lists.w3.org/Archives/Public/www-style/2014Sep/0187.html

This commit makes the test account for this, by increasing the expected
scrollHeight of the "wrap-reverse" pieces of the test (which have overflow that
should be scrollable).

Note that Chromium doesn't pass these amended subtests, because it doesn't
correctly handle scrolling with wrap-reverse; this is covered by
https://bugs.chromium.org/p/chromium/issues/detail?id=1114306 . (However, if I
modify the test to use "wrap" instead of "wrap-reverse", then Chromium does
pass these subtests. This indicates that Chromium does essentially agree on
the expected value here, aside from that bug report about making the overflow
scrollable.)

Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5574f8425595
Adjust test expectations in flexbox WPT negative-overflow.html, to indicate that negative cross-axis overflow in a "wrap-reverse" flexbox should be scrollable. r=TYLin
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/28389 for changes under testing/web-platform/tests
Whiteboard: [wpt] → [wpt], [wptsync upstream]
Status: NEW → RESOLVED
Closed: 1 month ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Upstream PR merged by moz-wptsync-bot
You need to log in before you can comment on or make changes to this bug.