Twitter Bootstrap buttons display unwanted horizontal line on hover.

RESOLVED FIXED in mozilla17

Status

()

Core
Graphics
RESOLVED FIXED
5 years ago
2 years ago

People

(Reporter: cheesypoof, Assigned: roc)

Tracking

({regression, testcase})

17 Branch
mozilla17
x86
Windows 7
regression, testcase
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(5 attachments)

(Reporter)

Description

5 years ago
Created attachment 647832 [details]
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

5 years ago
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

5 years ago
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
Blocks: 768775
Status: UNCONFIRMED → NEW
Component: Untriaged → Graphics
Ever confirmed: true
Keywords: regression
Product: Firefox → Core

Comment 3

5 years ago
Created attachment 647908 [details]
reduced html
Keywords: testcase
Assignee: nobody → renoroc
Er, sorry William - typo!
Assignee: renoroc → roc
Created attachment 648612 [details]
reduced somewhat further
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
https://tbpl.mozilla.org/?tree=Try&rev=3b3be5b3a7a5
Created attachment 649142 [details] [diff] [review]
fix
Attachment #649142 - Flags: review?(jmuizelaar)
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.
Attachment #649142 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/c9c7e1b9aed9
(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.
(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 :-(.
Created attachment 651262 [details] [diff] [review]
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.
Attachment #651262 - Flags: review?(jmuizelaar)
Attachment #651262 - Flags: review?(jmuizelaar) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/461c9816a3be
https://hg.mozilla.org/integration/mozilla-inbound/rev/276b57e03eba
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.
Attachment #649142 - Flags: approval-mozilla-aurora?
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.
Attachment #651262 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/461c9816a3be
https://hg.mozilla.org/mozilla-central/rev/276b57e03eba
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
Attachment #649142 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Attachment #651262 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/5b1b65bf7f4d
https://hg.mozilla.org/releases/mozilla-aurora/rev/8b243972bf82
status-firefox16: --- → fixed

Updated

5 years ago
Depends on: 787947
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?
Nothing. This patch can't be the real cause of bug 787947; at most, it revealed an existing graphics/layers bug.

Updated

5 years ago
status-firefox16: fixed → affected
Shouldn't this bug be re-opened due to comment 19?
Oh, backed out on beta only, sorry.

Comment 23

5 years ago
This backed out causes 16beta crash, see Bug 792903 comment#5 #6
So I hope reland this.
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
Attachment #649142 - Flags: approval-mozilla-beta?
Comment on attachment 649142 [details] [diff] [review]
fix

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

Actually, wait...
Attachment #649142 - Flags: approval-mozilla-beta?
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.

Updated

5 years ago
Depends on: 795837

Updated

5 years ago
Depends on: 795859
Depends on: 809478

Comment 27

2 years ago
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.
(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.
status-firefox16: affected → ---
You need to log in before you can comment on or make changes to this bug.