User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.9pre) Gecko/2008041907 Minefield/3.0pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; it; rv:1.9pre) Gecko/2008042303 Minefield/3.0pre After the restore of the 'Clear List' button on the download manager with the last patch of the bug 400495 , it hasn't an icon anymore, it is on the left side of the searchbar (instead of the right one) and it is always activated. Reproducible: Always Expected Results: 1. It needs an icon 2. It have to be on the right side of the searchbar 3. It have not to be always activated To see the correct behavior install on Firefox 3 Beta 5 this addon https://addons.mozilla.org/it/firefox/addon/6945

I don't agree it should be on the right side. But the other 2 points seem valid.

Comment on attachment 317303 [details] [diff] [review] Patch for the third point (the button is always activated) hmm, I think there might e a better way to do this. Ed?

I don't agree it needs an icon, or that it needs to be on the right side. Disabling when appropriate is correct though, morphing for that.

This works perfectly on my computer (Win XP)

Comment on attachment 317350 [details] [diff] [review] Patch v2 Way too many calls than necessary. It will also enable clear list when doing a search that matches nothing. We can be conservative and show clear list only when there are items and make sure not to enable it when there's nothing. I'll have a patch with test.

Update the clear list button: 1) Building the list 2) Adding a single item to an already open list 3) Removing a single item from an already open list Delay the notification because otherwise when we have 0 items, the notification gets sent before we disable the clear list button... so the ui isn't technically "done". Reenable the test on windows because it was probably because of "file://dummy/file". Update the test to check for disabled states. Also update the test to use the ui-done notification (this was and old test before we added the notification to simplify testing). Fix up styling on linux and os x.

You have done a lot of not useful changes that don't work properly IMHO. I've taken the code of the patch v2 from firefox 2 and it works without any problems. However, it should update the clear list button when (firefox 2 and patch v2 did them): 1) Start the download manager 2) A download is completed 3) A download is canceled 4) A download is resumed 5) A download is removed (might be the last one) 6) A download is retried after it was canceled 7) Click on 'Clear list' button With patch v3 I've seen also that clear list button is always activated if there is only a download in progress on the download manager and nothing else.

Yes. I know that it'll show the clear list button as clickable when there are active downloads. It'll be clickable whenever there's at least 1 item shown. If the user sees 1 done and 1 active download vs just 1 active download... Vs searching for terms and finding nothing..

Ok, but why? I think it's better to implement it only when there are unactive downloads IMHO (like firefox 2). And why don't you replace button.disabled = !gDownloadsView.itemCount; with button.disabled = !gDownloadManager.canCleanUp; ? It sounds better :P

Ah, can you add a margin-top of 3px/4px for the button under win xp/vista? Thanks.

Also fix bug 430647.

Make the clear list clickable only if the backend says we can clean up as well as if we have items in the list. No downloads: not clickable Only non-active: clickable Non and active: clickable Only active: not clickable (fixed with v3.1) Search + no downloads: not clickable Search + only non-active: clickable Search + non + active: clickable Search + only active: clickable** This will incorrectly show the clear list as clickable if we have an active download plus a search that matches no inactive downloads. But I've already spent too much time on this, so I don't really care right now.

Er. the only active -> not clickable is fixed with 3.2 not 3.1.

Comment on attachment 317591 [details] [diff] [review] v3.2 r=sdwilsh for code and test changes. I don't know anything about the css stuff, so I'll leave that for mconnor.

The button is 1 pixel bigger than default (but will allow larger fonts to let it expand) and more space above it.

Comment on attachment 317591 [details] [diff] [review] v3.2 looks good, ship it.

http://hg.mozilla.org/cvs-trunk-mirror/index.cgi/rev/e8f7ad982adf http://hg.mozilla.org/mozilla-central/index.cgi/rev/e8f7ad982adf Checking in toolkit/mozapps/downloads/content/downloads.js; /cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js new revision: 1.150; previous revision: 1.149 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.26; previous revision: 1.25 done Checking in toolkit/mozapps/downloads/tests/browser/browser_cleanup_search.js; /cvsroot/mozilla/toolkit/mozapps/downloads/tests/browser/browser_cleanup_search.js,v <-- browser_cleanup_search.js new revision: 1.3; previous revision: 1.2 done Checking in toolkit/themes/gnomestripe/mozapps/downloads/downloads.css; /cvsroot/mozilla/toolkit/themes/gnomestripe/mozapps/downloads/downloads.css,v <-- downloads.css new revision: 1.13; previous revision: 1.12 done Checking in toolkit/themes/pinstripe/mozapps/downloads/downloads.css; /cvsroot/mozilla/toolkit/themes/pinstripe/mozapps/downloads/downloads.css,v <-- downloads.css new revision: 1.31; previous revision: 1.30 done Checking in toolkit/themes/winstripe/mozapps/downloads/downloads.css; /cvsroot/mozilla/toolkit/themes/winstripe/mozapps/downloads/downloads.css,v <-- downloads.css new revision: 1.33; previous revision: 1.32 done

Verified FIXED: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9pre) Gecko/2008042704 Minefield/3.0pre Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9pre) Gecko/2008042704 Minefield/3.0pre -and- Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008042705 Minefield/3.0pre Filed bug 431105 to address the last paragraph in comment 13. in-litmus+: https://litmus.mozilla.org/show_test.cgi?id=5285