779399 - Twitter Bootstrap buttons display unwanted horizontal line on hover.

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[cheesypoof]

Attachment: Example Screenshot

User Agent: Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0
Build ID: 20120731030541

Steps to reproduce:

http://twitter.github.com/bootstrap/base-css.html#forms

Trigger the hover state of one of the 'Save changes' or 'Cancel' buttons.


Actual results:

A horizontal line is displayed across the middle of the button.


Expected results:

There doesn't appear to be any CSS explanation for why the horizontal line is being generated.

Comment 1

[freakabcd]

I can confirm the presence of this whitish horizontal line on most buttons on that page when the mouse is hovered over them.

Mozilla/5.0 (X11; Linux x86_64; rv:17.0) Gecko/17.0 Firefox/17.0

Surprisingly, it does not appear on some of the buttons; but I can see a white horizontal line flashing through the button vertically; similar to a horizontal scanline going through the button. I suspect this has something to do with the way gradients are handled.

Comment 2

[Alice0775 White]

Regression window(m-c)
Good:
http://hg.mozilla.org/mozilla-central/rev/c0a32cf903d8
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120726034055
Bad:
http://hg.mozilla.org/mozilla-central/rev/20db7c6d82cc
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120726052803
Pushlog:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=c0a32cf903d8&tochange=20db7c6d82cc


Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/fe77957061ea
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120725022855
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/5e637d1f8ec6
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/17.0 Firefox/17.0 ID:20120725023655
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fe77957061ea&tochange=5e637d1f8ec6

Suspected: Bug 768775

Comment 3

[Alice0775 White]

Attachment: reduced html

Comment 4

[Joe Drew (not getting mail)]

Er, sorry William - typo!

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: reduced somewhat further

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The problem is that we snap the fillRect, but the gradient pattern is set up to exactly fill the tileRect, so if the fillRect and tileRect were initially the same the gradient might no longer exactly fill the fillRect. Then when we set repeat on the gradient, pixels at the edge might sample from a repetition of the gradient.

Really when we snap the fillRect, we should also snap the tileRect and ensure that the gradient exactly fills the snapped tileRect. I have a patch to do that that fixes this bug.

https://tbpl.mozilla.org/?tree=Try&rev=3c68c6dfd58f

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://tbpl.mozilla.org/?tree=Try&rev=3b3be5b3a7a5

Comment 8

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Comment 9

[Jeff Muizelaar [:jrmuizel]]

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

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

::: layout/base/nsCSSRendering.cpp
@@ +2137,5 @@
> +        ctx->IdentityMatrix();
> +        ctx->Rectangle(fillRectSnapped);
> +        ctx->Translate(tileRectSnappedTopLeft);
> +        ctx->Scale((tileRectSnappedBottomRight.x - tileRectSnappedTopLeft.x)/tileRect.width,
> +                   (tileRectSnappedBottomRight.y - tileRectSnappedTopLeft.y)/tileRect.height);

I'm not thrilled about adding more code that makes our drawing code dependent on scroll position, but I don't have a solid suggestion for how to solve this right now.

Comment 10

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c7e1b9aed9

Comment 11

[Ed Morley [:emorley]]

(In reply to Robert O'Callahan (:roc) (Mozilla Corporation) from comment #10)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c7e1b9aed9

Push backed out for failures in meter-native-style.html:
https://tbpl.mozilla.org/php/getParsedLog.php?id=14292529&tree=Mozilla-Inbound

Couldn't see you in #developers and didn't seem obvious which changeset caused it, so had to back all three out.

Given this failure and the bustage on the initial landing that required a fixup, please can you consider https://groups.google.com/d/topic/mozilla.dev.platform/nrbJE1ixkIs/discussion next time using inbound.

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to Ed Morley [:edmorley] from comment #11)
> Given this failure and the bustage on the initial landing that required a
> fixup, please can you consider
> https://groups.google.com/d/topic/mozilla.dev.platform/nrbJE1ixkIs/
> discussion next time using inbound.

Sorry. I did a tryserver push in comment #8 but failed to notice that Android debug does not run reftests :-(.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: Fix Android reftest failure by adding fuzz

The Android reftest failure seems to be because of dithering; the scale(-1,1) gradient in the reference has its dithering reflected horizontally compared to the test. I'm not sure why this isn't failing on trunk, but it certainly seems reasonable to allow for different dithering in this test, so I've done that.

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/461c9816a3be
https://hg.mozilla.org/integration/mozilla-inbound/rev/276b57e03eba

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

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

Bug 768775 is approved for Aurora, but I want to land this regression fix alongside it. This fix is pretty simple and safe.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 651262 [details] [diff] [review]
Fix Android reftest failure by adding fuzz

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

Bug 768775 is approved for Aurora, but I want to land this regression fix alongside it. This fix is pretty simple and safe.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/461c9816a3be
https://hg.mozilla.org/mozilla-central/rev/276b57e03eba

Comment 18

[Robert O'Callahan (:roc) (email my personal email if necessary)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/5b1b65bf7f4d
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b243972bf82

Comment 19

[Benoit Girard (:BenWa)]

Backout on mozilla-beta because of regression:

https://hg.mozilla.org/releases/mozilla-beta/rev/fc24961171a3

What do we want to do for other channels?

Comment 20

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Nothing. This patch can't be the real cause of bug 787947; at most, it revealed an existing graphics/layers bug.

Comment 21

[Benoit Jacob [:bjacob] (mostly away)]

Shouldn't this bug be re-opened due to comment 19?

Comment 22

[Benoit Jacob [:bjacob] (mostly away)]

Oh, backed out on beta only, sorry.

Comment 23

[Alice0775 White]

This backed out causes 16beta crash, see Bug 792903 comment#5 #6
So I hope reland this.

Comment 24

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

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

I think we should reland this patch on beta, for the reasons given in
https://bugzilla.mozilla.org/show_bug.cgi?id=779399#c23
and
https://bugzilla.mozilla.org/show_bug.cgi?id=787947#c18

Comment 25

[Robert O'Callahan (:roc) (email my personal email if necessary)]

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

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

Actually, wait...

Comment 26

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The patch in bug 787947 more or less replaces this patch. But I'm not sure if we should land it this late in beta.

Comment 27

[Lode Claassen]

I still (or again) experience this behaviour on FF 41.0.2 even though https://bugzilla.mozilla.org/show_bug.cgi?id=787947 seems to be fixed.

Comment 28

[u279076]

(In reply to Lode Claassen from comment #27)
> I still (or again) experience this behaviour on FF 41.0.2 even though
> https://bugzilla.mozilla.org/show_bug.cgi?id=787947 seems to be fixed.

Please file a new bug report describing your steps to reproduce and be sure to provide a copy of about:support. Thank you.