Intermittent /css/css-flexbox/percentage-heights-004.html | Testing http://web-platform.test:8000/css/css-flexbox/percentage-heights-004.html == http://web-platform.test:8000/css/css-flexbox/percentage-heights-004-ref.html
Categories
(Core :: Layout: Flexbox, defect, P5)
Tracking
()
| Tracking | Status | |
|---|---|---|
| firefox75 | --- | fixed |
People
(Reporter: intermittent-bug-filer, Assigned: dholbert)
References
Details
(Keywords: intermittent-failure)
Attachments
(1 file)
Filed by: nerli [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=287436029&repo=mozilla-central
Full log: https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Q2AFexMEScuYOVRlibl5xA/runs/0/artifacts/public/logs/live_backing.log
Reftest URL: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://firefox-ci-tc.services.mozilla.com/api/queue/v1/task/Q2AFexMEScuYOVRlibl5xA/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1
[task 2020-02-04T11:59:56.010Z] 11:59:56 INFO - TEST-START | /css/css-flexbox/percentage-heights-004.html
[task 2020-02-04T11:59:56.019Z] 11:59:56 INFO - PID 10700 | 1580817596012 Marionette INFO Testing http://web-platform.test:8000/css/css-flexbox/percentage-heights-004.html == http://web-platform.test:8000/css/css-flexbox/percentage-heights-004-ref.html
[task 2020-02-04T11:59:56.265Z] 11:59:56 INFO - PID 10700 | 1580817596258 Marionette INFO No differences allowed
[task 2020-02-04T11:59:56.353Z] 11:59:56 INFO - TEST-UNEXPECTED-FAIL | /css/css-flexbox/percentage-heights-004.html | Testing http://web-platform.test:8000/css/css-flexbox/percentage-heights-004.html == http://web-platform.test:8000/css/css-flexbox/percentage-heights-004-ref.html
|
Assignee | |
Comment 1•6 years ago
•
|
||
Strange... this test "fails" 100% of the time for me locally on Linux, in Firefox release as well as Nightly. It also "fails" in Firefox Beta on Windows (viewed in BrowserStack).
I'm putting "fails" in quotes because I'm pretty suspicious of the test. In particular, I think its declaration about "You should not see red" doesn't take into consideration the possibility that scrollbars might take up space and also be transparent (which is the case for me in Firefox on Linux, as well as in Windows via BrowserStack at least). This means I see a red stripe at the bottom of each green div, but really that red stripe is just the scrollbar.
If I add this styling to the testcase & reference case to force an explicit transparent background for scrollbars...
#middle::-webkit-scrollbar, #middle2::-webkit-scrollbar {
background-color: transparent;
}
...then I see this same problem in Chrome, too. So I think this test is just incorrectly assuming that scrollbars are opaque by default.
Having said that: I have no idea why this is just getting reported as an intermittent failure now (and why it's intermittent), given that it fails reliably for me on multiple platforms and multiple versions...
But anyway, here's a minimal testcase, which paints an orange stripe (for the scrollbar background) below the text, for me:
data:text/html,<div style="overflow-x:scroll;float:left;background: orange"><span style="background:lime">teeeeeeeeeeeeeest
|
|
Assignee | |
Comment 2•6 years ago
|
||
As currently written (before this patch), the test improperly assumes that its
horizontal scrollbars will be opaque. Specifically: the testcase has a red
background behind two horizontal scrollbars, whereas the reference case has a
green background behind one of the scrollbars and no background behind the
other. This would all be fine if the scrollbars were guaranteed to be opaque,
but we don't have any such guarantee. On browsers where scrollbars are
transparent, this horizontal-scrollbar-area will end up red in the testcase
vs. green/white in the reference case.
This patch amends the files to address this, by adjusting the testcase like so:
- It changes the outer elements' background from red to tan, to avoid the
"something's definitely wrong" implication of red coloring. - It adjusts the prose to account for the fact that this color might shine
through a transparent scrollbar, and it's fine if it does.
...and similarly, this patch adjusts the reference case like so:
- It adds a tan background-color on both "outer" elements (instead of
green & default-transparent backgrounds). - It adds a green background to #inner, and gives that element 100% height to
make it fill the scrollable area with green (since #outer is no longer doing
that). - It reorders the declarations for #inner2 to match the ordering in the
testcase (just for consistency - this one's a non-functional change). - It makes the same adjustment to the prose as in the testcase.
Note that the testcase is mostly unchanged, so the same conditions are being
tested here.
Aside from the prose change, this patch doesn't affect either file's rendering
in Chrome, and it only affects the rendering in Firefox insomuch as it converts
the red scrollbar-background to tan.
|
||
Updated•6 years ago
|
|
|
Assignee | |
Comment 3•6 years ago
|
||
biesi, hope you don't mind that I'm tagging you for review on phabricator here - it looks like you're the test author, per https://github.com/web-platform-tests/wpt/commit/4d69922d6c6ee9ddef4c77f03fc7ed22983667c9#diff-ebbc303e1204dceb94e989ec50d8a38c
Let me know if you need any help with phabricator. I'm also open to submitting this review via github, but I'm more used to the phabricator workflow (and that'll still sync properly over to github) so I like to default to that for WPT changes. :)
| Comment hidden (Intermittent Failures Robot) |
The patch lgtm, and I see now that the description here answers the question I put in phabricator. Although I would probably reorder the testcase instead to put width before height.
|
|
Assignee | |
Comment 6•6 years ago
|
||
|
|
Assignee | |
Comment 7•6 years ago
|
||
(In reply to Christian :Biesinger (don't email me, ping me on IRC) from comment #5)
Although I would probably reorder the testcase instead to put width before height.
Sure - I've now changed the patch on phabricator so that it reorders in that direction now.
Thanks for the review!
|
||
Comment 11•6 years ago
|
||
| bugherder | ||
Description
•