Closed Bug 794752 Opened 8 years ago Closed 8 years ago

downloads toolbar button changes size the first time it's clicked

Categories

(Toolkit :: Downloads API, defect)

x86_64
All
defect
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla19

People

(Reporter: jfkthame, Assigned: mconley)

References

Details

Attachments

(12 files, 3 obsolete files)

7.77 KB, image/png
Details
5.42 KB, image/png
Details
79.23 KB, image/png
Details
11.09 KB, image/png
Details
2.42 KB, image/png
Details
2.26 KB, image/png
Details
1.84 KB, image/png
Details
1.71 KB, image/png
Details
4.25 KB, image/png
Details
7.90 KB, image/png
Details
6.64 KB, image/png
Details
8.17 KB, patch
Details | Diff | Splinter Review
I've been seeing this on OS X Nightly for a while, and it's rather disconcerting...

When Nightly is first launched, the size of the Downloads button in the toolbar matches the Home button next to it. However, the first time I click this button, to display the Downloads panel (doorhanger), it changes size to become distinctly wider. The new (mismatched) size persists until the browser is restarted.

I'll attach screenshots showing the initial size and the after-click size.
A further observation is that the tooltip shown for the Downloads button changes after that initial click (which triggers the size change). Before the button has been clicked, it shows a tooltip that reads "Display the progress of ongoing downloads", but after the button has been used, its tooltip is simply "Downloads".

The behavior gives the impression that there are actually two independent widgets involved, one (with the longer tooltip, and the smaller size) that is displayed initially, and a different, wider one with a short tooltip that replaces it when it is first used.
Aha, a little poking around reveals that there *are* two elements, a "placeholder" called downloads-button that's defined in browser.xul, and the "real" downloads-indicator element defined in indicatorOverlay.xul.

The problems described here, then, seem to indicate a mismatch between the sizes and tooltip texts of these two elements. They really ought to match, so that the replacement appears seamless.

(Is it really so expensive to load the downloads-indicator that it's worth the extra complexity of doing it lazily?)
Duplicate of this bug: 801593
The switching effect is even worse on Windows as Windows has two states of each button, one light and one dark, while the actual new downloads button has only dark state. So when the switch happens, the button becomes black, even if it was placed on titlebar, where the initial previous button was white.

So on windows 7 aero, 3 things happen:

1) Button changes color from white to black
2) Button increases in width
3) Tooltip changes

There are 2 fixes for this :

1. Do not use the old downloads button initially and then do the switch. Rather use make the new downloads button respect background color, so that it is white on the titlebar and use that from the beginning only
or
1. Make the width of the new downloads button to match that of old downloads button, and still make it respect the background color so that the switch is seamless.

I think this bug should be a high priority bug as the user experience is affected a lot.
the old button switching is needed till the feature can be disabled, that means until we decide it's stable enough to be released.
(In reply to Marco Bonardo [:mak] from comment #6)
> the old button switching is needed till the feature can be disabled, that
> means until we decide it's stable enough to be released.

Okay, then use the seconds option. Fix the new downloads button's width, tooltip and respect the background color :)
Duplicate of this bug: 803227
I seem to notice a slight change on Win7 as well, should be verified, the placeholder and the indicator must appear identical.
(In reply to Marco Bonardo [:mak] from comment #9)
> I seem to notice a slight change on Win7 as well, should be verified, the
> placeholder and the indicator must appear identical.

So there are two aspects of this:
1) When the button is on toolbars, it shrinks a little in height and width both, by around 2 px I think.

2) When the button is on the title bar , it expands by 8 pixel!! in width. (Also changes from white to black)
yeah, I suspect #downloads-indicator is lacking some of the rules that apply to #downloads-button
Assignee: nobody → mconley
For OSX, can I assume that the wider button is desired, to make enough room for the progress bar and time count?
Flags: needinfo?(mak77)
(In reply to Mike Conley (:mconley) from comment #12)
> For OSX, can I assume that the wider button is desired, to make enough room
> for the progress bar and time count?

I'm not sure, at first glance I'm not sure why Mac should differ in behavior from other OS, unless the buttons are usually smaller and not adequate for the contents. I didn't check live though.
Flags: needinfo?(mak77)
Here's a screenshot showing the button with progress bar and time-remaining indicator displayed. ISTM that there is more padding than needed on the sides; the button could comfortably have remained several pixels narrower. Not quite as narrow as the Home button next to it, though, unless the progress bar is shortened slightly.

IMO, it would be visually neatest for the Downloads button to match the Home button in size, given that (by default) they appear next to each other, and even have very similar-looking initial content (compare the buttons as shown in attachment 665286 [details]). So I'd suggest making the progress bar a couple of pixels shorter, and maintaining the initial "small" Downloads button size.
Could make sense, the progress bar has to be redesigned regardless, we need more contrast.  Making it shorter won't cause issues since the issue there is its height.
Attached patch WIP Patch 1 (obsolete) — Splinter Review
This fixes the tooltip (I chose the more descriptive one, and axed the one that just said "Downloads"), and fixes the button sizes on Windows 7 and XP.

I'll fix pinestripe and gnomestripe next.
Attached patch WIP Patch 2 (obsolete) — Splinter Review
This takes care of OSX. Screenshots coming.
Attachment #679178 - Attachment is obsolete: true
OS: Mac OS X → All
Not providing a before patch screenshot - I think the other screenshots illustrate the problem sufficiently.
Top is large icons, bottom is small icons.
Top is large icons, bottom is small icons.

I should also note that, for all of these screenshots, the left side represents the indicator, and the right side represents the placeholder.
Attached patch Patch v1 (obsolete) — Splinter Review
And here's the patch for each theme.
Attachment #679238 - Attachment is obsolete: true
Attachment #679306 - Flags: review?(mak77)
I tried this patch on OS X, and it looks pretty good IMO - even though the button remains small now, it doesn't feel like the progress bar is too crowded, so perhaps shaving a pixel off the ends of it (as suggested above) isn't necessary after all.
Comment on attachment 679306 [details] [diff] [review]
Patch v1

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

::: browser/themes/gnomestripe/downloads/downloads.css
@@ +173,5 @@
> +}
> +
> +toolbar[iconsize="large"] #downloads-indicator-anchor {
> +  width: 24px;
> +  height: 24px;

just to be coherent with the other themes I'd probably use min- versions

::: browser/themes/winstripe/downloads/downloads.css
@@ +173,5 @@
>  #downloads-indicator-icon {
>    background: -moz-image-rect(url("chrome://browser/skin/Toolbar.png"),
>                                0, 108, 18, 90) center no-repeat;
> +  min-height: 18px;
> +  min-width: 18px;

is this actually really needed since this is inside downloads-indicator-anchor that has min-width and height defined already?
Attachment #679306 - Flags: review?(mak77) → review+
(In reply to Marco Bonardo [:mak] from comment #27)
> Comment on attachment 679306 [details] [diff] [review]
> Patch v1
> 
> Review of attachment 679306 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/gnomestripe/downloads/downloads.css
> @@ +173,5 @@
> > +}
> > +
> > +toolbar[iconsize="large"] #downloads-indicator-anchor {
> > +  width: 24px;
> > +  height: 24px;
> 
> just to be coherent with the other themes I'd probably use min- versions
> 

Done.

> ::: browser/themes/winstripe/downloads/downloads.css
> @@ +173,5 @@
> >  #downloads-indicator-icon {
> >    background: -moz-image-rect(url("chrome://browser/skin/Toolbar.png"),
> >                                0, 108, 18, 90) center no-repeat;
> > +  min-height: 18px;
> > +  min-width: 18px;
> 
> is this actually really needed since this is inside
> downloads-indicator-anchor that has min-width and height defined already?

Good point - turns out the one on downloads-indicator-anchor is the redundant / useless one. I've removed it.  Thanks for catching that.

I'll upload a new patch with the changes.

Thanks for the r+!
(In reply to Mike Conley (:mconley) from comment #28)
> Good point - turns out the one on downloads-indicator-anchor is the
> redundant / useless one. I've removed it.  Thanks for catching that.

you sure? that's a stack I think its there to avoid the button contents to "flicker"
Though I didn't physically test that.
(In reply to Marco Bonardo [:mak] from comment #29)
> (In reply to Mike Conley (:mconley) from comment #28)
> > Good point - turns out the one on downloads-indicator-anchor is the
> > redundant / useless one. I've removed it.  Thanks for catching that.
> 
> you sure? that's a stack I think its there to avoid the button contents to
> "flicker"
> Though I didn't physically test that.

Removing the min-* values from downloads-indicator-icon, and leaving them on downloads-indicator-anchor seems to re-introduce the 2px difference between the placeholder and the indicator buttons.

I'm not seeing any flicker in the button contents - do you have STR?
no, don't have STR, I was mostly trying to remember why we put them there...
Btw if you tested it working I think we're fine here.
Attachment #679306 - Attachment is obsolete: true
Landed on inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/9fa862c22f51
Target Milestone: --- → mozilla19
https://hg.mozilla.org/mozilla-central/rev/9fa862c22f51
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Depends on: 810340
Verified as fixed on the latest Nightly - the downloads button size and tooltip are the same before and after clicking on it.

Verified on Windows 7, Windows XP, Mac OS X 10.8.2  and Ubuntu 12.10:

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20121204 Firefox/20.0 Build ID: 20121204030754
Mozilla/5.0 (Windows NT 5.1; rv:20.0) Gecko/20121204 Firefox/20.0 Build ID: 20121204030754
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20121204 Firefox/20.0 Build ID: 20121204030754
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20121204 Firefox/20.0 Build ID: 20121204030754
Status: RESOLVED → VERIFIED
I am still seeing this issue. I have Show small toolbar icons enabled.
(In reply to Kshitij Chawla from comment #36)
> I am still seeing this issue. I have Show small toolbar icons enabled.

I am unable to reproduce this issue on Nightly with small toolbar icons.

Kshitij - is it possible for you to record and post a short screencast demonstrating how you're experiencing the bug?
I am experiencing this on Aurora 20. I posted here since the issue has been verified fixed since Dec 4 2012, so I figured it should be in Aurora now unles it hasn't been uplifted? DO you still want the screen cast?
I cannot reproduce this on Windows 7 on Aero using Mozilla/5.0 (Windows NT 6.1; WOW64; rv:20.0) Gecko/20130203 Firefox/20.0.

Yes, I think the screencast would help.
I updated aurora well it auto updated) on the 5th and I can't reproduce the issue anymore.
Okay, I managed to replicate the problem and here is the link to the screencast.

http://youtu.be/AZlUMyZpGko
(In reply to Kshitij Chawla from comment #41)
> Okay, I managed to replicate the problem and here is the link to the
> screencast.
> 
> http://youtu.be/AZlUMyZpGko

this looks due to the high number of add-ons and customization on your toolbar, the problem is that on applying the binding the toolbar customization is reapplied and causes that glitch if some add-on modifies the toolbar set.
It's definitely not this specific bug, if you can reproduce in safe-mode with add-ons temporarily disabled it's worth filing a bug.
(In reply to Kshitij Chawla from comment #41)
> Okay, I managed to replicate the problem and here is the link to the
> screencast.
> 
> http://youtu.be/AZlUMyZpGko

Thanks for the screencast Kshitij!

I watched the video, and stepped through the frames where the resize happens (you can do this with the left and right cursor keys).

It looks like one of your add-ons is resizing, *not* the downloads button. There's an icon that is a circle with a "Z" in it that is changing size.

Here is the icon before it has resized:

http://i.imgur.com/zuYEBjO.png

and here it is after:

http://i.imgur.com/dbY27pQ.png

I suggest contacting the author of that add-on to see if they can help debug the problem.
You need to log in before you can comment on or make changes to this bug.