With the check-in of the patches on bug 403704 and bug 403707 we match the latest spec for in-progress/paused downloads. But the menu item "Open Containing Folder" / "Show in Finder" is grayed out. We should also enable the item while a download isn't finished yet. As I can see on the latest mockup the item isn't disabled: http://people.mozilla.com/~madhava/files/downloadmanager/download%20manager%202007-11-14.png

How are you downloading this? It's quite possible that the file doesn't yet exist, so it should be grayed out.

Well, I figured we could at least show the directory. Is there a way to reveal a file and have the directory shown but not the file selected? Is that the default behavior? This would allow the user to see the directory the partial file is being downloaded as well as be ready when the file finishes.

(In reply to comment #1) > How are you downloading this? It's quite possible that the file doesn't yet > exist, so it should be grayed out. It opens the folder where the file is download to. We don't open the file with this menu item. So why preventing the user to open the folder? Edward still points out all I wanted to say.

Make it always usable and have it do the right function. The original code was causing issues on OS X prompting for an application to handle it... so I made it do launch on the parent.

Is it right, that the return value of onDownloadDblClick is used to enable/disable the context menu entry? It's really hard to bind both things together. Edward, can you create a cvs diff?

I didn't understand either of those 2 questions.. ?? onDownloadDblClick doesn't return anything and a command is enabled/disabled with gDownloadViewController's isCommandEnabled function. cvs diff of what? instead of the hg diff patch that I attached?

Sorry, I missed the "@@ -669,13 +678,12 @@ var gDownloadViewController = {" line and thought your patch changes code within onDownloadDblClick(aEvent). But now it's clear. Yes, I meant cvs instead of hg but I'll wait until it's fixed now.

Comment on attachment 293484 [details] [diff] [review] v1 >+ // "Double click" the parent directory to show where the file should be >+ parent.launch(); parent should be a nsIFile, so you'll need to QI it to nsILocalFile to be able to call launch. r=sdwilsh with that fixed.

It works without QIing.. so I just add in.. ? let parent = f.parent.QueryInterface(Ci.nsILocalFile)

(In reply to comment #8) > >+ parent.launch(); > parent should be a nsIFile, so you'll need to QI it to nsILocalFile to be able > to call launch. Added in QueryInterface.

I don't understand why that works, but as far as I know we need the QI!

Comment on attachment 293624 [details] [diff] [review] v1.1 Just checking with the man, though I think that this is right ...

Comment on attachment 293624 [details] [diff] [review] v1.1 a=beltzner for 1.9, and I'm pretty sure it's the right thing to do from the UI perspective

Checking in toolkit/mozapps/downloads/content/downloads.js; /cvsroot/mozilla/toolkit/mozapps/downloads/content/downloads.js,v <-- downloads.js new revision: 1.119; previous revision: 1.118 done

https://litmus.mozilla.org/show_test.cgi?id=5051 in-litmus+

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2007121905 Minefield/3.0b3pre