Closed Bug 559125 Opened 11 years ago Closed 11 years ago

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

Categories

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

x86
Linux
defect

Tracking

()

RESOLVED FIXED
mozilla1.9.3a5

People

(Reporter: philor, Assigned: dbaron)

References

Details

(Keywords: intermittent-failure)

Attachments

(2 files)

Attached file 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.
Whiteboard: [orange]
No longer blocks: 558910
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?
I have fedora 12 right here.  I'll see if I can reproduce.
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.
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.
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?
The failure threshold on my machine is between 11px and 12px, so I think 16px should be safe.
Attachment #443556 - Flags: review?(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.
Attachment #443556 - Flags: review?(bzbarsky) → review+
16px looks like it's cutting it close in that regard; I'll switch to 14px to split the difference.
http://hg.mozilla.org/mozilla-central/rev/ad8c76b91c33
Assignee: nobody → dbaron
Status: NEW → RESOLVED
Closed: 11 years ago
Priority: -- → P3
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3a5
Blocks: 438871
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.