Fix bug 189301 -- downloading files to dir with Japanese name doesn't work if showing progress dialog. Fix nsIDownloadManager idl to not trash non-ASCII UTF8 paths, and fix nsDownloadProxy.h to use a UTF8 path, rather than a native path. Other fixes noted in bug. r=bz, sr=jag

xpfe/components/download-manager/public/nsIDownloadManager.idl

@@ -83,41 +83,37 @@ interface nsIDownloadManager : nsISupports {
   /**
   *  Retrieves an in-progress download managed by the download manager.
   *
-  *  @param aPersistentDescriptor The unique identifier used to describe a
-  *                               a download, and an attribute of nsILocalFile.
-  *                               On Windows and Linux, this is just the path
-  *                               of the target, but on Mac this is guaranteed
-  *                               to be unique.
+  *  @param aTargetPath           A UTF8-encoded path to the target file.
   *
-  *  @return The download with the specified persistent descriptor.
+  *  @return The download with the specified path.
   */
 
-  nsIDownload getDownload(in string aPersistentDescriptor);
+  nsIDownload getDownload(in AUTF8String aTargetPath);
 
   /**
-  * Cancels the download with the specified persistent descriptor if it's
+  * Cancels the download with the specified target path if it's
   * currently in progress.  If a "persist" was specified for the download,
   * nsIWebBrowserPersist::CancelSave will be called.  If an observer was set
   * on the nsIDownload, it will be notified with the "oncancel" topic.  Clients
   * that don't provide a "persist" must listen for this topic and cancel the
   * download.
   *
-  * @param aPersistentDescriptor The persistent descriptor of the download to
+  * @param aTargetPath           The target path of the download to
   *                              be cancelled.
   */
 
-  void cancelDownload(in string aPersistentDescriptor);
+  void cancelDownload(in AUTF8String aTargetPath);
 
   /**
-  * Removes the download with the specified persistent descriptor if it's not
+  * Removes the download with the specified target path if it's not
   * currently in progress.  Whereas cancelDownload simply cancels the transfer
   * but retains information about it, removeDownload removes all knowledge of it.
   *
-  * @param aPersistentDescriptor The persistent descriptor of the download to
+  * @param aTargetPath           The target path of the download to
   *                              be removed.
   */
 
-  void removeDownload(in string aPersistentDescriptor);
+  void removeDownload(in AUTF8String aTargetPath);
 
   // UI-related methods
 
@@ -135,13 +131,13 @@ interface nsIDownloadManager : nsISupports {
   /**
   * Opens an individual progress dialog displaying progress for the download.
   *
-  * @param aPersistentDescriptor The persistent descriptor of the download to
+  * @param aTargetPath           The target path of the download to
   *                              display progress for.
   *
   * @param aParent The parent, or opener, of the front end (optional).
   */
 
-  void openProgressDialogFor(in string aPersistentDescriptor, in nsIDOMWindow aParent);
+  void openProgressDialogFor(in AUTF8String aTargetPath, in nsIDOMWindow aParent);
 
   /**
   * Called when the download manager front end is closed.  Useful for

xpfe/components/download-manager/resources/downloadmanager.js

@@ -163,7 +163,7 @@ var downloadViewController = {
 
     var selectedItem = getSelectedItem();
     var isDownloading = selectedItem && gDownloadManager.getDownload(selectedItem.id);
-    
+
     switch (aCommand) {
     case "cmd_openfile":
       try {
@@ -174,9 +174,9 @@ var downloadViewController = {
         return false;
       }      
     case "cmd_showinshell":
-      // some apps like kazaa/morpheus let you "preview" in-progress downloads because
-      // that's possible for movies and music. for now, just disable indiscriminately.
-      return selectionCount == 1;
+      // we can't reveal until the download is complete, because we have not given
+      // the file its final name until them.
+      return selectionCount == 1 && !isDownloading;
     case "cmd_properties":
       return selectionCount == 1 && isDownloading;
     case "cmd_pause":
@@ -204,7 +204,7 @@ var downloadViewController = {
     switch (aCommand) {
     case "cmd_properties":
       selectedItem = getSelectedItem();
-      if (selectedItem)
+      if (selectedItem && gDownloadManager.getDownload(selectedItem.id))
         gDownloadManager.openProgressDialogFor(selectedItem.id, window);
       break;
     case "cmd_openfile":
@@ -327,7 +327,7 @@ function getSelectedItems()
 
 function getFileForItem(aElement)
 {
-  var itemResource = gRDFService.GetResource(aElement.id);
+  var itemResource = gRDFService.GetUnicodeResource(aElement.id);
   var fileResource = gDownloadView.database.GetTarget(itemResource, gNC_File, true);
   fileResource = fileResource.QueryInterface(Components.interfaces.nsIRDFResource);
   return createLocalFile(fileResource.Value);

xpfe/components/download-manager/resources/downloadmanager.xul

@@ -48,11 +48,13 @@
 
   <script type="application/x-javascript" src="chrome://communicator/content/downloadmanager/downloadmanager.js"/>
 
-  <stringbundle id="dlMgrBundle"
-                src="chrome://communicator/locale/downloadmanager/downloadmanager.properties"/>
-  <stringbundle id="dlProgressDlgBundle"
-                src="chrome://global/locale/nsProgressDialog.properties"/>
-
+  <stringbundleset id="stringbundleset">
+    <stringbundle id="dlMgrBundle"
+                  src="chrome://communicator/locale/downloadmanager/downloadmanager.properties"/>
+    <stringbundle id="dlProgressDlgBundle"
+                  src="chrome://global/locale/nsProgressDialog.properties"/>
+  </stringbundleset>
+  
   <data id="strings.confirmCancel">&confirmCancel;</data>
   <data id="strings.progressMsgNoRate">&progressMsgNoRate;</data>
   <data id="strings.rateMsg">&rateMsg;</data>

xpfe/components/download-manager/resources/mac/jar.mn

@@ -1,3 +1,3 @@
 comm.jar:
    content/communicator/downloadmanager/contents.rdf (contents.rdf)
-   
+   content/communicator/downloadmanager/dlmanagermenuoverlay.xul (dlmanagermenuoverlay.xul)

xpfe/components/download-manager/src/nsDownloadManager.cpp

@@ -163,21 +163,21 @@ nsDownloadManager::Init()
 }
 
 nsresult
-nsDownloadManager::DownloadStarted(const char* aPath)
+nsDownloadManager::DownloadStarted(const nsACString& aTargetPath)
 {
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (mCurrDownloads.Exists(&key))
-    AssertProgressInfoFor(aPath);
+    AssertProgressInfoFor(aTargetPath);
 
   return NS_OK;
 }
 
 nsresult
-nsDownloadManager::DownloadEnded(const char* aPath, const PRUnichar* aMessage)
+nsDownloadManager::DownloadEnded(const nsACString& aTargetPath, const PRUnichar* aMessage)
 {
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (mCurrDownloads.Exists(&key)) {
-    AssertProgressInfoFor(aPath);
+    AssertProgressInfoFor(aTargetPath);
     mCurrDownloads.Remove(&key);
   }
 
@@ -246,7 +246,6 @@ nsDownloadManager::AssertProgressInfo()
 {
   nsCOMPtr<nsISupports> supports;
   nsCOMPtr<nsIRDFResource> res;
-  const char* uri;
   nsCOMPtr<nsIRDFInt> intLiteral;
 
   gRDFService->GetIntLiteral(DOWNLOADING, getter_AddRefs(intLiteral));
@@ -258,19 +257,20 @@ nsDownloadManager::AssertProgressInfo()
   downloads->HasMoreElements(&hasMoreElements);
 
   while (hasMoreElements) {
+    const char* uri;
     downloads->GetNext(getter_AddRefs(supports));
     res = do_QueryInterface(supports);
     res->GetValueConst(&uri);
-    AssertProgressInfoFor(uri);
+    AssertProgressInfoFor(nsDependentCString(uri));
     downloads->HasMoreElements(&hasMoreElements);
   }
   return rv;
 }
 
 nsresult
-nsDownloadManager::AssertProgressInfoFor(const char* aPath)
+nsDownloadManager::AssertProgressInfoFor(const nsACString& aTargetPath)
 {
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (!mCurrDownloads.Exists(&key))
     return NS_ERROR_FAILURE;
  
@@ -287,7 +287,7 @@ nsDownloadManager::AssertProgressInfoFor(const char* aPath)
   nsCOMPtr<nsIRDFResource> res;
   nsCOMPtr<nsIRDFLiteral> literal;
 
-  gRDFService->GetResource(aPath, getter_AddRefs(res));
+  gRDFService->GetResource(PromiseFlatCString(aTargetPath).get(), getter_AddRefs(res));
 
   DownloadState state;
   internalDownload->GetDownloadState(&state);
@@ -516,14 +516,14 @@ nsDownloadManager::AddDownload(nsIURI* aSource,
 }
 
 NS_IMETHODIMP
-nsDownloadManager::GetDownload(const char* aPath, nsIDownload** aDownloadItem)
+nsDownloadManager::GetDownload(const nsACString & aTargetPath, nsIDownload** aDownloadItem)
 {
   NS_ENSURE_ARG_POINTER(aDownloadItem);
 
   // if it's currently downloading we can get it from the table
   // XXX otherwise we should look for it in the datasource and
   //     create a new nsIDownload with the resource's properties
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (mCurrDownloads.Exists(&key)) {
     *aDownloadItem = NS_STATIC_CAST(nsIDownload*, mCurrDownloads.Get(&key));
     NS_ADDREF(*aDownloadItem);
@@ -535,10 +535,10 @@ nsDownloadManager::GetDownload(const char* aPath, nsIDownload** aDownloadItem)
 }
 
 NS_IMETHODIMP
-nsDownloadManager::CancelDownload(const char* aPath)
+nsDownloadManager::CancelDownload(const nsACString & aTargetPath)
 {
   nsresult rv = NS_OK;
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (!mCurrDownloads.Exists(&key))
     return NS_ERROR_FAILURE;
   
@@ -573,7 +573,7 @@ nsDownloadManager::CancelDownload(const char* aPath)
     if (NS_FAILED(rv)) return rv;
   }
   
-  DownloadEnded(aPath, nsnull);
+  DownloadEnded(aTargetPath, nsnull);
   
   // if there's a progress dialog open for the item,
   // we have to notify it that we're cancelling
@@ -589,9 +589,9 @@ nsDownloadManager::CancelDownload(const char* aPath)
 }
 
 NS_IMETHODIMP
-nsDownloadManager::RemoveDownload(const char* aPath)
+nsDownloadManager::RemoveDownload(const nsACString & aTargetPath)
 {
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   
   // RemoveDownload is for downloads not currently in progress. Having it
   // cancel in-progress downloads would make things complicated, so just return.
@@ -605,7 +605,7 @@ nsDownloadManager::RemoveDownload(const char* aPath)
   if (NS_FAILED(rv)) return rv;
   
   nsCOMPtr<nsIRDFResource> res;
-  gRDFService->GetResource(aPath, getter_AddRefs(res));
+  gRDFService->GetResource(PromiseFlatCString(aTargetPath).get(), getter_AddRefs(res));
 
   // remove all the arcs for this resource, and then remove it from the Seq
   nsCOMPtr<nsISimpleEnumerator> arcs;
@@ -735,10 +735,10 @@ nsDownloadManager::Open(nsIDOMWindow* aParent, nsIDownload* aDownload)
 }
 
 NS_IMETHODIMP
-nsDownloadManager::OpenProgressDialogFor(const char* aPath, nsIDOMWindow* aParent)
+nsDownloadManager::OpenProgressDialogFor(const nsACString & aTargetPath, nsIDOMWindow* aParent)
 {
   nsresult rv;
-  nsCStringKey key(aPath);
+  nsCStringKey key(aTargetPath);
   if (!mCurrDownloads.Exists(&key))
     return NS_ERROR_FAILURE;
 
@@ -853,13 +853,12 @@ nsDownloadManager::Observe(nsISupports* aSubject, const char* aTopic, const PRUn
       nsDownload* download = NS_STATIC_CAST(nsDownload*, mCurrDownloads.Get(&key));
       download->SetDialog(nsnull);
       
-      return CancelDownload(utf8Path.get());  
+      return CancelDownload(utf8Path);  
     }
   }
   else if (nsCRT::strcmp(aTopic, "quit-application") == 0) {
     nsCOMPtr<nsISupports> supports;
     nsCOMPtr<nsIRDFResource> res;
-    const char* uri;
     nsCOMPtr<nsIRDFInt> intLiteral;
 
     gRDFService->GetIntLiteral(DOWNLOADING, getter_AddRefs(intLiteral));
@@ -871,10 +870,12 @@ nsDownloadManager::Observe(nsISupports* aSubject, const char* aTopic, const PRUn
     downloads->HasMoreElements(&hasMoreElements);
 
     while (hasMoreElements) {
+      const char* uri;
+
       downloads->GetNext(getter_AddRefs(supports));
       res = do_QueryInterface(supports);
       res->GetValueConst(&uri);
-      CancelDownload(uri);
+      CancelDownload(nsDependentCString(uri));
       downloads->HasMoreElements(&hasMoreElements);
     }    
   }
@@ -901,7 +902,7 @@ nsDownload::~nsDownload()
   nsresult rv = mTarget->GetPath(path);
   if (NS_FAILED(rv)) return;
 
-  mDownloadManager->AssertProgressInfoFor(NS_ConvertUCS2toUTF8(path).get());
+  mDownloadManager->AssertProgressInfoFor(NS_ConvertUCS2toUTF8(path));
 }
 
 nsresult
@@ -1028,7 +1029,7 @@ nsDownload::OnProgressChange(nsIWebProgress *aWebProgress,
     if (NS_FAILED(rv)) return rv;
 
     mDownloadState = DOWNLOADING;
-    mDownloadManager->DownloadStarted(NS_ConvertUCS2toUTF8(path).get());
+    mDownloadManager->DownloadStarted(NS_ConvertUCS2toUTF8(path));
   }
 
   if (aMaxTotalProgress > 0)
@@ -1091,7 +1092,7 @@ nsDownload::OnStatusChange(nsIWebProgress *aWebProgress,
     nsAutoString path;
     nsresult rv = mTarget->GetPath(path);
     if (NS_SUCCEEDED(rv))
-      mDownloadManager->DownloadEnded(NS_ConvertUCS2toUTF8(path).get(), aMessage);
+      mDownloadManager->DownloadEnded(NS_ConvertUCS2toUTF8(path), aMessage);
   }
 
   if (mListener)
@@ -1176,7 +1177,7 @@ nsDownload::OnStateChange(nsIWebProgress* aWebProgress,
       rv = mTarget->GetPath(path);
       // can't do an early return; have to break reference cycle below
       if (NS_SUCCEEDED(rv)) {
-        mDownloadManager->DownloadEnded(NS_ConvertUCS2toUTF8(path).get(), nsnull);
+        mDownloadManager->DownloadEnded(NS_ConvertUCS2toUTF8(path), nsnull);
       }
     }
 

xpfe/components/download-manager/src/nsDownloadManager.h

@@ -83,9 +83,9 @@ protected:
   nsresult GetInternalListener(nsIDownloadProgressListener** aInternalListener);
   nsresult GetDataSource(nsIRDFDataSource** aDataSource);
   nsresult AssertProgressInfo();
-  nsresult AssertProgressInfoFor(const char* aPersistentDescriptor);
-  nsresult DownloadStarted(const char* aPersistentDescriptor);
-  nsresult DownloadEnded(const char* aPersistentDescriptor, const PRUnichar* aMessage);
+  nsresult AssertProgressInfoFor(const nsACString& aTargetPath);
+  nsresult DownloadStarted(const nsACString& aTargetPath);
+  nsresult DownloadEnded(const nsACString& aTargetPath, const PRUnichar* aMessage);
   PRBool MustUpdateUI() { if (mDocument) return PR_TRUE; return PR_FALSE; }
 
 private:

xpfe/components/download-manager/src/nsDownloadProxy.h

@@ -79,10 +79,12 @@ public:
     if (behavior == 0)
       return dm->Open(nsnull, this);
     if (behavior == 1) {
-      nsCAutoString path;
-      rv = aTarget->GetNativePath(path);
+      nsAutoString path;
+      rv = aTarget->GetPath(path);
       if (NS_FAILED(rv)) return rv;
-      return dm->OpenProgressDialogFor(path.get(), nsnull);
+
+      NS_ConvertUCS2toUTF8 utf8Path(path);
+      return dm->OpenProgressDialogFor(utf8Path, nsnull);
     }
     return rv;
   }