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.

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.

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?

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 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.

(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.

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.

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.

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.

"the case if the button being removed" => "the case of the button being removed"

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 on attachment 596503 [details] Current tryserver build I'm OK with landing the current version on Nightly. And yes, it has visual bugs. :)

(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?

(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.

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

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 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.

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?

(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.

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

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.

(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.

And I say review comments need to be addressed regardless of some blocking classification.

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.

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.

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.

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

(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 :-)

If possible please keep the discussions in bug 564934, this is an implementation bug.

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 on attachment 597771 [details] [diff] [review] No migration code and other minor changes review attached, will flag when requirements from previous comment are satisfied

As a side note, if you want to meet up in person, to do final checks and fixes, we can adjust that.

Tryserver builds for compare-talos just started: https://tbpl.mozilla.org/?tree=Try&rev=ffb696a80bc7 https://tbpl.mozilla.org/?tree=Try&rev=a988a8a0e290

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.

(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.

(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 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.

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.

(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.

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.

This should give a measurable startup performance gain.

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.)

(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.

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?

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 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?

Latest patch talos results: http://tinyurl.com/7db9334

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.

https://hg.mozilla.org/integration/mozilla-inbound/rev/ba7da6256b23 Note: this feature will be disabled by default in Aurora.

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.

w00t! \o/

Was the tracking-firefox13 flag intentionally cleared? The post that set it and the post that cleared it were only a minute apart.

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

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.

(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 :-)

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

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.

In the latest UX builds the download button shows nothing on hover.

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.

(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.

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.

third -If a download is in progress , on closing firefox , there is no notification indicating about them.

(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.

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.

(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.

(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.

(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.

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.

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.

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.

Good news. However I think theming and visual design issues (bug 726447 ) are the most important things to fix before landing.

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.

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.

(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.

(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 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)

We're ready to land this version on the mozilla-incoming branch.

Pushed to mozilla-inbound: https://hg.mozilla.org/integration/mozilla-inbound/rev/913f7811c068

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.

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.

(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.

(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.)

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...

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.

A couple of Litmus tests will have to be updated now with this feature has been landed.

(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.

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.

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 ?

(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.

(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 ?

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.

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.

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!

(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

Disabling the feature in Aurora is now tracked in bug 748541.

(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.

(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.