Closed Bug 1280799 Opened 8 years ago Closed 8 years ago

"Clear Downloads" button is grayed out after download file is finished and/or cancelled

Categories

(Toolkit :: Downloads API, defect, P1)

47 Branch
defect

Tracking

()

VERIFIED FIXED
mozilla50
Iteration:
50.2 - Jul 4
Tracking Status
firefox47 --- wontfix
firefox48 --- verified
firefox49 --- verified
firefox50 --- verified

People

(Reporter: mozilla, Assigned: jaws)

References

Details

(Keywords: nightly-community, Whiteboard: [fxprivacy])

Attachments

(2 files)

Attached image asdsdasdasd2.jpg
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:47.0) Gecko/20100101 Firefox/47.0
Build ID: 20160604131506

Steps to reproduce:

Try to download a file and open the downloads window, then cancel the download.


Actual results:

The clear button stays grayed out.


Expected results:

The clear button should be working as it was before in previous Firefox versions.  I noticed you can close the downloads window, re-open it and then the clear downloads button is functional again as normal.  Looks like the event handler for downloads list item cancel buttons does not refresh the clear downloads button state.
Confirmed with those STR on Windows 10 with Firefox 47. This was working with Firefox 45.0.1.
Thanks for reporting! Let me add a few details below to help people reproduce the issue.

I can also reproduce on OS X 10.11 using latest Nightly.

Detailed STR, with a new profile:


1. Start a download, for instance http://www.ubuntu.com/download/desktop/thank-you?country=FR&version=16.04&architecture=amd64
2. Click on "Show all downloads" to open download manager
3. Click on the "x" from the download manager to cancel current download (e.g don’t cancel from the download panel in the toolbar)

Actual results:
"Clear downloads" button remains grayed out.

Expected results:
We should be able to click on "Clear downloads" button.
Status: UNCONFIRMED → NEW
Has Regression Range: --- → no
Has STR: --- → yes
Component: Untriaged → Download Manager
Ever confirmed: true
Product: Firefox → Toolkit
Mozregression seem to blame bug 1117145. Paolo, do you think this could have caused this bug?
Blocks: 1117145
Has Regression Range: no → yes
Flags: needinfo?(paolo.mozmail)
Thanks Théo for triaging and complementarizing my bug report :-)
Is bug 1280799 related? I'm also seeing the button disabled in FF 47 when it shouldn't be.
See Also: → 1276973
Uhh... you linked this very page, Jorg lol?
The bug added to the "see also" field (bug 1276973) might be related indeed.
Flags: needinfo?(paolo.mozmail)
(In reply to Théo Chevalier [:tchevalier] from comment #2)
> Mozregression seem to blame bug 1117145. Paolo, do you think this could have
> caused this bug?

And yes, this could be the bug that introduced the regression.
Whiteboard: [fxprivacy][triage]
(In reply to that-ben from comment #5)
> Uhh... you linked this very page, Jorg lol?
Sorry: Is bug 1276973 related.
Priority: -- → P3
Whiteboard: [fxprivacy][triage] → [fxprivacy]
See Also: 1276973
Summary: "Clear Downloads" button is grayed out after cancelling a download file → "Clear Downloads" button is grayed out after download file is finished and/or cancelled
Just to clarify, this same buggy behavior is also seen by just downloading a file and waiting until the download is complete, don't even have to cancel it.  Just let it finish normally and the clear downloads button is still grayed out.
Now that we established that bug 1117145 caused this bug 1280799 and the related bug 1276973 (Loic, why did you clear my "See also"?), what are the plans to fix it? Sadly the bug has gone unnoticed during Aurora and Beta, so it would be good to get it fixed soon according to the "ship no regressions" policy.
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(jaws)
See Also: → 1276973
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #10)
> Now that we established that bug 1117145 caused this bug 1280799 and the
> related bug 1276973 (Loic, why did you clear my "See also"?), what are the
> plans to fix it? Sadly the bug has gone unnoticed during Aurora and Beta, so
> it would be good to get it fixed soon according to the "ship no regressions"
> policy.

During our "[fxprivacy]" team triage meeting we acknowledged that this is a regression caused by work our team did, and concluded that it's not important enough to be fixed at least until our current Notifications campaign ends, which is currently scheduled for Release 55.

We may always pick this bug on individual initiative if we're ahead of schedule, but the plan of record is that this bug will not be fixed at least until Release 55, unless other teams take responsibility for it. I don't necessarily think this is the best outcome, but it accurately documents our collective decision. This is indicated by the P3 in the "priority" field, we have indeed accumulated a good number of P3 bugs by now:

https://wiki.mozilla.org/Firefox/privacyandsecurity#Product_Backlog
Flags: needinfo?(paolo.mozmail)
Flags: needinfo?(jaws)
(In reply to :Paolo Amadini from comment #11)
> I don't
> necessarily think this is the best outcome, but it accurately documents our
> collective decision.
Hmm, how does that fit in with the "ship no regressions" policy? I think this is a terrible decision. Something goes bust that is very visible, puzzling and quite annoying, and the user has to wait eight cycles, so a little short of a year, to see it fixed? Don't you believe that Firefox will lose users due to such policy?

I'm from the Thunderbird team (actually, Thunderbird and Mailnews peer and Thunderbird Council member). If we break something so obvious, we bend over backwards to fix it as soon as possible. And we don't even have paid staff.

How hard can it be to have the button enabled when it should be? I haven't looked into it, but is it more than checking the length of the list and enable the button when the list has entries which are currently not downloading (completed, failed or cancelled)?

Hey, I can set/clear the disabled on the "clearDownloadsButton" using the DOM inspector ;-)
I'd like to clarify a few things. First, this is an acknowledged bug and therefore one we are determined to fix. The timing of the fix is a more complicated matter, as the individuals with the most experience in this specific area of the code are few and in high demand. There are other bugs we are trying to fix at the moment with even higher priority than this one, since it happens only in secondary UI (not the downloads panel), does not result in data/privacy/security loss or permanent breakage, and has a workaround (clear all history from the context menu). 

We clearly failed the policy to ship no regressions, since this bug is already in 47. But that's life, similar issues occurred in the past and I am sure they will happen again. Looking constructively at this specific bug, what we need is someone with the time and inclination to fix it. If the assessment from comment 12 is correct it shouldn't even be too hard to do. I can promise that this team will try to fix it as soon as other more important problems are dealt with, but in the traditional spirit of open source, patches are more than welcome!
Philip and rsx11m, our Firefox friends broke something and now they don't have to resources to fix it. In the DOM inspector I can see that the "Clear Downloads" button has ID clearDownloadsButton. If it's disabled, it has an attribute disabled=true. Maybe you S/M guys know more about the FF internals than we TB people. S/M has a downloads panel and a "Clear List" button. I looked (https://dxr.mozilla.org/mozilla-central/search?q=clearDownloadsButton) but I couldn't see where the disabled attribute is set. Could you please take a look.
Flags: needinfo?(rsx11m.pub)
Flags: needinfo?(philip.chee)
(In reply to Jorg K (GMT+2, PTO during summer, NI me) from comment #12)
> How hard can it be to have the button enabled when it should be? I haven't
> looked into it, but is it more than checking the length of the list and
> enable the button when the list has entries which are currently not
> downloading (completed, failed or cancelled)?

Exactly what's on my mind...
Assignee: nobody → jaws
Status: NEW → ASSIGNED
Flags: needinfo?(rsx11m.pub)
Flags: needinfo?(philip.chee)
Attachment #8764693 - Flags: review?(paolo.mozmail) → review+
Comment on attachment 8764693 [details]
Bug 1280799 - Update the state of the 'Clear Downloads' button when download item state changes, even if the item is selected.

https://reviewboard.mozilla.org/r/60404/#review57330
This patches browser/components/downloads/content/allDownloadsViewOverlay.js rather than toolkit/, thus apparently the other applications will have to do the same on their end.
Blocks: 1276973
https://hg.mozilla.org/integration/fx-team/rev/6de5b59dbcf95bb561d3c60f63743311f3f5d653
Bug 1280799 - Update the state of the 'Clear Downloads' button when download item state changes, even if the item is selected. r=paolo
(In reply to rsx11m from comment #18)
> This patches browser/components/downloads/content/allDownloadsViewOverlay.js
> rather than toolkit/, thus apparently the other applications will have to do
> the same on their end.

I'm unfamiliar with other applications, but the proper fix here was in browser/ not toolkit/ so there isn't more that I could do. If other applications use the same code they can apply this patch to their codebase.
https://hg.mozilla.org/mozilla-central/rev/6de5b59dbcf9
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Iteration: --- → 50.2
Flags: qe-verify?
Priority: P3 → P1
Comment on attachment 8764693 [details]
Bug 1280799 - Update the state of the 'Clear Downloads' button when download item state changes, even if the item is selected.

Approval Request Comment
[Feature/regressing bug #]: Bug 1117145.
[User impact if declined]: very visible, puzzling and quite annoying behaviour
[Describe test coverage new/current, TreeHerder]: Manual test.
[Risks and why]: Very low, one line of JS was moved.
[String/UUID change made/needed]: None.
Attachment #8764693 - Flags: approval-mozilla-release?
Attachment #8764693 - Flags: approval-mozilla-beta?
Attachment #8764693 - Flags: approval-mozilla-aurora?
Hello, could you please verify this issue is fixed as expected on a latest Nightly build (6/26 on wards)? Thanks!
Flags: needinfo?(mozilla)
Hi Florin, Andrei, could you please verify this fix works well? I might consider including it in the 47.0.1 release. Please let me know. Thanks!
Flags: needinfo?(florin.mezei)
Flags: needinfo?(andrei.vaida)
With Nightly of 26 June 2016 this is fixed:
Open downloads window, start a download, cancel download, "Clear downloads" button is available again.
Comment on attachment 8764693 [details]
Bug 1280799 - Update the state of the 'Clear Downloads' button when download item state changes, even if the item is selected.

This patch fixes the gray out "Clear Downloads" button and is verified. Take it in 48 beta 4 and aurora.
Attachment #8764693 - Flags: approval-mozilla-beta?
Attachment #8764693 - Flags: approval-mozilla-beta+
Attachment #8764693 - Flags: approval-mozilla-aurora?
Attachment #8764693 - Flags: approval-mozilla-aurora+
I looked at STR and was unable to repro the original issue on win10 47 06/06 build. Also given that there is a workaround (close/reopen the downloads window) I am leaning towards wontfixing this for 47.
Attachment #8764693 - Flags: approval-mozilla-release? → approval-mozilla-release-
I investigated this issue and managed to reproduce it on Nightly 2016-06-19 using
- Windows 10 x64
- Ubuntu 14.04 x64
- Mac OS X 10.11
The Fix has successfully landed and it works as expected on latest Nightly 50.0a1 (2016-06-27) on above mentioned OSs. Also, I found no misbehavior, except Bug 1276973. So, I will set the flags accordingly.
Status: RESOLVED → VERIFIED
Clearing the needinfo for me and Andrei. The qe-verify+ flag stands for additional verification in Beta 48.
Flags: needinfo?(florin.mezei)
Flags: needinfo?(andrei.vaida)
Flags: qe-verify? → qe-verify+
QA Contact: iulia.cristescu
I verified this issue on latest Aurora 49.0a2 (2016-06-28) and 48.0b4 build1 (20160627144420), too, using
- Windows 10 x64
- Ubuntu 14.04 x86
- Mac OS X 10.11
The fix looks good and I found no misbehavior, except Bug 1276973.
Flags: needinfo?(mozilla)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: