Closed Bug 830205 Opened 7 years ago Closed 7 years ago
Download button shows progress remaining after download finishes
Setting browser.download.panel.removeFinishedDownloads = true breaks the download button in various ways: * The finished notification animation isn't displayed * The button continues to should some amount of time/progress left even through the download has finished. I'm not sure when this started happening, but it should probably be fixed for Firefox 20 if it is going to be enabled by default. STR 1. Set browser.download.panel.removeFinishedDownloads = true 2. Download a file Expected results: When the download finishes, the button turns green and the notification animation is displayed. Actual results: When the download finishes, the button continues to display some time remaining and/or the progress bar. Clicking the button resets it to the default appearance.
I agree, a simple workaround would be to just not support that pref for 20, in case a proper fix is complicated. Let's see.
I think the first issue is minor, though the wrong progressbar is not.
Confirmed on Ubuntu and OSX. Investigating...
Assignee: nobody → mconley
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Linux → All
It looks like when browser.download.panel.removeFinishedDownloads is true, the download is removed *during* the state change from in-progress to complete, and so we get notified of its removal *before* we hear about it changing state to complete. I believe this confuses our indicator code, since it gets a state change update when there are no downloads underway. This patch makes the indicator data provider a little smarter in that it wipes out the counter and progress values when there are no more downloads in progress. This doesn't, however, cause the indicator to get attention when the download completes - and this might be fine, since I don't know if we really want the download indicator to get attention on a completed download, since the button itself won't display anything useful after clicking it.
This seems to fix the bug.
The fact is that, for the indicator only, we enumerate the dataItems directly from the DownloadsData object (without keeping our own copy of the list) during the removal notification. But, we invoke the removal notification before we actually removed the dataItem from the list. Probably, moving the removal as the first line in _removeDataItem, before onDataItemRemoved is called, will fix the issue. I then suggest double-checking that the Library view isn't regressed by this change, but I think not because it properly keeps its own copy of the dataItems, it doesn't seem to use the global list.
Comment on attachment 702011 [details] [diff] [review] Patch v1.1 Mike, if the approach in comment 6 works, please go for it instead of this workaround - no need to ask for review again unless you just want more eyes on the new patch. Otherwise, feel free to land the workaround.
Attachment #702011 - Flags: review?(paolo.mozmail) → review+
This change does indeed fix the problem - thanks for clearing that up! Library doesn't seem to be affected at all.
Attachment #702011 - Attachment is obsolete: true
Landed Paolo's fix on mozilla-inbound as https://hg.mozilla.org/integration/mozilla-inbound/rev/c9e635c7c6eb
Comment on attachment 702460 [details] [diff] [review] Patch v2 (r+'d by paolo) [Approval Request Comment] Bug caused by (feature/regressing bug #): downloads panel feature User impact if declined: broken ui Testing completed (on m-c, etc.): m-i, merge pending Risk to taking this patch (and alternatives if risky): limited to the feature String or UUID changes made by this patch: none
Attachment #702460 - Flags: approval-mozilla-aurora?
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 21
Attachment #702460 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I confirm the fix is verified on FF 20.b1. Verified fixed due to Testday 20130222 too. Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20100101 Firefox/20.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20100101 Firefox/20.0
(In reply to MarioMi (:MarioMi) from comment #13) Verified fixed on FF21b1. Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20100101 Firefox/21.0 Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20100101 Firefox/21.0 Builds ID: 20130401192816
You need to log in before you can comment on or make changes to this bug.