431188 - Clear list active / inactive state not working properly

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Damian Shaw [Quan]]

On:

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9pre) Gecko/2008042806 Minefield/3.0pre ID:2008042806

I get some really weird results, I have several Ubuntu ISO and .torrent files, some of them are still openable from the download manager, some are not (because they got deleted), but:

1) I search "Ub"
2) I get all my Ubuntu results
3) "Clear List" is inactive

Also if I:

1) Make any search which yields only non-openable downloads (move or deleted
2) "Clear List" is inactive

I can't work out how to duplicate the first result, but the 2nd is pretty easy and I can duplicate every time.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

Easiest solution is just to remove the "Clear List" option on all searches, IMO. Consistent, clear, perhaps limiting, but kills a lot of bugs dead.

Comment 2

[Ed Lee :Mardak]

Attachment: v1

Meh. If we don't find any downloads within the first search chunk, we set it to disabled.

Fix by updating the button when we finish populating the list as well. Has testcase too.

Comment 3

[Damian Shaw [Quan]]

I was chatting with Mossop on IRC and I'm now a little confused what the expected behavior is, to me it should simply be:

if search has 0 inactive downloads: inactive state
else: active state

Comment 4

[Mike Beltzner [:beltzner, not reading bugmail]]

This will not block the final release of Firefox 3. Any patch will need unit tests in order to be approved.

Comment 5

[Shawn Wilsher :sdwilsh]

Comment on attachment 318208 [details] [diff] [review]
v1

perhaps I'm missing something but how does the test advance through different states?

Comment 6

[Ed Lee :Mardak]

(In reply to comment #5)
> perhaps I'm missing something but how does the test advance through different
> states?
The switch statement's condition is a postincrement like the other tests. ;)

http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/tests/browser/browser_cleanup_search.js#104
http://mxr.mozilla.org/seamonkey/source/toolkit/mozapps/downloads/tests/browser/browser_multiword_search.js#100

Saves a testCase++ in each case body.

Comment 7

[Shawn Wilsher :sdwilsh]

right, but I don't see how the method keeps getting called.  Not sure how I didn't notice that in the other tests...

Comment 8

[Ed Lee :Mardak]

searchbox.doCommand() will rebuild the list and trigger notification
clearList.doCommand() will rebuild the list and trigger notification

Comment 9

[Shawn Wilsher :sdwilsh]

Comment on attachment 318208 [details] [diff] [review]
v1

duh.  r=sdwilsh (although you might want to add a comment to the test saying that)

Comment 10

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 318208 [details] [diff] [review]
v1

a1.9=beltzner

Comment 11

[Damian Shaw [Quan]]

Just curious, this going to make RC1?

Comment 12

[Ed Lee :Mardak]

http://hg.mozilla.org/cvs-trunk-mirror/index.cgi/rev/49b95e14b6a8
http://hg.mozilla.org/mozilla-central/index.cgi/rev/49b95e14b6a8

Checking in toolkit/mozapps/downloads/content/downloads.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v  <--  downloads.js
new revision: 1.151; previous revision: 1.150
done
Checking in toolkit/mozapps/downloads/tests/browser/Makefile.in;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/Makefile.in,v  <--  Makefile.in
new revision: 1.27; previous revision: 1.26
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_search_clearlist.js,v
done
Checking in toolkit/mozapps/downloads/tests/browser/browser_search_clearlist.js;
/cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_search_clearlist.js,v  <--  browser_search_clearlist.js
initial revision: 1.1
done

Comment 13

[Stephen Donner [:stephend] Not actively reading bugmail]

I spun off bug 432696 to cover the case where the "Clear List" button is enabled with an active download and no search matches.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Verified FIXED using:

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008050704 Minefield/3.0pre

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050706 Minefield/3.0pre

-and-

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008050707 Minefield/3.0pre

with bug 431105 noted.

Damian, if you figure out how to reproduce (and still see) the first part of comment 0, please file a new bug; I've verified that your second case is fixed.  Thanks!

Comment 15

[Damian Shaw [Quan]]

This patch fixes the first part of my bug :).

Comment 16

[Shawn Wilsher :sdwilsh]

We don't need a litmus test because we have automated test coverage.