Bug 668038 - nsFilePicker returned filenames are not canonicalized. r=neil
authorBrian R. Bondy <netzen@gmail.com>
Fri, 19 Aug 2011 13:55:03 -0400
changeset 75575 9f28a8fec3cbd789ea67237e5780f382077967f1
parent 75574 e4b0baf5db728876dc26c44034b4ab7fc55e764a
child 75576 bd9dd7e4c7ff976e31ddbd14be211219573aa2a8
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersneil
bugs668038
milestone9.0a1
Bug 668038 - nsFilePicker returned filenames are not canonicalized. r=neil
widget/src/windows/nsFilePicker.cpp
widget/src/windows/nsFilePicker.h
--- a/widget/src/windows/nsFilePicker.cpp
+++ b/widget/src/windows/nsFilePicker.cpp
@@ -50,16 +50,17 @@
 #include "nsFilePicker.h"
 #include "nsILocalFile.h"
 #include "nsIURL.h"
 #include "nsIStringBundle.h"
 #include "nsEnumeratorUtils.h"
 #include "nsCRT.h"
 #include <windows.h>
 #include <shlobj.h>
+#include <shlwapi.h>
 
 // commdlg.h and cderr.h are needed to build with WIN32_LEAN_AND_MEAN
 #include <commdlg.h>
 #include <cderr.h>
 
 #include "nsString.h"
 #include "nsToolkit.h"
 
@@ -138,18 +139,17 @@ static UINT_PTR CALLBACK FilePickerHook(
           requiredBufLength = CommDlg_OpenSave_GetFolderPathW(parentHWND, 
                                                               NULL, 0);
           if(requiredBufLength >= 0)
             newBufLength += requiredBufLength;
           else
             newBufLength += MAX_PATH;
 
           // Check if lpstrFile and nMaxFile are large enough
-          if (newBufLength > lpofn->lpOFN->nMaxFile)
-          {
+          if (newBufLength > lpofn->lpOFN->nMaxFile) {
             if (lpofn->lpOFN->lpstrFile)
               delete[] lpofn->lpOFN->lpstrFile;
 
             // We allocate FILE_BUFFER_SIZE more bytes than is needed so that
             // if the user selects a file and holds down shift and down to 
             // select  additional items, we will not continuously reallocate
             newBufLength += FILE_BUFFER_SIZE;
 
@@ -202,62 +202,55 @@ NS_IMETHODIMP nsFilePicker::ShowW(PRInt1
 
     BROWSEINFOW browserInfo;
     browserInfo.hwndOwner      = (HWND)
       (mParentWidget.get() ? mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : 0); 
     browserInfo.pidlRoot       = nsnull;
     browserInfo.pszDisplayName = (LPWSTR)dirBuffer;
     browserInfo.lpszTitle      = mTitle.get();
     browserInfo.ulFlags        = BIF_USENEWUI | BIF_RETURNONLYFSDIRS;
-    if (initialDir.Length())
-    {
+    if (initialDir.Length()) {
       // the dialog is modal so that |initialDir.get()| will be valid in 
       // BrowserCallbackProc. Thus, we don't need to clone it.
       browserInfo.lParam       = (LPARAM) initialDir.get();
       browserInfo.lpfn         = &BrowseCallbackProc;
-    }
-    else
-    {
+    } else {
     browserInfo.lParam         = nsnull;
       browserInfo.lpfn         = nsnull;
     }
     browserInfo.iImage         = nsnull;
 
     LPITEMIDLIST list = ::SHBrowseForFolderW(&browserInfo);
     if (list != NULL) {
       result = ::SHGetPathFromIDListW(list, (LPWSTR)fileBuffer);
       if (result) {
           mUnicodeFile.Assign(fileBuffer);
       }
   
       // free PIDL
       CoTaskMemFree(list);
     }
-  }
-  else 
-  {
+  } else {
 
     OPENFILENAMEW ofn;
     memset(&ofn, 0, sizeof(ofn));
     ofn.lStructSize = sizeof(ofn);
     nsString filterBuffer = mFilterList;
                                   
     if (!initialDir.IsEmpty()) {
       ofn.lpstrInitialDir = initialDir.get();
     }
     
     ofn.lpstrTitle   = (LPCWSTR)mTitle.get();
     ofn.lpstrFilter  = (LPCWSTR)filterBuffer.get();
     ofn.nFilterIndex = mSelectedType;
     ofn.hwndOwner    = (HWND) (mParentWidget.get() ? mParentWidget->GetNativeData(NS_NATIVE_TMP_WINDOW) : 0); 
     ofn.lpstrFile    = fileBuffer;
     ofn.nMaxFile     = FILE_BUFFER_SIZE;
-
-    ofn.Flags = OFN_NOCHANGEDIR | OFN_SHAREAWARE |
-                OFN_LONGNAMES | OFN_OVERWRITEPROMPT |
+    ofn.Flags = OFN_SHAREAWARE | OFN_LONGNAMES | OFN_OVERWRITEPROMPT |
                 OFN_HIDEREADONLY | OFN_PATHMUSTEXIST;
 
     // Handle add to recent docs settings
     nsCOMPtr<nsIPrivateBrowsingService> pbs =
       do_GetService(NS_PRIVATE_BROWSING_SERVICE_CONTRACTID);
     PRBool privacyModeEnabled = PR_FALSE;
     if (pbs) {
       pbs->GetPrivateBrowsingEnabled(&privacyModeEnabled);
@@ -287,16 +280,29 @@ NS_IMETHODIMP nsFilePicker::ShowW(PRInt1
           //XXX Actually, behavior is sort of weird:
           //    often appends ".html" even if you have an extension
           //    It obeys your extension if you put quotes around name
           ofn.lpstrDefExt = htmExt.get();
         }
       }
     }
 
+    // When possible, instead of using OFN_NOCHANGEDIR to ensure the current
+    // working directory will not change from this call, we will retrieve the
+    // current working directory before the call and restore it after the 
+    // call.  This flag causes problems on Windows XP for paths that are
+    // selected like  C:test.txt where the user is currently at C:\somepath
+    // In which case expected result should be C:\somepath\test.txt
+    AutoRestoreWorkingPath restoreWorkingPath;
+    // If we can't get the current working directory, the best case is to
+    // use the OFN_NOCHANGEDIR flag
+    if (!restoreWorkingPath.HasWorkingPath()) {
+      ofn.Flags |= OFN_NOCHANGEDIR;
+    }
+    
     MOZ_SEH_TRY {
       if (mMode == modeOpen) {
         // FILE MUST EXIST!
         ofn.Flags |= OFN_FILEMUSTEXIST;
         result = ::GetOpenFileNameW(&ofn);
       }
       else if (mMode == modeOpenMultiple) {
         ofn.Flags |= OFN_FILEMUSTEXIST | OFN_ALLOWMULTISELECT | 
@@ -373,44 +379,54 @@ NS_IMETHODIMP nsFilePicker::ShowW(PRInt1
           dirName.Append((PRUnichar)'\\');
         
         nsresult rv;
         while (current && *current && *(current + nsCRT::strlen(current) + 1)) {
           current = current + nsCRT::strlen(current) + 1;
           
           nsCOMPtr<nsILocalFile> file = do_CreateInstance("@mozilla.org/file/local;1", &rv);
           NS_ENSURE_SUCCESS(rv,rv);
+
+          // Only prepend the directory if the path specified is a relative path
+          nsAutoString path;
+          if (PathIsRelativeW(current)) {
+            path = dirName + nsDependentString(current);
+          } else {
+            path = current;
+          }
+
+          nsAutoString canonicalizedPath;
+          GetQualifiedPath(path.get(), canonicalizedPath);
           
-          rv = file->InitWithPath(dirName + nsDependentString(current));
+          rv = file->InitWithPath(canonicalizedPath);
           NS_ENSURE_SUCCESS(rv,rv);
           
           rv = mFiles.AppendObject(file);
           NS_ENSURE_SUCCESS(rv,rv);
         }
         
         // handle the case where the user selected just one
         // file.  according to msdn.microsoft.com:
         // If you specify OFN_ALLOWMULTISELECT and the user selects 
         // only one file, the lpstrFile string does not have 
         // a separator between the path and file name.
         if (current && *current && (current == fileBuffer)) {
           nsCOMPtr<nsILocalFile> file = do_CreateInstance("@mozilla.org/file/local;1", &rv);
           NS_ENSURE_SUCCESS(rv,rv);
           
-          rv = file->InitWithPath(nsDependentString(current));
+          nsAutoString canonicalizedPath;
+          GetQualifiedPath(current, canonicalizedPath);
+          rv = file->InitWithPath(canonicalizedPath);
           NS_ENSURE_SUCCESS(rv,rv);
           
           rv = mFiles.AppendObject(file);
           NS_ENSURE_SUCCESS(rv,rv);
         }
-      }
-      else {
-        // I think it also needs a conversion here (to unicode since appending to nsString) 
-        // but doing that generates garbage file name, weird.
-        mUnicodeFile.Assign(fileBuffer);
+      } else {
+        GetQualifiedPath(fileBuffer, mUnicodeFile);
       }
     }
     if (ofn.hwndOwner) {
       ::DestroyWindow(ofn.hwndOwner);
     }
   }
 
   if (result) {
@@ -572,16 +588,29 @@ void nsFilePicker::InitNative(nsIWidget 
                               const nsAString& aTitle,
                               PRInt16 aMode)
 {
   mParentWidget = aParent;
   mTitle.Assign(aTitle);
   mMode = aMode;
 }
 
+void 
+nsFilePicker::GetQualifiedPath(const PRUnichar *aInPath, nsString &aOutPath)
+{
+  // Prefer a qualified path over a non qualified path.
+  // Things like c:file.txt would be accepted in Win XP but would later
+  // fail to open from the download manager.
+  PRUnichar qualifiedFileBuffer[MAX_PATH];
+  if (PathSearchAndQualifyW(aInPath, qualifiedFileBuffer, MAX_PATH)) {
+    aOutPath.Assign(qualifiedFileBuffer);
+  } else {
+    aOutPath.Assign(aInPath);
+  }
+}
 
 NS_IMETHODIMP
 nsFilePicker::AppendFilter(const nsAString& aTitle, const nsAString& aFilter)
 {
   mFilterList.Append(aTitle);
   mFilterList.Append(PRUnichar('\0'));
 
   if (aFilter.EqualsLiteral("..apps"))
@@ -594,8 +623,25 @@ nsFilePicker::AppendFilter(const nsAStri
       filter.AppendLiteral(".*");
     mFilterList.Append(filter);
   }
 
   mFilterList.Append(PRUnichar('\0'));
 
   return NS_OK;
 }
+
+AutoRestoreWorkingPath::AutoRestoreWorkingPath() 
+{
+  DWORD bufferLength = GetCurrentDirectoryW(0, NULL);
+  mWorkingPath = new PRUnichar[bufferLength];
+  if (GetCurrentDirectoryW(bufferLength, mWorkingPath) == 0) {
+    mWorkingPath = NULL;
+  }
+}
+
+AutoRestoreWorkingPath::~AutoRestoreWorkingPath()
+{
+  if (HasWorkingPath()) {
+    ::SetCurrentDirectoryW(mWorkingPath);
+  }
+}
+
--- a/widget/src/windows/nsFilePicker.h
+++ b/widget/src/windows/nsFilePicker.h
@@ -39,16 +39,17 @@
  * ***** END LICENSE BLOCK ***** */
 
 #ifndef nsFilePicker_h__
 #define nsFilePicker_h__
 
 #include "nsILocalFile.h"
 #include "nsISimpleEnumerator.h"
 #include "nsCOMArray.h"
+#include "nsAutoPtr.h"
 
 #include "nsICharsetConverterManager.h"
 #include "nsBaseFilePicker.h"
 #include "nsString.h"
 #include "nsdefs.h"
 #include <windows.h>
 /**
  * Native Windows FileSelector wrapper
@@ -76,27 +77,42 @@ public:
   NS_IMETHOD ShowW(PRInt16 *aReturnVal); 
   NS_IMETHOD AppendFilter(const nsAString& aTitle, const nsAString& aFilter);
 
 protected:
   /* method from nsBaseFilePicker */
   virtual void InitNative(nsIWidget *aParent,
                           const nsAString& aTitle,
                           PRInt16 aMode);
-
-
+  static void GetQualifiedPath(const PRUnichar *aInPath, nsString &aOutPath);
   void GetFilterListArray(nsString& aFilterList);
 
   nsCOMPtr<nsIWidget>    mParentWidget;
   nsString               mTitle;
   PRInt16                mMode;
   nsCString              mFile;
   nsString               mDefault;
   nsString               mDefaultExtension;
   nsString               mFilterList;
   PRInt16                mSelectedType;
   nsCOMArray<nsILocalFile> mFiles;
   static char            mLastUsedDirectory[];
   nsString               mUnicodeFile;
   static PRUnichar      *mLastUsedUnicodeDirectory;
 };
 
+// The constructor will obtain the working path, the destructor
+// will set the working path back to what it used to be.
+class AutoRestoreWorkingPath
+{
+public:
+  AutoRestoreWorkingPath();
+  ~AutoRestoreWorkingPath();
+  inline bool AutoRestoreWorkingPath::HasWorkingPath() const
+  {
+    return mWorkingPath != NULL;
+  }
+
+private:
+  nsAutoArrayPtr<PRUnichar> mWorkingPath;
+};
+
 #endif // nsFilePicker_h__