Closed Bug 801055 Opened 7 years ago Closed 7 years ago

When Background Theme applied, Downloads toolbar button is strangely tall.

Categories

(Firefox :: Downloads Panel, defect)

x86
macOS
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 19

People

(Reporter: mconley, Assigned: mconley)

References

(Blocks 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

STR:

1)  In a Nightly, go to https://www.getpersonas.com/en-US/
2)  Hover your mouse over some themes.

What happens?

The Downloads button gets strangely tall. (Also, for dark background themes, the icon disappears. This is being taken care of in bug 779223).

Seen screenshots.

What's expected?

The button should persist its height, regardless of the background theme.
Blocks: 726447
highly visible in primary ui :(
Attached patch Patch v1 (obsolete) — Splinter Review
This was caused by the stack having a min-height of 20px.

This was different from the winstripe / gnomestripe themes, which have a min-height of 18px. There was still a perceptible difference in height with 18px, so I just removed the min-height entirely.

I'm not entirely sure what purpose the min-height served...I did some testing without it, and I'm not seeing any difference (except that the buttons are the same height with and without a background theme enabled). So that's my solution, until further notice.
Assignee: nobody → mconley
Attachment #671977 - Flags: review?(mak77)
Comment on attachment 671977 [details] [diff] [review]
Patch v1

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

So, 2 things:
- not sure how this acts on lwtheme, unless there is a non-lwtheme rule that overrides the min-height
- if we remove it from here, makes sense to remove it from win and gnome as well
I suppose you may check with DOMi wich rule is applied here
Flags: needinfo?(mconley)
Comment on attachment 671977 [details] [diff] [review]
Patch v1

Ok, I think I've narrowed down the problem some - removing the toolbarbutton-icon class from the downloads-indicator-anchor seems to fix things.

Digging into why exactly that is...
Attachment #671977 - Attachment is obsolete: true
Attachment #671977 - Flags: review?(mak77)
Flags: needinfo?(mconley)
Attached patch Patch v2Splinter Review
Ah hah - figured it out. It wasn't a misbehaving rule - it was a missing rule that was being applied to all other default toolbar buttons.
Attachment #673985 - Flags: review?(mak77)
(To make reviewing easier, I just added #downloads-indicator to the list)
Comment on attachment 673985 [details] [diff] [review]
Patch v2

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

this makes a lot of sense, shame on me for having missed this in the original review
Attachment #673985 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #9)
> ... shame on me for having missed this in the
> original review

No worries - omissions are often the hardest defects to detect.

Landed on inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/8bfca415b643
https://hg.mozilla.org/mozilla-central/rev/8bfca415b643
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7; rv:19.0) Gecko/19.0 Firefox/19.0
Build ID: 20121113030658

Verified  as fixed on the latest Nightly - the Downloads button does not change  dimension when a background theme is applied.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.