Last Comment Bug 726444 - Implement the Downloads Panel
: Implement the Downloads Panel
Status: RESOLVED FIXED
[=== See the STATUS PAGE before posti...
: addon-compat
Product: Firefox
Classification: Client Software
Component: Downloads Panel (show other bugs)
: Trunk
: All All
: -- normal with 13 votes (vote)
: Firefox 20
Assigned To: :Paolo Amadini
:
Mentors:
https://wiki.mozilla.org/User:P.A./Pa...
: 663772 685357 697678 697679 697680 697683 697686 697688 708566 (view as bug list)
Depends on: 734701 736172 746787 746853 747018 747310 781455
Blocks: 726445 726447 726450 726451 726453 726456 DownloadsPanel 726454 732924
  Show dependency treegraph
 
Reported: 2012-02-12 09:38 PST by :Paolo Amadini
Modified: 2013-10-21 00:50 PDT (History)
46 users (show)
hskupin: in‑moztrap+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Coalesced patch (266.77 KB, patch)
2012-02-12 09:48 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Current Downloads Panel design (5.82 KB, text/plain)
2012-02-12 11:53 PST, :Paolo Amadini
limi: ui‑review+
Details
Current tryserver build (100 bytes, text/plain)
2012-02-12 12:04 PST, :Paolo Amadini
limi: ui‑review+
Details
No migration code and other minor changes (256.68 KB, patch)
2012-02-16 04:23 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
review of attachment 597771 (46.56 KB, text/plain)
2012-02-27 16:39 PST, Marco Bonardo [::mak]
no flags Details
The patch (250.53 KB, patch)
2012-03-05 05:15 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Reverted one change in the panel opening sequence (250.53 KB, patch)
2012-03-06 02:28 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Removed constants and general-purpose modules from the global scope (250.51 KB, patch)
2012-03-06 11:42 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Removed a few calls in the global scope and fixed use of system colors (250.25 KB, patch)
2012-03-09 03:12 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Lazily load the panel overlay. (12.91 KB, patch)
2012-03-09 03:14 PST, :Paolo Amadini
dao+bmo: review-
Details | Diff | Splinter Review
Don't use overlays. (18.15 KB, patch)
2012-03-09 04:06 PST, :Paolo Amadini
no flags Details | Diff | Splinter Review
Improved startup and shutdown performance (254.61 KB, patch)
2012-03-10 00:34 PST, :Paolo Amadini
mak77: review+
Details | Diff | Splinter Review
Other performance and styling changes (255.09 KB, patch)
2012-03-24 03:21 PDT, :Paolo Amadini
no flags Details | Diff | Splinter Review
Nearly optimal startup and shutdown performance (254.58 KB, patch)
2012-04-12 10:41 PDT, :Paolo Amadini
no flags Details | Diff | Splinter Review
Use the current Downloads button when the feature is disabled (253.58 KB, patch)
2012-04-16 10:01 PDT, :Paolo Amadini
mak77: review+
Details | Diff | Splinter Review
Finally! (253.89 KB, patch)
2012-04-17 04:34 PDT, :Paolo Amadini
no flags Details | Diff | Splinter Review

Description :Paolo Amadini 2012-02-12 09:38:36 PST
Initial version of the Downloads Panel, to be included in Nightly builds for
wider testing, enabled by default. The feature is ready to be disabled in the
Aurora channel until all the release criteria has been met.

This does not include the changes to the Downloads folder in the Library window.
Comment 1 :Paolo Amadini 2012-02-12 09:48:04 PST
Created attachment 596494 [details] [diff] [review]
Coalesced patch

This initial patch includes all the changes from bug 564934, bug 663772,
bug 685357, bug 708566, bug 697686, bug 697688, bug 697683, bug 697680,
bug 697678, and Javi Rueda's patch in bug 697679.
Comment 2 :Paolo Amadini 2012-02-12 11:38:31 PST
*** Bug 663772 has been marked as a duplicate of this bug. ***
Comment 3 :Paolo Amadini 2012-02-12 11:38:32 PST
*** Bug 685357 has been marked as a duplicate of this bug. ***
Comment 4 :Paolo Amadini 2012-02-12 11:38:34 PST
*** Bug 708566 has been marked as a duplicate of this bug. ***
Comment 5 :Paolo Amadini 2012-02-12 11:38:37 PST
*** Bug 697686 has been marked as a duplicate of this bug. ***
Comment 6 :Paolo Amadini 2012-02-12 11:38:39 PST
*** Bug 697688 has been marked as a duplicate of this bug. ***
Comment 7 :Paolo Amadini 2012-02-12 11:38:43 PST
*** Bug 697683 has been marked as a duplicate of this bug. ***
Comment 8 :Paolo Amadini 2012-02-12 11:38:53 PST
*** Bug 697680 has been marked as a duplicate of this bug. ***
Comment 9 :Paolo Amadini 2012-02-12 11:38:54 PST
*** Bug 697678 has been marked as a duplicate of this bug. ***
Comment 10 :Paolo Amadini 2012-02-12 11:39:35 PST
*** Bug 697679 has been marked as a duplicate of this bug. ***
Comment 11 :Paolo Amadini 2012-02-12 11:53:35 PST
Created attachment 596501 [details]
Current Downloads Panel design

This is the current design from the Downloads Panel feature page. Can you check
if it matches what we want to do in the end when the panel is released?
Comment 12 :Paolo Amadini 2012-02-12 12:04:35 PST
Created attachment 596503 [details]
Current tryserver build

Different question :-) Here you can find the latest tryserver build, including
the latest developments and bug fixes:

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/paolo.mozmail%40amadzone.org-76ac14c73097/

Our goal is to land this on Nightly _enabled_, for wider testing, and disable
the feature on Aurora until it is ready for release. Do you think that this
trybuild is ready to land enabled? If not, what is missing exactly?

Note that we don't need a list of issues we should fix for release, just for
making the feature available to Nightly users. Lists of known issues are
already filed as follow-ups of this bug. See in particular bug 726447 for
issues related to theming and visual design. We know that many (maybe all)
of them do not need to be fixed for Nightly users.
Comment 13 Dão Gottwald [:dao] 2012-02-13 02:41:55 PST
Comment on attachment 596494 [details] [diff] [review]
Coalesced patch

>+      <toolbarbutton id="downloads-button"
>+                     class="toolbarbutton-1 chromeclass-toolbar-additional"
>+                     label="&downloads.label;"
>+                     tooltiptext="&downloads.tooltip;"
>+                     removable="true"/>

There's no feedback upon starting a download with the button removed. What should happen instead seems pretty to me: The button should appear transiently and disappear again when starting up without active downloads.
Comment 14 Marco Bonardo [::mak] 2012-02-13 03:59:58 PST
(In reply to Dão Gottwald [:dao] from comment #13)
> There's no feedback upon starting a download with the button removed. What
> should happen instead seems pretty to me: The button should appear
> transiently and disappear again when starting up without active downloads.

While it's an important case to handle, I don't think should block Nightly testing, rather it should block Aurora enable-by-default.  We should file a tracking bug for what blocks enable-by-default on Aurora and add dependencies on it.
Comment 15 Dão Gottwald [:dao] 2012-02-13 04:09:18 PST
Given my seemingly unheard preaching on related issues*, I'd rather see it addressed upfront.

*: I haven't yet seen an explanation why the behavior described in comment 13 isn't the /default/ behavior.
Comment 16 :Paolo Amadini 2012-02-13 04:27:11 PST
This bug is about landing enabled in Nightly only, we shouldn't block on other
improvements here, even if they're very important for release (like accessibility).

There is a pending UX review for determining the Nightly landing criteria for
the feature behavior. This is explicitly a responsibility of the UX team. If
there are any suggestions for them, they should probably be discussed in the
dev.usability forum.
Comment 17 Dão Gottwald [:dao] 2012-02-13 05:47:08 PST
It's a responsibility from peers as well. I can make this explicit if you want: you're adding a removable button without reasonably handling the case if the button being removed. This is r- worthy and therefore belongs in this bug, not newsgroups. "Nightly only" doesn't matter too much. There's only one mozilla-central and stuff wanting to land there needs to meet certain criteria.

By the way, what exactly is the plan for disabling on Aurora? Flipping a pref won't be enough, given the permanent modifications to the navigation toolbar.
Comment 18 Dão Gottwald [:dao] 2012-02-13 05:50:14 PST
"the case if the button being removed" => "the case of the button being removed"
Comment 19 Alex Limi (:limi) — Firefox UX Team 2012-02-13 14:31:59 PST
Comment on attachment 596501 [details]
Current Downloads Panel design

I'm OK with landing the current version on Nightly. And yes, it has visual bugs. :)
Comment 20 Alex Limi (:limi) — Firefox UX Team 2012-02-13 14:32:11 PST
Comment on attachment 596503 [details]
Current tryserver build

I'm OK with landing the current version on Nightly. And yes, it has visual bugs. :)
Comment 21 Dietrich Ayala (:dietrich) 2012-02-13 17:31:18 PST
(In reply to Dão Gottwald [:dao] from comment #17)
> you're adding a removable button without reasonably handling the case
> if the button being removed. 

Limi: What is the expected behavior for when a user customizes-away the downloads button?
Comment 22 :Paolo Amadini 2012-02-14 04:27:44 PST
(In reply to Dão Gottwald [:dao] from comment #17)
> By the way, what exactly is the plan for disabling on Aurora? Flipping a
> pref won't be enough, given the permanent modifications to the navigation
> toolbar.

That's a good point. In fact I think that, for testing in Nightly, we will go for
a solution that does not require permanent modifications to the navigation bar,
with no migration code involved. Just flipping a preference should revert Nightly
users to the previous state, as if the feature never existed.

This is a bit more work now, but gives us maximum flexibility when figuring out
the final behavior, position, etc. for the button without limiting our choices
or creating more work for reverting later.
Comment 23 :Paolo Amadini 2012-02-16 04:23:26 PST
Created attachment 597771 [details] [diff] [review]
No migration code and other minor changes

This patch implements the "no migration code" approach.

- The button is shown in the navigation bar by default.
- For now you cannot customize the button away from the navigation bar, and the
  only way to remove it is to disable the entire feature. For this reason, the
  button is currently hidden when there are no downloads.
- If you placed the downloads button on a toolbar, we show the indicator there,
  always visible. Note that if you test this build in the same profile where
  you tested the previous ones, you already have the button there, so you have
  to remove the placeholder from the toolbar again for the button to disappear
  when there are no downloads (or test on a clean profile).

We are free to improve on this design without affecting existing profiles.

To disable feature: flip "browser.download.manager.useWindowUI" and restart.

Tryserver build: https://tbpl.mozilla.org/?tree=Try&rev=fed844f653d2
Comment 24 :Paolo Amadini 2012-02-16 04:28:55 PST
Limi, are you fine with Nightly users testing the design in comment 23, thus
with the behavior (on a clean profile) of the tryserver build that will soon
be available here?

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/paolo%2Emozmail%40amadzone%2Eorg-fed844f653d2
Comment 25 Dão Gottwald [:dao] 2012-02-16 04:33:52 PST
Comment on attachment 597771 [details] [diff] [review]
No migration code and other minor changes

>--- /dev/null
>+++ b/browser/themes/winstripe/downloads/downloads-aero.css

some of the following comments apply to other files as well

>+#downloadsPanel .panel-inner-arrowcontent {

use the child selector

>+@media all and (-moz-windows-default-theme) {

remove "all and"

>+  richlistitem[type="download"][state="1"] .downloadInfo {
>+    border: 1px solid transparent;
>+    -moz-padding-end: 8px;
>+  }
>+
>+  richlistitem[type="download"][state="1"] .downloadInfo:hover {
>+    border: 1px solid hsl(213,45%,65%);
>+    box-shadow: 0 0 0 1px hsla(0,0%,100%,.5) inset,
>+                0 1px 0 hsla(0,0%,100%,.3) inset;
>+    background-image: -moz-linear-gradient(hsl(212,86%,92%), hsl(212,91%,86%));
>+    color: black;
>+  }

child selector

>+@media not all and (-moz-windows-classic) {
>+  richlistitem[type="download"][state="1"]:hover .downloadButton {

child selector

>+  @media all and (-moz-windows-classic) {

this doesn't seem to make sense. you're still inside "@media not all and (-moz-windows-classic)".

>+    richlistitem[type="download"][state="1"] .downloadInfo:hover .downloadDetails {

child selector

>+@media all and (-moz-windows-compositor) {

remove "all and"

>+++ b/browser/themes/winstripe/downloads/downloads.css

>+@media not all and (-moz-windows-classic) {
>+  #downloadsHistory {
>+    background-color: hsla(216,45%,88%,.98);
>+    box-shadow: 0px 1px 2px rgb(204,214,234) inset;  
>+  }
>+}

I'm doubtful that this is the right color for all Windows versions and themes except classic.
Comment 26 Marco Bonardo [::mak] 2012-02-16 07:05:37 PST
While some css comments can be fixed now (thanks for any feedback on any simple fixes we can do immediately), I think we should be fine with fixing theme glitches live in Nightly.  Obviously we woulnd't ship it in Aurora unless it reaches our quality levels.
As you know, there may be many small details in themes and would be an infinite work to fix all of them in a 260KB patch.  Would you be fine with handling theme details in follow-ups so we can work on small cleanup patches?
Comment 27 Dão Gottwald [:dao] 2012-02-16 16:45:56 PST
(In reply to Marco Bonardo [:mak] from comment #26)
> While some css comments can be fixed now (thanks for any feedback on any
> simple fixes we can do immediately), I think we should be fine with fixing
> theme glitches live in Nightly.  Obviously we woulnd't ship it in Aurora
> unless it reaches our quality levels.

What ensures this? What's the downside of addressing review comments before landing stuff? It happens too often that followup bugs are filed but never taken care of.
Comment 28 Marco Bonardo [::mak] 2012-02-17 03:20:37 PST
We should address comments before landing, we should not hang trying to reach perfection in every single detail though, like getting the right color or deciding a font should be 1px larger.
Once we have followups we can prioritize them as blocking or not, and blocking ones MUST be fixed before the feature can be enabled on Aurora. What we gain is:
1. more testing
2. more eyes on the code
3. more developers able to contribute fixes, rather than a single one
4. small incremental patches, easier to review and not bitrotting every other day
Comment 29 Dão Gottwald [:dao] 2012-02-17 04:04:49 PST
I wasn't lamenting over color shades or font sizes. I pointed out code that looks bogus to me. Code identified as being bogus should simply be fixed before it lands unless there's a good reason why that's not practical in a particular case. "I want to land this as soon as possible" isn't such a reason.

Prioritizing known bugs post-landing is what my previous question aimed at: How does it ensure things get addressed? Experience shows that bugs classified as non-blocking likely just won't be taken care of. This is a recipe for eroding code quality and one central reason why we require code review.
Comment 30 Marco Bonardo [::mak] 2012-02-17 04:57:53 PST
(In reply to Dão Gottwald [:dao] from comment #29)
> Experience shows that bugs
> classified as non-blocking likely just won't be taken care of.

Bugs classified as non blocking are non blocking, otherwise would be classified differently. If the classification was wrong that's another story.
Comment 31 Dão Gottwald [:dao] 2012-02-17 05:01:34 PST
And I say review comments need to be addressed regardless of some blocking classification.
Comment 32 Manoj 2012-02-22 10:50:09 PST
I see this over and over in bug reports, "It would help if you provided a patch". Going with that philosophy Dao, you are, "... doubtful that this is the right color for all Windows versions and themes except classic." 

Are you doubtful or are you certain? If you are certain, suggest the right color for all Windows versions and themes except classic so that the change can be made as per your request.

We've been waiting for the new download manager since Limi first proposed the design in April 2011. Let's figure out what needs to be done to remove obstacles in the path of landing the feature that has been mimicked by Safari for about 8 months now.
Comment 33 Dão Gottwald [:dao] 2012-02-22 11:18:34 PST
I can't suggest the right color, because the part that looks wrong is "@media not all and (-moz-windows-classic)". There's very little that the themes targeted there have in common.
Comment 34 Manoj 2012-02-22 11:32:38 PST
There are a number of theme developers on the mozillazine forum commenting on daily builds. Without this feature landing in Nightly, there is no way to solicit their feedback on the impact of the changes to their theming efforts.

If getting theme impact is material to fixing this bug, I suggest posting a try build to the forum and gauging the response. The community is eager to test and comment on this new feature.
Comment 35 bogas04 2012-02-24 22:48:01 PST
Its now in latest UX builds : http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/ux-win32/

I made a video preview of it http://www.youtube.com/watch?v=eon_jVq2GOs
Comment 36 :Paolo Amadini 2012-02-25 02:07:17 PST
(In reply to bogas04 from comment #35)
> I made a video preview of it http://www.youtube.com/watch?v=eon_jVq2GOs

Thank you! This is a very effective overview.

I noticed you had the downloads button placed on the toolbar, since you probably
tested on the same profile where you also tried out a previous iteration.

In the current version, the button is not placed on the navigation bar
automatically, but still appears there if there are any recent downloads.
It also disappears when you remove all the download items from the list.
To see this new default behavior, you can just remove the button from any
toolbar during the customization.

We don't know yet what the exact behavior will be in the final version.

With regard to availability, it's very probable that the feature will be present
but still *disabled* by default in the Aurora channel for Firefox 13. Might be
worth mentioning in the video's description :-)
Comment 37 Marco Bonardo [::mak] 2012-02-25 04:15:07 PST
If possible please keep the discussions in bug 564934, this is an implementation bug.
Comment 38 Marco Bonardo [::mak] 2012-02-27 16:39:06 PST
Created attachment 601123 [details]
review of attachment 597771 [details] [diff] [review]

globally it's still good enough to land.
After comments are addressed, there is still a strong requirement before landing, that is to do a couple try runs without and with the patch and compare-talos them to check for eventual regressions.
If you have concerns on the comments feel free to ping me on IRC, or drop me a mail.
Comment 39 Marco Bonardo [::mak] 2012-02-27 16:39:50 PST
Comment on attachment 597771 [details] [diff] [review]
No migration code and other minor changes

review attached, will flag when requirements from previous comment are satisfied
Comment 40 Marco Bonardo [::mak] 2012-02-27 16:42:24 PST
As a side note, if you want to meet up in person, to do final checks and fixes, we can adjust that.
Comment 41 :Paolo Amadini 2012-03-05 05:15:49 PST
Created attachment 602855 [details] [diff] [review]
The patch

Tryserver builds for compare-talos just started:

https://tbpl.mozilla.org/?tree=Try&rev=ffb696a80bc7
https://tbpl.mozilla.org/?tree=Try&rev=a988a8a0e290
Comment 42 Marco Bonardo [::mak] 2012-03-05 16:37:41 PST
I have retriggered all Talos, so we can account for noise.
Looks like there is a new failure on Mac in browser_delete_key_removes.js.
Comment 43 :Paolo Amadini 2012-03-06 02:28:43 PST
Created attachment 603207 [details] [diff] [review]
Reverted one change in the panel opening sequence

(In reply to Marco Bonardo [:mak] from comment #42)
> I have retriggered all Talos, so we can account for noise.

I'm still not sure how to interpret the values I see in compare-talos.

> Looks like there is a new failure on Mac in browser_delete_key_removes.js.

I've already run a separate tryserver build to pinpoint the problem. It's
confirmed to be caused by one of the changes made to the panel focus functions,
though I can't directly test on Mac so I can't understand the issue exactly.
Probably, on Mac, selectedIndex should be set before moving focus to the list.
Since I don't have enough information to file a bug or add a specific comment,
I've simply reverted the change. The regression test will ensure that the code
continues to work as expected.

This change doesn't affect Talos, so there's no need to do another tryserver
build for all platforms.
Comment 44 Marco Bonardo [::mak] 2012-03-06 08:40:49 PST
(In reply to Paolo Amadini from comment #43)
> I'm still not sure how to interpret the values I see in compare-talos.

Show details here, I removed the tests with uninteresting results.
http://tinyurl.com/73ar4ql
Looks like there is an about 10ms hit on Win and Mac (Lion excluded) in Ts paint and Tpaint. We should recheck all the init and see what we can do there.
Comment 45 Dão Gottwald [:dao] 2012-03-06 08:51:33 PST
Comment on attachment 603207 [details] [diff] [review]
Reverted one change in the panel opening sequence

>--- /dev/null
>+++ b/browser/components/downloads/content/downloads.js

>+Cu.import("resource://gre/modules/XPCOMUtils.jsm");
>+Cu.import("resource://gre/modules/Services.jsm");

Those are already in utilityOverlay.js.

>+XPCOMUtils.defineLazyModuleGetter(this, "NetUtil",
>+                                  "resource://gre/modules/NetUtil.jsm");

This also seems to be in the global browser window scope already, although I don't know where it's coming from. Anyway, general-purpose modules shouldn't be loaded in overlays.

>+const nsIDM = Ci.nsIDownloadManager;

Please don't put it internal stuff like this in the global scope that you share with other scripts.
Comment 46 :Paolo Amadini 2012-03-06 11:42:56 PST
Created attachment 603390 [details] [diff] [review]
Removed constants and general-purpose modules from the global scope
Comment 47 :Paolo Amadini 2012-03-06 12:12:20 PST
To help in understanding the cause of the startup performance reduction, I've
started a new series of Talos tests in these conditions:

https://tbpl.mozilla.org/?tree=Try&rev=ea16e55764b7
-- Removed downloadsOverlay.xul entirely.

https://tbpl.mozilla.org/?tree=Try&rev=f4f897f9157e
-- Kept the overlay but removed the two calls on startup.

https://tbpl.mozilla.org/?tree=Try&rev=0c6c69f31fa4
-- Removed the synchronous startup call, kept the delayed call.

https://tbpl.mozilla.org/?tree=Try&rev=23df0304d7be
-- Kept the synchronous startup call, removed the delayed call.

https://tbpl.mozilla.org/?tree=Try&rev=fb7f94c85413
-- Kept everything as in the Talos run from a few days ago. The only change is
   that we don't re-import two general-purpose JavaScript modules.
Comment 48 Marco Bonardo [::mak] 2012-03-07 06:17:32 PST
(In reply to Paolo Amadini from comment #47)
> https://tbpl.mozilla.org/?tree=Try&rev=ea16e55764b7
> -- Removed downloadsOverlay.xul entirely.

this is the only one that shows no regression, not so surprisingly.

The modules import removal seems to have helped a little bit, 1 or 2ms. Though it still shows a 4-6ms hit.
Comment 49 :Paolo Amadini 2012-03-08 02:14:31 PST
We've started two more tests:

https://tbpl.mozilla.org/?tree=Try&rev=7f3a403d0381
-- Kept everything except the calls in the global scope of the scripts.

https://tbpl.mozilla.org/?tree=Try&rev=7f017ea4d29b
-- Kept the scripts but removed the overlay and the two calls on startup.
Comment 50 :Paolo Amadini 2012-03-09 03:12:49 PST
Created attachment 604357 [details] [diff] [review]
Removed a few calls in the global scope and fixed use of system colors
Comment 51 :Paolo Amadini 2012-03-09 03:14:18 PST
Created attachment 604358 [details] [diff] [review]
Lazily load the panel overlay.

This should give a measurable startup performance gain.
Comment 52 Dão Gottwald [:dao] 2012-03-09 03:32:55 PST
Comment on attachment 604358 [details] [diff] [review]
Lazily load the panel overlay.

This will trigger bug 640158.

(Using overlays was probably the wrong design choice, since you don't want to plug this into anything but browser.xul. If you're doing it for code separation, the parts could be put together at build time rather than run time.)
Comment 53 :Paolo Amadini 2012-03-09 03:45:32 PST
(In reply to Dão Gottwald [:dao] from comment #52)
> This will trigger bug 640158.

Thanks! I see that there is a workaround for this problem, if the performance
gain of delaying the creation of the panel element is significant.

> (Using overlays was probably the wrong design choice, since you don't want
> to plug this into anything but browser.xul. If you're doing it for code
> separation, the parts could be put together at build time rather than run
> time.)

This is something I was also considering. I will do a tryserver build using an
"#include" for the elements directly in browser.xul. If the performance gain is
comparable to lazy loading, it is definitely a better solution.
Comment 54 :Paolo Amadini 2012-03-09 04:06:24 PST
Created attachment 604361 [details] [diff] [review]
Don't use overlays.
Comment 55 Marco Bonardo [::mak] 2012-03-09 13:39:02 PST
So, along the last hours we examined a bunch of Talos results and we identified two main issues:

1. There is some minor work at shutdown that we can avoid in most cases. Paolo will update the main patch for that tomorrow.

2. There is a small regression on window opening.  We fixed most of the low hanging fruits for that (all global functions and modules have been removed).  There is still one clear cause that is the overlay.  As suggested by Dão the main overlay will be included at build time, though after measuring this, we found there is still a small regression in Ts and Tpaint (http://tinyurl.com/83396wl). So the plan is still to do lazy loading of just the panel overlay, obviously workarounding bug 640158, as we did for the star panel. The measurement shows this is indeed effective (http://tinyurl.com/73425q3).

Any doubts or concerns before we do this?
Comment 56 :Paolo Amadini 2012-03-10 00:34:25 PST
Created attachment 604615 [details] [diff] [review]
Improved startup and shutdown performance

A new tryserver build with Talos tests enabled is running here:
https://tbpl.mozilla.org/?tree=Try&rev=04495213dc7e

The build above is based on the same base changeset as the previous tests:
https://tbpl.mozilla.org/?tree=Try&rev=f1278af7662f

A unit-test only build on a more recent changeset is running here:
https://tbpl.mozilla.org/?tree=Try&rev=c0f2c2e99f9f
Comment 57 Marco Bonardo [::mak] 2012-03-10 01:37:34 PST
Comment on attachment 604615 [details] [diff] [review]
Improved startup and shutdown performance

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

::: browser/components/downloads/content/downloads.js
@@ +115,5 @@
> +  initialize: function DP_initialize(aCallback)
> +  {
> +    if (this._panelState != this.kPanelUninitialized) {
> +      this._ensureOverlayLoaded(aCallback);
> +      return;

kPanelUninitialized is the initial value, in such a case we know the overlay has to be loaded, and indeed we proceed, in all the other cases couldn't we just aCallback() since we know the overlay loading has already been started?
Comment 58 Marco Bonardo [::mak] 2012-03-10 03:20:16 PST
Latest patch talos results: http://tinyurl.com/7db9334
Comment 59 Marco Bonardo [::mak] 2012-03-10 05:35:07 PST
Comment on attachment 604615 [details] [diff] [review]
Improved startup and shutdown performance

Discusses on irc.  The numbers look good, there is still some minor unclear result, but considered the 16ms resolution of Windows timers, we may end figthing ghosts for days or weeks.  Not sure there's anything more we could do right now.
So I suggest proceeding with the landing and see real numbers in the tree, a backout in any case is easy and won't leave any traces.
Comment 60 :Paolo Amadini 2012-03-10 06:04:35 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba7da6256b23

Note: this feature will be disabled by default in Aurora.
Comment 61 :Paolo Amadini 2012-03-10 06:09:20 PST
As said, the plan is to disable this feature using a preference in Firefox 13,
however it has also been suggested to keep it enabled for one week, like it was
done for the New Tab page.
Comment 62 bogas04 2012-03-10 06:10:35 PST
w00t! \o/
Comment 63 Wes Kocher (:KWierso) 2012-03-10 19:31:35 PST
Was the tracking-firefox13 flag intentionally cleared? The post that set it and the post that cleared it were only a minute apart.
Comment 64 Ed Morley [:emorley] 2012-03-11 16:15:07 PDT
There are numerous regression reports on dev.tree-management for this. Mak, happy to let you decide best course of action, but they are across multiple tests and platforms, so seem a lot more real than suspected in comment 59.

Regression :( Tp5r MozAfterPaint (Private Bytes) increase 3.98% on Linux Mozilla-Inbound-Non-PGO
------------------------------------------------------------------------------------------------
    Previous: avg 377455366.667 stddev 2673427.601 of 30 runs up to revision b74bbbfafde3
    New     : avg 392469200.000 stddev 837328.908 of 5 runs since revision ba7da6256b23
    Change  : +15013833.333 (3.98% / z=5.616)
    Graph   : http://mzl.la/A8uYbx 

Regression :( Trace Malloc MaxHeap increase 1.18% on CentOS release 5 (Final) Mozilla-Inbound
---------------------------------------------------------------------------------------------
    Previous: avg 26491933.333 stddev 79171.526 of 30 runs up to revision b74bbbfafde3
    New     : avg 26804620.000 stddev 21195.448 of 5 runs since revision ba7da6256b23
    Change  : +312686.667 (1.18% / z=3.949)
    Graph   : http://mzl.la/wlp1ZK 

Regression :( Paint increase 9.68% on Win7 Mozilla-Inbound-Non-PGO
------------------------------------------------------------------
    Previous: avg 184.530 stddev 3.704 of 30 runs up to revision b74bbbfafde3
    New     : avg 202.400 stddev 3.270 of 5 runs since revision ba7da6256b23
    Change  : +17.870 (9.68% / z=4.825)
    Graph   : http://mzl.la/yX22u3 

Regression :( Trace Malloc Allocs increase 1.91% on CentOS (x86_64) release 5 (Final) Mozilla-Inbound
-----------------------------------------------------------------------------------------------------
    Previous: avg 449646.600 stddev 749.481 of 30 runs up to revision b74bbbfafde3
    New     : avg 458254.800 stddev 308.326 of 5 runs since revision ba7da6256b23
    Change  : +8608.200 (1.91% / z=11.486)
    Graph   : http://mzl.la/yIYdKc 

Regression :( Trace Malloc MaxHeap increase 1.66% on CentOS (x86_64) release 5 (Final) Mozilla-Inbound
------------------------------------------------------------------------------------------------------
    Previous: avg 36321716.667 stddev 5774.085 of 30 runs up to revision b74bbbfafde3
    New     : avg 36924800.000 stddev 9138.928 of 5 runs since revision ba7da6256b23
    Change  : +603083.333 (1.66% / z=104.447)
    Graph   : http://mzl.la/ynu1ix  

Regression :( Trace Malloc MaxHeap increase 1.73% on WINNT 5.2 Mozilla-Inbound
------------------------------------------------------------------------------
    Previous: avg 25351820.000 stddev 64804.211 of 30 runs up to revision b74bbbfafde3
    New     : avg 25790620.000 stddev 3439.041 of 5 runs since revision ba7da6256b23
    Change  : +438800.000 (1.73% / z=6.771)
    Graph   : http://mzl.la/ysjjdf 

Regression :( Tp5r MozAfterPaint (Private Bytes) increase 3.63% on Linux x64 Mozilla-Inbound-Non-PGO
----------------------------------------------------------------------------------------------------
    Previous: avg 422754200.000 stddev 2998802.490 of 30 runs up to revision b74bbbfafde3
    New     : avg 438108800.000 stddev 2696260.781 of 5 runs since revision ba7da6256b23
    Change  : +15354600.000 (3.63% / z=5.120)
    Graph   : http://mzl.la/A7ysNU 

Regression :( Paint increase 5.82% on Win7 Mozilla-Inbound
----------------------------------------------------------
    Previous: avg 155.514 stddev 1.319 of 30 runs up to revision b74bbbfafde3
    New     : avg 164.569 stddev 1.817 of 5 runs since revision c29f828d1ce4
    Change  : +9.054 (5.82% / z=6.864)
    Graph   : http://mzl.la/x4PUTX 

Regression :( Tp5r MozAfterPaint (Private Bytes) increase 3.48% on Linux x64 Mozilla-Inbound
--------------------------------------------------------------------------------------------
    Previous: avg 424051766.667 stddev 2622374.795 of 30 runs up to revision ad71757b3229
    New     : avg 438804000.000 stddev 1400962.883 of 5 runs since revision ba7da6256b23
    Change  : +14752233.333 (3.48% / z=5.626)
    Graph   : http://mzl.la/wHkZr6
Comment 65 Marco Bonardo [::mak] 2012-03-11 16:31:33 PDT
no way out from a plain backout for now. If anyone could do that would be appreciated, since I will be out tomorrow in the morning.

Actually the only real worrying thing is Paint increase 9.68% on Win7 Mozilla-Inbound-Non-PGO.  A bit more memory may be expected, and a small regression in Paint could exist (after all we are adding something to all the browser windows), but we should understand why Win7 is affected so much.
Comment 66 Ed Morley [:emorley] 2012-03-11 16:40:45 PDT
(In reply to Marco Bonardo [:mak] from comment #65)
> no way out from a plain backout for now. If anyone could do that would be
> appreciated, since I will be out tomorrow in the morning.

Backed out in:
https://hg.mozilla.org/integration/mozilla-inbound/rev/f0a006794b94

Thanks mak :-)
Comment 67 Daniel Holbert [:dholbert] 2012-03-11 19:47:59 PDT
Just for the record, here are the m-c csets for the landing & backout (both merged to m-c):
  https://hg.mozilla.org/mozilla-central/rev/ba7da6256b23
  https://hg.mozilla.org/mozilla-central/rev/f0a006794b94
Comment 68 :Paolo Amadini 2012-03-13 11:25:47 PDT
We worked in the past few days to track the performance issues, and we tried a
few more tests, but we couldn't get convincing results. In general we noticed
differences and anomalies that we couldn't explain clearly, that were often
platform-specific, and accounting for measurement noise on try builds was also
difficult.

So we've decided to change approach and we've booked a dedicated branch (cedar)
with continuous builds and tests, on which we'll re-create the feature
incrementally and see more clearly how the Talos tests evolve over time.
Comment 69 Guillaume C. [:ge3k0s] 2012-03-23 07:47:57 PDT
In the latest UX builds the download button shows nothing on hover.
Comment 70 Girish Sharma [:Optimizer] 2012-03-23 23:53:51 PDT
In the latest UX builds, there is an error in the css applied to the selected richlist item . In hover state, the hovered item has a kind of blue gradient background and black text. But the selected item has white text and no gradient background (i.e. white background) Thus nothing is visible until we hover it.
This is confirmed that the selected item has this problem by pressing the arrow keys and watching the text of the corresponding item getting white.

One more suggestion, on hovering/selecting the gradient is not covering the whole horizontal space, but leaving the search kinda icon with white background. This looks little inconsistent and ugly. IMHO, the gradient should cover the whole horizontal space.
Even the height of each item can be reduced by 10px. Its too wide.
Comment 71 :Paolo Amadini 2012-03-24 03:01:54 PDT
(In reply to Girish Sharma from comment #70)
> In the latest UX builds, there is an error in the css applied to the
> selected richlist item . In hover state, the hovered item has a kind of blue
> gradient background and black text. But the selected item has white text and
> no gradient background (i.e. white background) Thus nothing is visible until
> we hover it.
> This is confirmed that the selected item has this problem by pressing the
> arrow keys and watching the text of the corresponding item getting white.

We might have fixed this while working on recent improvements, I'll check out
the status of the UX build in the next few days.

> One more suggestion, on hovering/selecting the gradient is not covering the
> whole horizontal space, but leaving the search kinda icon with white
> background. This looks little inconsistent and ugly. IMHO, the gradient
> should cover the whole horizontal space.

This is part of the UX design we'll address once the first version is available.

> Even the height of each item can be reduced by 10px. Its too wide.

This also probably changed already.
Comment 72 :Paolo Amadini 2012-03-24 03:21:00 PDT
Created attachment 608980 [details] [diff] [review]
Other performance and styling changes
Comment 73 Guillaume C. [:ge3k0s] 2012-03-31 08:31:51 PDT
I've got two big bugs on latest UX build :
-The first download in the list only shows icon and progress bar.
-Arrow is not considered as a button.
Comment 74 bogas04 2012-03-31 08:44:20 PDT
third
-If a download is in progress , on closing firefox , there is no notification indicating about them.
Comment 75 Guillaume C. [:ge3k0s] 2012-03-31 10:26:15 PDT
(In reply to bogas04 from comment #74)
> third
> -If a download is in progress , on closing firefox , there is no
> notification indicating about them.

Not a bug. But it's a very good thing to implement.
Comment 76 Girish Sharma [:Optimizer] 2012-03-31 10:31:06 PDT
In your reply [1] , you said that the height of each download item has been reduced (as there was too much of padding) and also when hovering the item, the full width gets highlighted . But none of those change are visible in the latest build.

Also, the downloads panel's toolbar button (the down arrow button) should be of white color on glass, like all other buttons are.
Comment 77 rob64rock 2012-03-31 13:25:10 PDT
(In reply to Guillaume C. [:GE3K0S] from comment #75)
> (In reply to bogas04 from comment #74)
> > third
> > -If a download is in progress , on closing firefox , there is no
> > notification indicating about them.
> 
> Not a bug. But it's a very good thing to implement.

If you or someone else files a new bug# for that enhancement please post it's bug# here, Thanks.
Comment 78 Guillaume C. [:ge3k0s] 2012-04-01 09:32:38 PDT
(In reply to rob64rock from comment #77)
> (In reply to Guillaume C. [:GE3K0S] from comment #75)
> > (In reply to bogas04 from comment #74)
> > > third
> > > -If a download is in progress , on closing firefox , there is no
> > > notification indicating about them.
> > 
> > Not a bug. But it's a very good thing to implement.
> 
> If you or someone else files a new bug# for that enhancement please post
> it's bug# here, Thanks.

I think UX team and Paolo should seriously take this into consideration. The dialog should also have the option "don't show this dialog anymore".

I've also checked download arrow is the only button that doesn't act like one. Maybe it's because of the recently resolved bug about borderless button.
Comment 79 Constantine 2012-04-02 02:33:40 PDT
(In reply to rob64rock from comment #77)
> (In reply to Guillaume C. [:GE3K0S] from comment #75)
> > (In reply to bogas04 from comment #74)
> > > third
> > > -If a download is in progress , on closing firefox , there is no
> > > notification indicating about them.
> > 
> > Not a bug. But it's a very good thing to implement.
> 
> If you or someone else files a new bug# for that enhancement please post
> it's bug# here, Thanks.

I have filed Bug 741312 for it.
Comment 80 Vykintas Vyšniauskas 2012-04-07 01:14:45 PDT
In the current UX build, if a download fails, the context menu contains an entry to cancel the download instead of one for removing it. The only way to remove it is to clear all the list.
Comment 81 :Paolo Amadini 2012-04-12 10:36:04 PDT
Thank you to everyone that provided feedback and reported bugs! I will look into
them next week, now that the work on startup performance is almost finished.

Hopefully, we will be able to land the feature in Nightly in a short time. Even
if not all of the current issues will be necessarily fixed at that time, once the
feature lands we will be able to fix the issues incrementally, and more rapidly.

Similarly to what we planned for Firefox 13, even if the feature will land in
Nightly builds for Firefox 14, it be disabled by default in the Aurora channel.

When the feature lands, I will provide a more thorough status update, detailing
what to expect from the current state of the feature.
Comment 82 :Paolo Amadini 2012-04-12 10:41:38 PDT
Created attachment 614439 [details] [diff] [review]
Nearly optimal startup and shutdown performance

This version should provide near-zero startup and shutdown performance impact in
case there are no downloads in the panel, which is the most common case now, and
at all times when the feature is disabled.

A very little impact is expected for the code we cannot load later, like
setting up startup observers (that however don't do anything unless required).

Ideally we should share the delay load code with the bookmarks panel, but we
can do that in a follow-up.
Comment 83 Guillaume C. [:ge3k0s] 2012-04-16 03:54:33 PDT
Good news. However I think theming and visual design issues (bug 726447 ) are the most important things to fix before landing.
Comment 84 :Paolo Amadini 2012-04-16 09:55:48 PDT
I've verified the issue with the download indicator styling in the toolbars on
Windows. The indicator does not get any "hover" or "open" state when the toolbar
is in large icons mode, but works correctly in small icons mode.

This is probably due to the recent Australis design changes tracked in bug 734373
(maybe the borderless buttons implemented in bug 735691). It could be related to
bug 738304, affecting non-default buttons, though this should be investigated.

It's possible that when those bugs are fixed the indicator will be fixed as well.

To limit the possible regressions when the feature is disabled, I've made sure
that, in that case, we show the current downloads button (which is styled
correctly) instead of the downloads indicator in neutral state. This also fixes
an issue for which we would have needed an update of all the custom themes to
implement the stylesheets for the indicator, even with the feature disabled.

As long as the feature in disabled state doesn't regress, we are fine with making
the feature available in Nightly, even if there are known cosmetic issues when it
is enabled.
Comment 85 :Paolo Amadini 2012-04-16 10:01:23 PDT
Created attachment 615371 [details] [diff] [review]
Use the current Downloads button when the feature is disabled

Another update of the patch. I forgot to mention that in the last patch I also
removed the test requiring focusing the panel, because on the latest trunk it
failed intermittently on my machine (even with the changes inherited from the
UX branch). We can add it back in a follow-up when we figure out more precisely
how the panel and list box focus work.
Comment 86 Dão Gottwald [:dao] 2012-04-16 11:18:52 PDT
(In reply to Paolo Amadini [:paolo] from comment #84)
> It could be related to bug 738304, affecting non-default buttons,

Not really, no. The styling is broken because your button's internal structure is unlike the structure of standard toolbar buttons.
Comment 87 :Paolo Amadini 2012-04-16 11:50:35 PDT
(In reply to Dão Gottwald [:dao] from comment #86)
> (In reply to Paolo Amadini [:paolo] from comment #84)
> > It could be related to bug 738304, affecting non-default buttons,
> 
> Not really, no. The styling is broken because your button's internal
> structure is unlike the structure of standard toolbar buttons.

Thank you very much for the indication! So, I verified that the styling issue can
be solved nicely by assigning the "toolbarbutton-icon" class to the dynamic
element representing the button's contents. No need to change the browser's CSS.
Comment 88 Marco Bonardo [::mak] 2012-04-17 02:59:23 PDT
Comment on attachment 615371 [details] [diff] [review]
Use the current Downloads button when the feature is disabled

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

the load overlay object is a bit confusing with that re-enqueing, but not a blocking thing.

::: browser/base/content/browser.js
@@ +1641,5 @@
>      }
>  #endif
>    }, 10000);
>  
> +  // The object hanlding the downloads indicator is also initialized here in the

typo: hanlding

::: browser/components/downloads/content/downloads.js
@@ +66,5 @@
> +  get kPanelHidden() 1,
> +  /** The panel will be shown as soon as possible. */
> +  get kPanelShowing() 2,
> +  /** The panel is open, though download data might still be loading. */
> +  get kPanelShown() 3,

actually, in a follow-up to avoid creating possible regressions, you may just track uninitialized and use panel.state to track other states (probably can figure out an alternative way to track uninitialized with some other internal tracker)

@@ +85,5 @@
> +  initialize: function DP_initialize(aCallback)
> +  {
> +    if (this._panelState != this.kPanelUninitialized) {
> +      DownloadsOverlayLoader.ensureOverlayLoaded(
> +                             this.kDownloadsOverlay, aCallback);

nit: reindent (you can just indent aCallback)
add a comment on why we still need to go through ensureOverlayLoaded

@@ +100,5 @@
> +
> +    // Now that data loading has eventually started, load the required XUL
> +    // elements and initialize our views.
> +    DownloadsOverlayLoader.ensureOverlayLoaded(
> +                           this.kDownloadsOverlay, function DP_I_callback() {

nit: reindent (just function on new line)
Comment 89 :Paolo Amadini 2012-04-17 04:34:21 PDT
Created attachment 615665 [details] [diff] [review]
Finally!

We're ready to land this version on the mozilla-incoming branch.
Comment 90 :Paolo Amadini 2012-04-17 04:38:26 PDT
Pushed to mozilla-inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/913f7811c068
Comment 91 :Paolo Amadini 2012-04-17 04:43:23 PDT
Again, the plan is to disable this feature for Firefox 14 in the Aurora channel,
by enabling the preference named "browser.download.useToolkitUI".

We might also keep it enabled for one week on the Aurora channel to get more
early feedback, like it was done for the New Tab page.
Comment 92 :Ehsan Akhgari 2012-04-17 08:13:50 PDT
This regressed Talos tests such as Tp5 MozAfterPaint and a bunch of others by 3-11%.  I think we should back this out for now and investigate the performance regressions.
Comment 93 Marco Bonardo [::mak] 2012-04-17 12:10:53 PDT
(In reply to Ehsan Akhgari [:ehsan] from comment #92)
> This regressed Talos tests such as Tp5 MozAfterPaint and a bunch of others
> by 3-11%.  I think we should back this out for now and investigate the
> performance regressions.

Where do you see a 11% tp5 regression?  I only see a couple private bytes regressions (max 3%, and only Linux, Windows is happy) and a couple 1% MaxHeap.
We can't expect to duplicate functionality (cause this is actually duping all the DM code) and add a whole new feature without a tiny memory hit, if we do, this is 99% wontfix, like a bunch of other features.  That said, we will investigate further (We already did, and found no reason in the code) using Cedar numbers we have, but don't think a backout will bring anything useful.
Comment 94 :Ehsan Akhgari 2012-04-17 12:25:30 PDT
(In reply to Marco Bonardo [:mak] from comment #93)
> (In reply to Ehsan Akhgari [:ehsan] from comment #92)
> > This regressed Talos tests such as Tp5 MozAfterPaint and a bunch of others
> > by 3-11%.  I think we should back this out for now and investigate the
> > performance regressions.
> 
> Where do you see a 11% tp5 regression?  I only see a couple private bytes
> regressions (max 3%, and only Linux, Windows is happy) and a couple 1%
> MaxHeap.
> We can't expect to duplicate functionality (cause this is actually duping
> all the DM code) and add a whole new feature without a tiny memory hit, if
> we do, this is 99% wontfix, like a bunch of other features.  That said, we
> will investigate further (We already did, and found no reason in the code)
> using Cedar numbers we have, but don't think a backout will bring anything
> useful.

Appologies, this was probably my mistake (I seem to have read the graphs wrong or something.)
Comment 95 Marco Bonardo [::mak] 2012-04-17 12:29:39 PDT
no worries, rather I should file a bug cause looks like Cedar is not properly running and reporting the new Tp5 Row MozAfterPaint SuperExtraMegaCool measures...
Comment 96 Marco Bonardo [::mak] 2012-04-18 05:16:49 PDT
https://hg.mozilla.org/mozilla-central/rev/913f7811c068

Please note:
- There are known bugs, see the blocked bugs and the dependencies in bug 564934 (we will probably unify all the deps in an "enable the new downaloads panel by default" bug.
- This will be disabled in Aurora until we consider the quality high enough, will stay enabled in Nightly to help figuring and fixing issues, can be disabled by setting browser.download.useToolkitUI to true.
Comment 97 Henrik Skupin (:whimboo) 2012-04-18 15:18:33 PDT
A couple of Litmus tests will have to be updated now with this feature has been landed.
Comment 98 Marco Bonardo [::mak] 2012-04-18 15:28:48 PDT
(In reply to Henrik Skupin (:whimboo) from comment #97)
> A couple of Litmus tests will have to be updated now with this feature has
> been landed.

note it will stay enabled only in Nightly, for now.
Comment 99 bogas04 2012-04-19 05:42:57 PDT
Not sure if already filled but here are few things NewDM should be fixed at and i do realize this is not the place to tell but can't file new bugs too :

1) NewDM should notify about on-going downloads when Firefox is closed.
2) The panel when with an empty list , looks weird , IMO it should mention "There are no downloads for the session" 
3) The text for time left seems very thin , is it using default font? or is it rendered in some other way?

Other than that it feels very modern and IMO the best DM i have ever seen.
Comment 100 Girish Sharma [:Optimizer] 2012-04-19 07:04:38 PDT
One regression (and an important one) caused by the new DM :

You cannot visit the directory for a downloaded file from a previous session, as the Downloads panel gets empty on each session end. You cannot know the directory of any file from previous session since the Library does not have a column for that or any button to open the containing folder.

Is there a bug or should I file one ?
Comment 101 Jim Jeffery not reading bug-mail 1/2/11 2012-04-19 07:48:36 PDT
(In reply to Girish Sharma [:Optimizer] from comment #100)
> One regression (and an important one) caused by the new DM :
> 
> You cannot visit the directory for a downloaded file from a previous
> session, as the Downloads panel gets empty on each session end. You cannot
> know the directory of any file from previous session since the Library does
> not have a column for that or any button to open the containing folder.
> 
> Is there a bug or should I file one ?

If the Downloads Arrow is showing, clicking on it will show your 'last download', clicking on the teeny-tiny magnifying glass will open the 'Containing Folder'

In the Library use the 'View' in the Menu - Add columns - check 'Visit Date', then you will see in the Library a date for all your downloads.  

Nothing really missing, just rearranged.
Comment 102 Girish Sharma [:Optimizer] 2012-04-19 08:46:30 PDT
(In reply to Jim Jeffery not reading bug-mail 1/2/11 from comment #101)
> In the Library use the 'View' in the Menu - Add columns - check 'Visit
> Date', then you will see in the Library a date for all your downloads.  
> 
> Nothing really missing, just rearranged.

Seriously ??
I was talking about the fact that in the Library, there is no way to know the downloads directory of any particular download. Where did I even intend to know the visit date of a download ?
Comment 103 Jordan Bates 2012-04-19 12:11:34 PDT
Icon doesn't show up in different Firefox themes -- for example, fxchrome https://www.google.com/url?sa=t&rct=j&q=&esrc=s&source=web&cd=1&ved=0CC4QFjAA&url=https%3A%2F%2Faddons.mozilla.org%2Fen-US%2Ffirefox%2Faddon%2Ffxchrome%2F&ei=wGKQT6r8Maq8iwKk-pG-Aw&usg=AFQjCNFCgA1JCoPObUYXuBiOLoELPrPcBw&sig2=zYFMAhrzeciBV8bWUwc-Pw&cad=rja --  despite the button still being there and there being downloads in it, effectively resulting in what looks at first glance to be wasted space. At this point it won't let me change the icon or background image with CSS (Stylish addon) either. Using the default theme works fine, and in these themes the icon is there and completely visible in the customize toolbars mode.
Comment 104 :Paolo Amadini 2012-04-20 08:40:08 PDT
There is a new feature status page detailing some of the known issues:

https://wiki.mozilla.org/User:P.A./Panel-based_Download_Manager/Status

Note that I'm listing only the most noticeable issues there, while all the other
known issues are in bugs blocking bug 564934. There is a query linked from the
page to see the list of all those bugs.
Comment 105 mlngr1960 2012-04-20 13:56:59 PDT
Where is the option to disable the new Downloads Panel and revert to the Original Downloads Window for all the users who don't like the new panel and prefer the extra versatility of the original window?

Perhaps I've missed it.  If not, then will you please make the new panel vs the original window a selectable option.  This is in keeping with the trend in browsers to provide additional new options to users while keeping the original options available.  It has proven to be much preferred over replacing one option with a different one.

Providing users selectable options keeps everybody happy, rather than making many users unhappy while pleasing some of the remaining users (there are always others who could care less, as we all know). ;)

After all, customizing (or personalizing) your experience -is- the big thing these days. :)

p.s. You can default whichever option you feel the most users will prefer.

Thanks!
Comment 106 :Felipe Gomes (needinfo me!) 2012-04-20 14:05:58 PDT
(In reply to mlngr1960 from comment #105)
> Where is the option to disable the new Downloads Panel and revert to the
> Original Downloads Window for all the users who don't like the new panel and
> prefer the extra versatility of the original window?

go to about:config and toggle browser.download.useToolkitUI
Comment 107 :Paolo Amadini 2012-04-24 14:28:45 PDT
Disabling the feature in Aurora is now tracked in bug 748541.
Comment 108 Simona B [:simonab ] -PTO- back Sept 5th 2013-10-21 00:32:19 PDT
(In reply to Henrik Skupin (:whimboo) from comment #97)
> A couple of Litmus tests will have to be updated now with this feature has
> been landed.

Tests that verifies the feature's functionality have been added in Moztrap starting Firefox 20. Removing the "in-litmus" flag.
Comment 109 Henrik Skupin (:whimboo) 2013-10-21 00:50:50 PDT
(In reply to Simona B [QA] from comment #108)
> Tests that verifies the feature's functionality have been added in Moztrap
> starting Firefox 20. Removing the "in-litmus" flag.

Please ensure to set the in-moztrap+ flag in those cases. As best even provide a link to the test. That would be very helpful for later found regressions.

Note You need to log in before you can comment on or make changes to this bug.