815678 - Downloads Panel changes width when scanning completed download

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect)

Description

[Simona Badau]

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20.0 Firefox/20.0
Build ID: 20121127030907

On the latest Nightly the width of the Downloads panel changes the first time a download is ever made through the panel. This behavior is not encountered on Ubuntu 12.04 and Mac OS X 10.7. This is only reproducible on Windows.

Reproducible: always

Steps to reproduce:
1. Launch Firefox with a new profile.
2. Navigate to http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/ and download the any exe file
3. Observe the Downloads Panel until the download is complete.

Expected results:
The width of the downloads panel is the same until the download is completed.

Actual results:
The width of the downloads panel changes back and forth in the moment of the completion. 
Please see the screencast for more details: http://screencast.com/t/7lcjPWEqv

Comment 1

[Marco Bonardo [:mak]]

This shouldn't really happen :(

Comment 2

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Simona:

Thanks so much for catching this - and especially for the screencast. I was able to go through each frame and see the cause of the panel resize.

The cause seems to be when we're in the "scanning" state, the download item button/icon is collapsed.

For the toolkit downloads manager, we also hide the icons when scanning, but the effect is not jarring because the window does not automatically resize.

I'll see if I can rig a blank placeholder for the button / icon so that we don't resize.

Thanks again Simona!

-Mike

Comment 3

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: The panel when downloading, and when scanning

Comment 4

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: WIP Patch 1

Fixes the issue for winstripe.

Comment 5

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: Patch on Windows 7

Comment 6

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: Patch on Windows XP

Comment 7

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: WIP Patch 2

Updating pinstripe to account for downloadButtonContainer.

Comment 8

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: Patch v1

This takes care of gnomestripe too.

Comment 9

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Comment on attachment 685806 [details] [diff] [review]
Patch v1

So this patch puts the download item buttons into a XUL box that has a min-height and min-width set on it.

Not sure if we need the downloadButtonContainer stylings on gnomestripe and pinstripe, since I've never seen virus scanning through Firefox on either of those platforms, but I went for it anyway.

Also, since I added the new container, I had to relax some of the CSS rules that assumed that the buttons were directly beneath the downloaditem.

Comment 10

[Marco Bonardo [:mak]]

What I dislike is the hardcoded values, basically any change in the buttons css (even in toolkit) will break this.
Mike is going to try a solution with a <stack> and visibility: hidden.

Comment 11

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: Patch v2

The stack and toggling visibility: hidden seems to do the trick!

This gets rid of those hard-coded width values, which is awesome.

Testing on Windows next, and then I'll r?.

Comment 12

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Comment on attachment 686127 [details] [diff] [review]
Patch v2

This fixes the issue on Windows as well. I feel a lot better about this approach.

Comment 13

[Marco Bonardo [:mak]]

Comment on attachment 686127 [details] [diff] [review]
Patch v2

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

::: browser/components/downloads/content/downloads.css
@@ +68,5 @@
>    display: none;
>  }
>  
> +/*** Visibility of download item icons, and the controls inside the downloads
> +     indicator ***/

"download item icons" is confusing, I thought of the icon for each download... let's use "buttons" instead of "icons".

Actually "Visibility of download buttons and indicator controls" is enough imo.

Comment 14

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Attachment: Patch v3 (r+'d by mak)

Thanks Marco! Comment fixed.

Comment 15

[Mike Conley (:mconley) (:⚙️) (Catching up on needinfos)]

Landed on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/d263e80907f9

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d263e80907f9

Comment 17

[Simona Badau]

Verified as fixed on the latest Nightly - the panel doesn't change width when scanning completed download.

Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20121203 Firefox/20.0
Build ID: 20121203030801