559125 - Rev3 Fedora permaorange: reftests/bugs/180085-1.html and reftests/bugs/359903-1.html

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect, P3)

Description

[Phil Ringnalda (:philor)]

Attachment: reftest log

RelEng is switching from running reftests on the old CentOS refplatform to the
Talos Rev3 Fedora 12 refplatform. One of the failures blocking the switch is
that reftests/text-decoration/underline-block-propagation-2-standards.html
consistently fails, e.g., 

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1271106747.1271107786.6273.gz
Rev3 Fedora 12 mozilla-central opt test reftest on 2010/04/12 14:12:27
s: talos-r3-fed-004

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central-fedora-opt-u-reftest/build/reftest/tests/layout/reftests/bugs/180085-1.html

REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central-fedora-opt-u-reftest/build/reftest/tests/layout/reftests/bugs/359903-1.html

At least visually, these two appear to be the same problem: the button or buttons are wider in the test than in the reference.

Comment 1

[Armen [:armenzg]]

Hi Boris,
I am looking into fixing the remaining Fedora perma-oranges and the test 359903-1.html is one that it is still orange. Could you please have a look and see what is going on?

If I got you access to a Fedora machine would it help you find out?

Comment 2

[Boris Zbarsky [:bzbarsky]]

I have fedora 12 right here.  I'll see if I can reproduce.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

So these are tests that are testing that varying the amount of padding on a button doesn't change its appearance at all.  It ends up changing the width.

I could imagine that having an effect if GetMinimumWidgetSize is interacting incorrectly with padding.

It's also possible that these tests just fail at some font sizes.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I can reproduce the failures by adding
  input, button { font-size: 10px }
to the tests (180085).  I think that's about the size of the fonts in the images, too.  And I think that size comes out of system fonts.

Comment 5

[Boris Zbarsky [:bzbarsky]]

So what this test was expected to test is that if the width is insufficient to fit the text and the focuspadding and the padding and the border (given the box-sizing buttons use by default) then we effectively reduce the padding by scooting the text into it.  It tries to do this, as far as I can see, by setting a width small enough that we can fit the border+padding into that width, but can't actually fit the border+padding+focuspadding+"width of 'M'" so the 'M' ends up taking up part of the padding for display.  At least that's the only way I see this working with the way the reference is written without hitting the failure this bug is about.

It looks like what's going on here is that 1.5em at the relevant font-size is not big enough to fit the padding and border, even without the focuspadding or 'M'.  In other words, given border-box sizing, the only way to have a nonnegative content-box width is to end up with a border-box width of > 1.5em.  So the button does just that.

So to fix this we need to use a width that's big enough to fit the border+padding without being big enough to fit the text too.  Perhaps we can more or less guarantee this by using a width like 0.5em along with a much larger font size explicitly set in the test?

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: so why don't we just set the font size?

The failure threshold on my machine is between 11px and 12px, so I think 16px should be safe.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 443556 [details] [diff] [review]
so why don't we just set the font size?

Just setting the font-size large enough might mean that the test no longer tests what it wants to test, if 1.5em is enough to fit the border/padding/focuspadding/'M'.  Hence my 0.5em suggestion, which is presumably smaller than the width of an 'M'.

Then again, at 16px we're probably safe.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

16px looks like it's cutting it close in that regard; I'll switch to 14px to split the difference.

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/ad8c76b91c33

Comment 10

[Legacy TBPL/Treeherder Robot]

log: https://treeherder.mozilla.org/logviewer.html#?repo=fx-team&job_id=2003618
repository: fx-team
start_time: 2015-02-16T04:16:15
who: tomcat[at]mozilla[dot]com
machine: tst-linux32-spot-143
buildname: Ubuntu VM 12.04 fx-team pgo test reftest-no-accel
revision: f0048ba13213

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/180085-1.html | image comparison (==), max difference: 6, number of differing pixels: 405
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/forms/input/checkbox/gtk-theme-width-height.html | image comparison (==), max difference: 9, number of differing pixels: 49
REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/forms/input/radio/gtk-theme-width-height.html | image comparison (==), max difference: 9, number of differing pixels: 16

Comment 11

[Legacy TBPL/Treeherder Robot]

log: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=6673070
repository: mozilla-inbound
start_time: 2015-02-17T01:46:42
who: tomcat[at]mozilla[dot]com
machine: tst-linux32-spot-177
buildname: Ubuntu VM 12.04 mozilla-inbound opt test reftest-1
revision: 471c75dc6c56

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/180085-1.html | image comparison (==), max difference: 6, number of differing pixels: 405

Comment 12

[Legacy TBPL/Treeherder Robot]

log: https://treeherder.mozilla.org/logviewer.html#?repo=fx-team&job_id=2139206
repository: fx-team
start_time: 2015-03-03T03:47:37
who: tomcat[at]mozilla[dot]com
machine: tst-linux32-spot-080
buildname: Ubuntu VM 12.04 fx-team opt test reftest-1
revision: 5b8627537883

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/180085-1.html | image comparison (==), max difference: 6, number of differing pixels: 405

Comment 13

[Legacy TBPL/Treeherder Robot]

log: https://treeherder.mozilla.org/logviewer.html#?repo=mozilla-inbound&job_id=7402414
repository: mozilla-inbound
start_time: 2015-03-10T01:23:14
who: tomcat[at]mozilla[dot]com
machine: tst-linux32-spot-020
buildname: Ubuntu VM 12.04 mozilla-inbound opt test reftest-1
revision: 74a48fdaac01

REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/test/build/tests/reftest/tests/layout/reftests/bugs/180085-1.html | image comparison (==), max difference: 6, number of differing pixels: 405