Closed Bug 832471 Opened 7 years ago Closed 7 years ago

Library | Clear Downloads doesn't update remove all downloads from the view

Categories

(Firefox :: Downloads Panel, defect)

x86_64
Linux
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 21
Tracking Status
firefox20 --- verified

People

(Reporter: dholbert, Assigned: mak)

References

Details

Attachments

(1 file, 1 obsolete file)

Reporting on behalf of "testman" who was just on IRC. (I can reproduce the bug, too.)

STR:
 1. Have some download history. (I had 10+ downloads in my list when I triggered this; testman suggests 5+ downloads.)
 2. Click download icon, to the right of search bar, and pick "Show All Downloads"
 3. Click "Clear Downloads" on the window that appears (w/ title "Library")

ACTUAL RESULTS: Downloads list in the Library window isn't actually cleared.
EXPECTED RESULTS: Downloads list should be cleared.


If you close the Library window and reopen it by performing step 2 from STR, *then* the list is cleared. So it looks like we're just not updating the already-open UI.

Mozilla/5.0 (X11; Linux x86_64; rv:21.0) Gecko/20130116 Firefox/21.0
I just did that today and it worked... though it was broken some builds ago... do you happen to know which build he was on?
also errors from the console would have been useful :(
No idea what build he was on, but I'm on 20130116, per comment 0 (which is 2 days out of date -- not sure if that's as old as the old builds you mentioned in comment 1)
did you also reproduce the bug? could even be he was on Aurora, and we can't land some fixes since it's permaclosed from 2 days...
I did reproduce it (on nightly), yes.

I just reproduced this on an up-to-date nightly (on a different machine), too:
Mozilla/5.0 (X11; Linux x86_64; rv:21.0) Gecko/20130118 Firefox/21.0

Two entries were added to Error Console when I clicked "Clear Downloads":
{
Timestamp: 01/18/2013 01:32:43 PM
Warning: Empty string passed to getElementById().
Source File: chrome://global/content/bindings/richlistbox.xml
Line: 361
Timestamp: 01/18/2013 01:32:43 PM
Warning: Empty string passed to getElementById().
Source File: chrome://global/content/bindings/richlistbox.xml
Line: 366
}
no idea how that may happen, but blocking for investigation.
I can now reproduce locally, trying to figure it out, thanks.
Assignee: nobody → mak77
Status: NEW → ASSIGNED
So the problem is in invalidateContainer

    for (let element of this._richlistbox.childNodes) {
      if (element._shell.placesNode)
        this._removeHistoryDownloadFromView(element._shell.placesNode);
    }

_removeHistoryDownloadFromView invokes removeChild()
looks like there may be another problem too, we cause select events on inactive items and onSelect throws, we should probably check if the shell is active before trying to fetchTargetInfo
It's quite strange I was unable to reproduce on my profile :/
Attached patch patch v1.0 (obsolete) — Splinter Review
Attachment #704151 - Flags: review?(mano)
we need a test harness soon... Maybe we may ask help to appcoast to write these tests, since we hardly have enough resources. Will evaluate with Gavin once he's back.
Flags: in-testsuite?
Comment on attachment 704151 [details] [diff] [review]
patch v1.0

>   onSelect: function DPV_onSelect() {
>     goUpdateDownloadCommands();
> 
>     let selectedElements = this._richlistbox.selectedItems;
>     for (let elt of selectedElements) {
>-      if (elt._shell)
>+      if (elt._shell.active)
>         elt._shell.onSelect();

Let's do the active check in DES_onSelect.
Attachment #704151 - Flags: review?(mano) → review+
Attached patch patch v1.1Splinter Review
Attachment #704151 - Attachment is obsolete: true
Summary: Library | Clear Downloads doesn't update UI → Library | Clear Downloads doesn't update remove all downloads from the view
Comment on attachment 704194 [details] [diff] [review]
patch v1.1

[Approval Request Comment]
Bug caused by (feature/regressing bug #): downloads panel feature
User impact if declined: Library Downloads view is broken after Clear Downloads
Testing completed (on m-c, etc.): m-i, local
Risk to taking this patch (and alternatives if risky): limited to the feature
String or UUID changes made by this patch: none
Attachment #704194 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/a523c072eced
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
I get this on Aurora 20 and the list is not cleared even after reopening.
That's a different bug, then. (This bug is just about the view not being updated immediately.)

I think mak mentioned a related bug which affected aurora but not nightly -- he mentioned that something "was broken some builds ago" in comment 1. mak, do you know what bug aceman might be seeing?  (if not, I imagine aceman should file a new bug?)
(In reply to Daniel Holbert [:dholbert] from comment #17)
> I think mak mentioned a related bug which affected aurora but not nightly --
> he mentioned that something "was broken some builds ago" in comment 1. mak,
> do you know what bug aceman might be seeing?  (if not, I imagine aceman
> should file a new bug?)

Aurora was lacking some fixes that landed yesterday (those include a fix for an exception that should not fire but instead does, blocking some commands) and is currently lacking this one. I think he should try to reproduce the bug in the Aurora build after this bug lands in Aurora, or try to reproduce it with current Nightly.
Comment on attachment 704194 [details] [diff] [review]
patch v1.1

Approving on aurora as download panel a new feature in aurora and requesting help with verification once this lands.
Attachment #704194 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed on the latest Nightly and Aurora - all the downloads are cleared from the Library after the Clear downloads button or option from the context menu are selected. 

Verified by clearing more than 20 downloads on Windows 7, Ubuntu 12.10 and Mac OS X 10.8:

Mozilla/5.0 (Windows NT 6.1; rv:21.0) Gecko/20130122 Firefox/21.0 Build ID: 20130122030956
Mozilla/5.0 (Windows NT 6.1; rv:20.0) Gecko/20130123 Firefox/20.0 Build ID: 20130123042017

Mozilla/5.0 (X11; Linux i686; rv:21.0) Gecko/20130123 Firefox/21.0 Build ID: 20130123030907
Mozilla/5.0 (X11; Linux i686; rv:20.0) Gecko/20130123 Firefox/20.0 Build ID: 20130123042017

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:21.0) Gecko/20130123 Firefox/21.0 Build ID: 20130123030907
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.8; rv:20.0) Gecko/20130123 Firefox/20.0 Build ID: 20130123042017
Status: RESOLVED → VERIFIED
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.