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 76886 9f28a8fec3cbd789ea67237e5780f382077967f1
parent 76885 e4b0baf5db728876dc26c44034b4ab7fc55e764a
child 76887 bd9dd7e4c7ff976e31ddbd14be211219573aa2a8
push id78
push userclegnitto@mozilla.com
push dateFri, 16 Dec 2011 17:32:24 +0000
treeherdermozilla-release@79d24e644fdd [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersneil
bugs668038
milestone9.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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__