816357 - tweak flexbox reftest manifest to use new "default-preferences" feature

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

heycam recently added a "default-preferences" statement that can go in a reftest manifest, to specify a pref to be toggled for the remaining tests in that directory.

The flexbox reftest manifest
  https://mxr.mozilla.org/mozilla-central/source/layout/reftests/flexbox/reftest.list
would benefit from that feature. Filing this bug on taking advantage of it there.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 2

[Cameron McCormack (:heycam)]

Comment on attachment 686376 [details] [diff] [review]
fix

Review of attachment 686376 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, just one nit.  r=me with that.

::: layout/reftests/flexbox/reftest.list
@@ +6,5 @@
>  
> +# Enable pref for remaining tests
> +# (Most tests only need it in the testcase, but a few use it in the
> +# reference case, so we'll just enable it using "pref()" to make
> +# it available for both.

Needs a closing paren.

Comment 3

[Daniel Holbert [:dholbert]]

Thanks! Fixed and pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/82837149f001

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82837149f001

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/82837149f001

Comment 6

[Ed Morley [:emorley]]

Sorry, backed this out on suspicion of causing intermittent Linux flexbox reftest failures, eg:

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-basic-textarea-vert-2.xhtml | image comparison (==), max difference: 1, number of differing pixels: 2

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/test/build/reftest/tests/layout/reftests/flexbox/flexbox-basic-fieldset-vert-2.xhtml | image comparison (==), max difference: 1, number of differing pixels: 1

https://tbpl.mozilla.org/php/getParsedLog.php?id=17450843&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445650&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445548&tree=Mozilla-Inbound

More retriggers on way:
https://tbpl.mozilla.org/?tree=Mozilla-Inbound&jobname=Fedora%2012%20&tochange=b86fba8cc592&fromchange=602add2d02e6
...but I'm pretty sure this bug is the cause.

Backout:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5369e76e0ffb

Comment 7

[Daniel Holbert [:dholbert]]

This bug's patch just controls how the flexbox pref is enabled for flexbox reftests. If it broke stuff, it'd be in a "flexbox not enabled correctly --> test renders completely wrong" sort of mode, not in a "max difference: 1, number of differing pixels: 1" kind of way...

So I don't _think_ thinks could've caused comment 6, and it's possible that whatever did cause it is still checked-in... (Still, it's strange that this is specifically failing in flexbox tests.)

FWIW, the test-failure looks like a 1-2px difference just inside the lower-left corner of the border, with a color-channel difference of "1".  If this ends up being mysterious, we could just mark this with a sufficiently-specific "fuzzy-if" annotation.

Comment 8

[Ed Morley [:emorley]]

Also:
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445650&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17445548&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17450843&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17456229&tree=Mozilla-Inbound
https://tbpl.mozilla.org/php/getParsedLog.php?id=17452194&tree=Mozilla-Inbound

Point taken about 1-2px difference making it unlikely, but range is now narrowing as retriggers come back. (And if not this bug, will be useful to have the above logs in one place to transpose).

Comment 9

[Ed Morley [:emorley]]

https://tbpl.mozilla.org/php/getParsedLog.php?id=17458856&tree=Mozilla-Inbound

Comment 10

[Daniel Holbert [:dholbert]]

Mysterious as it is, this bug's change did seem to be responsible for comment 6.  The test-failures stopped after it was backed out, and two Try pushes provided additional evidence:

Try push with this bug's patch *applied*:
  https://tbpl.mozilla.org/?tree=Try&rev=1ebebced152b
That had 7 instances of comment 6's test-failure, out of 40+ reftest runs.

Try push with this bug's patch *backed out*:
  https://tbpl.mozilla.org/?tree=Try&rev=16de4a994590
That had no oranges at all, out of 40+ reftest runs.

Comment 11

[Daniel Holbert [:dholbert]]

Sorry, didn't mean to re-resolve this bug... not sure how that happened. I blame bugzilla & session-restore resurrecting this tab in an odd state.

Comment 12

[Cameron McCormack (:heycam)]

That's super strange.  I can't think how the use of default-preferences could cause random orange like this.

Comment 13

[Daniel Holbert [:dholbert]]

In each case, the difference is 1 or 2 pixels along the diagonal between the left-border and bottom border of a textarea or a fieldset.  These borders are "sunken"/beveled, and in some cases, there's a little bit of purple (the background color) showing through on these pixels, in both the testcase & the reference case, e.g. in this log from comment 8:
 https://tbpl.mozilla.org/php/getParsedLog.php?id=17450843&tree=Mozilla-Inbound

This seems like an issue with drawing these beveled borders reliably, and I think we could work around it by just specifying e.g. 'border: 1px solid gray" on these text areas & fieldsets.  

(We could also use fuzzy-if, but I'd rather avoid that if there's an easy test-fix, which I think there is in this case.)

Comment 14

[Daniel Holbert [:dholbert]]

I re-landed this with a helper-patch that just adds a simple 1px border to fieldsets & textareas in flexbox reftests, as I suggested in comment 13.

helper patch:       https://hg.mozilla.org/integration/mozilla-inbound/rev/95e6c6b6e939
default-pref patch: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5f98e718546

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/95e6c6b6e939
https://hg.mozilla.org/mozilla-central/rev/c5f98e718546