Community
Participate
Working Groups
Created attachment 197310 [details] snippet that shows how FileDialog changes the working directory The standard Windows file dialog opened with GetSaveFileName has the nasty habit of changing the working directory of the process to the directory the user selects in the dialog. It is changed back after dialog closes, but operations run in different threads are messed up because of this. There is a snippet in the attachment that shows the problem. The result is the application writes and reads files to/from random locations. I did some tests and it seems that the Common File Dialog does not change the working directory while browsing for files. I used the sample code from http://archive.msdn.microsoft.com/shellapplication The new API is available only since Windows Vista. Currently, the only solution I see is to use Swing's JFileChooser, which has a sort of OK layout in XP, but it is far from looking native on Windows 7. As it is now, the FileDialog is for my application a bomb waiting to explode.
I am aware of the new APIs in Windows Vista, but we never had the time to rewrite the entire FileDialog to use this new API. I will bring this working item up during the planning for 3.8. A code contribution would be most welcome.
Felipe, I shall be available to give a try for this contribution. Please confirm whether this is going to be considered for future versions.
(In reply to comment #2) > Felipe, I shall be available to give a try for this contribution. Please > confirm whether this is going to be considered for future versions. Hi Praveen, if you have the time please go foward with it. I will be here to help you to get the code in the right shape. Some innitial notes: - we will have to keep the old code around for windows < vista - make sure all the native code compiles with old Win SDK, by using dynamic calls, OLE interfaces, etc.
Sure Filepe, I have the draft copy ready in my workspace (the new dialog opens up and the working directory does not change :-). Currently working on the patch to make sure that the new common file dialog supports all the features as the previous version.
Created attachment 198052 [details] Draft Patch missing the native code Filepe, I have attached a draft patch that contains the code changes for opening the File Dialog (Open and Save) with the new Common Item Dialog from Vista and above variants. However, due to unknown reason, I am unable to build the native code with the new Structure and functions added to OS.java. Would it be possible for you either to help me in generating natives, or provide me native code so that I can pursue further. I have got the latest version of SWT tools, modified the build.xml as per suggestions in FAQ. Here is the exception stack while I try to build native libs : c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(208) : error C2143: syntax error : missing '{' before '*' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(208) : error C2143: syntax error : missing ')' before '*' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(208) : error C2081: 'COMDLG_FILTERSPEC' : name in formal parameter list illegal [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(208) : error C2143: syntax error : missing '{' before '*' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(208) : error C2059: syntax error : ')' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(209) : error C2143: syntax error : missing ')' before '*' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(209) : error C2081: 'COMDLG_FILTERSPEC' : name in formal parameter list illegal [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(209) : error C2143: syntax error : missing '{' before '*' [exec] c:\work\myworkspace\org.eclipse.swt\bin\library\os_structs.h(209) : error C2059: syntax error : ')' [exec] os.c(640) : error C2065: 'COMDLG_FILTERSPEC' : undeclared identifier [exec] os.c(8846) : error C2065: 'COMDLG_FILTERSPEC' : undeclared identifier [exec] os.c(8846) : error C2146: syntax error : missing ';' before identifier '_arg1' [exec] os.c(8846) : error C2065: '_arg1' : undeclared identifier [exec] os.c(8846) : error C2065: 'lparg1' : undeclared identifier [exec] os.c(8846) : error C2100: illegal indirection [exec] os.c(8846) : warning C4047: '=' : 'int' differs in levels of indirection from 'void *' [exec] os.c(8852) : error C2065: 'lparg1' : undeclared identifier [exec] os.c(8852) : warning C4013: 'getCOMDLG_FILTERSPECFields' undefined; assuming extern returning int [exec] os.c(8852) : error C2065: '_arg1' : undeclared identifier [exec] os.c(8852) : warning C4047: '==' : 'int' differs in levels of indirection from 'void *' [exec] os.c(8853) : error C2065: 'lparg1' : undeclared identifier Please verify whether you can shed any light on this issue.
It seems that the version of the SDK we use does not include COMDLG_FILTERSPEC, we will have to make a copy for it in os.h (see BP_PAINTPARAMS for example). Besides, I can't read your patch, I think you added tabs in every line making the diff useless. Please add a patch that lets me see the differences only where the code is actually changing. Thank you
Created attachment 198327 [details] Patch v01 Felipe, Please find that attached patch that implements the Common File Dialog for Windows version >= Vista. However, the API IFileDialog::SetFileTypes() fails while passing the filterTypes. please find the 'TODO' section in the patch/code to find my comments regarding this. I would need your help in addressing this particular API. Please review and post your comments on this patch.
Review: These should be locals: int /*long*/ [] ppv = new int /*long*/ [1]; int /*long*/ mFileDialog; leaking lpFiltersArray (created but never used) can't move a java array to native array needs to alloc the native array and move element-by-element can't you release all the memory right after SetFileTypes ? defaultFolderShellItem, can't you release it right after SetDefaultFolder ? I'm getting an exception here (in my testcase I don't call setFilterExtensions()): /* SetFileTypeIndex() */ hr = OS.VtblCall(5, mFileDialog, nFilterIndex); if (hr != OS.S_OK) error (SWT.ERROR_INVALID_ARGUMENT); don't you need to release the return object from getResutls, and getResult (leaking the array and the elements) ? I think you do, all the example I saw where using COM smart pointer, which means these objects need to be releaed - why don't you always use getResutls() ? need to free the result of GetDisplayName (needs to be free with CoTaskMemFree) do you need this code? /* * Feature in Windows. For some reason, the WH_MSGFILTER filter * does not run for GetSaveFileName() or GetOpenFileName(). The * fix is to allow async messages to run in the WH_FOREGROUNDIDLE * hook instead. * * Bug in Windows 98. For some reason, when certain operating * system calls such as Shell_NotifyIcon(), GetOpenFileName() * and GetSaveFileName() are made during the WH_FOREGROUNDIDLE * hook, Windows hangs. The fix is to disallow async messages * during WH_FOREGROUNDIDLE. */ boolean oldRunMessagesInIdle = display.runMessagesInIdle; display.runMessagesInIdle = !OS.IsWin95; do you need this code? /* Make the parent shell be temporary modal */ Dialog oldModal = null; Display display = parent.getDisplay (); if ((style & (SWT.APPLICATION_MODAL | SWT.SYSTEM_MODAL)) != 0) { oldModal = display.getModalDialog (); display.setModalDialog (this); }
review (more): all the places where you have "error (SWT.ERROR_INVALID_ARGUMENT);" I think it is wrong. all the places where you have "return fileName;" I think it is wrong. For some of these strings that you are passing to iFileDialog, you could allocate them in stack (instead of putting everything in the heap) so you wouldn't need to need to free them.
Something else to test: SetOptions/GetOptions take a DWORD and you are passing int /*long*/ I suspect that in 64 bit this can be wrong.
Created attachment 199513 [details] crash Crashed on my machine when I select a file from Document Library.
My test case: public static void main(String[] args) { final Display display = new Display(); final Shell shell = new Shell(display); shell.setLayout(new GridLayout()); final Button button1 = new Button(shell, SWT.PUSH); button1.setText("OLD"); final Button button2 = new Button(shell, SWT.PUSH); button2.setText("NEW"); Listener listener = new Listener() { public void handleEvent(Event event) { boolean newdialgo = event.widget == button2; int style = SWT.OPEN; // style = SWT.SAVE; style |= SWT.MULTI; // style |= SWT.RIGHT_TO_LEFT; FileDialog dialog = new FileDialog(shell, style); dialog.setText(newdialgo ? "New Dialog" : "Old Dialog"); dialog.setFileName("nsd.exe"); dialog.setFilterPath("C:\\notes\\"); dialog.setFilterExtensions(new String[] {"*.*", "*.txt", "*.exe"}); dialog.setFilterNames(new String[] {"All", "Text", "Executables"}); dialog.setFilterIndex(2); String result = newdialgo ? dialog.open2() : dialog.open(); System.out.println("result " + result); System.out.println("getFileName " + dialog.getFileName()); System.out.println("getFilterPath " + dialog.getFilterPath()); String[] filesNames = dialog.getFileNames(); for (int i = 0; i < filesNames.length; i++) { System.out.println("getFileNames()["+i+"]"+filesNames[i]); } System.out.println("getFilterIndex " + dialog.getFilterIndex()); System.out.println("------------------------------------"); } }; button1.addListener(SWT.Selection, listener); button2.addListener(SWT.Selection, listener); shell.pack(); shell.open(); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); }
My implementation of open using ifiledialog public String open2() { boolean save = (style & SWT.SAVE) != 0; int /*long*/ [] ppv = new int /*long*/ [1]; byte [] clsid = save ? CLSID_FileSaveDialog : CLSID_FileOpenDialog; byte [] iid = save ? IID_IFileSaveDialog : IID_IFileOpenDialog; String fullPath = null; if (OS.CoCreateInstance(clsid, 0, OS.CLSCTX_INPROC_SERVER, iid, ppv) == OS.S_OK) { int /*long*/ mFileDialog = ppv [0]; int [] fos = new int [1]; /* GetOptions() */ if ((OS.VtblCall(10, mFileDialog, fos)) == OS.S_OK) { fos [0] |= OS.FOS_FORCEFILESYSTEM | OS.FOS_NOCHANGEDIR; if (save && overwrite) fos [0] |= OS.FOS_OVERWRITEPROMPT; if ((style & SWT.MULTI) != 0) fos [0] |= OS.FOS_ALLOWMULTISELECT; /* SetOptions() */ OS.VtblCall(9, mFileDialog, fos [0]); } if (title == null) title = ""; if (title.length () > 0) { char [] buffer = new char [title.length() + 1]; title.getChars(0, title.length(), buffer, 0); /* SetTitle() */ OS.VtblCall(17, mFileDialog, buffer); } if (fileName == null) fileName = ""; if (fileName.length() > 0) { char [] buffer = new char [fileName.length() + 1]; fileName.getChars(0, fileName.length(), buffer, 0); /* SetFileName() */ OS.VtblCall(15, mFileDialog, buffer); } if (filterPath == null) filterPath = ""; if (filterPath.length() > 0) { String path = filterPath.replace ('/', '\\'); char [] buffer = new char [path.length() + 1]; path.getChars(0, path.length(), buffer, 0); if (OS.SHCreateItemFromParsingName(buffer, 0, IID_IShellItem, ppv) == OS.S_OK) { int /*long*/ psi = ppv [0]; /* * SetDefaultDirectory does not work if the dialog has persisted * recently used folder. The fix is to clear the persisted data. */ /* ClearClientData() */ OS.VtblCall(25, mFileDialog); /* SetDefaultFolder() */ OS.VtblCall(11, mFileDialog, psi); /* Release ()*/ OS.VtblCall (2, psi); } } if (filterNames == null) filterNames = new String [0]; if (filterExtensions == null) filterExtensions = new String [0]; if (filterExtensions.length > 0) { int /*long*/ hHeap = OS.GetProcessHeap (); COMDLG_FILTERSPEC [] filters = new COMDLG_FILTERSPEC [filterExtensions.length]; int /*long*/ pFilters = OS.HeapAlloc (hHeap, OS.HEAP_ZERO_MEMORY, filters.length * COMDLG_FILTERSPEC.sizeof); for (int i = 0; i < filterExtensions.length; i++) { COMDLG_FILTERSPEC filter = filters[i] = new COMDLG_FILTERSPEC(); String ext = filterExtensions[i]; String name = ext; if (filterNames != null && i < filterNames.length) name = filterNames[i]; char [] buffer = new char [name.length() + 1]; name.getChars(0, name.length(), buffer, 0); filter.pszName = OS.HeapAlloc (hHeap, OS.HEAP_ZERO_MEMORY, buffer.length * 2); OS.MoveMemory(filter.pszName, buffer, buffer.length * 2); buffer = new char [ext.length() + 1]; ext.getChars(0, ext.length(), buffer, 0); filter.pszSpec = OS.HeapAlloc (hHeap, OS.HEAP_ZERO_MEMORY, buffer.length * 2); OS.MoveMemory(filter.pszSpec, buffer, buffer.length * 2); OS.MoveMemory(pFilters + (i * COMDLG_FILTERSPEC.sizeof), filter, COMDLG_FILTERSPEC.sizeof); } /* SetFileTypes() */ OS.VtblCall(4, mFileDialog, filters.length, pFilters); /* SetFileTypeIndex() */ OS.VtblCall(5, mFileDialog, filterIndex + 1); OS.HeapFree(hHeap, 0, pFilters); for (int i=0; i<filters.length; i++) { OS.HeapFree (hHeap, 0, filters[i].pszName); OS.HeapFree (hHeap, 0, filters[i].pszSpec); } } else { //TODO add *.* } //TODO test asyncs //TODO test BIDI //TODO test modal fileNames = new String [0]; int /*long*/ hwndOwner = parent.handle; /* Show() */ if (OS.VtblCall(3, mFileDialog, hwndOwner) == OS.S_OK) { if (save || (style & SWT.MULTI) == 0) { /* GetResult() */ if (OS.VtblCall(20, mFileDialog, ppv) == OS.S_OK) { int /*long*/ psi = ppv [0]; /*IShellItem::GetDisplayName*/ if (OS.VtblCall (5, psi, OS.SIGDN_FILESYSPATH, ppv) == OS.S_OK) { int /*long*/ wstr = ppv [0]; int length = OS.wcslen (wstr); char [] buffer = new char [length]; OS.MoveMemory (buffer, wstr, length * 2); OS.CoTaskMemFree (wstr); fullPath = new String (buffer); int index = fullPath.lastIndexOf('\\'); fileName = fullPath.substring(index + 1); filterPath = fullPath.substring(0, index); fileNames = new String [1]; fileNames[0] = fileName; } /* Release ()*/ OS.VtblCall (2, psi); } } else { if (OS.VtblCall(27, mFileDialog, ppv) == OS.S_OK) { int /*long*/ shellItemArray = ppv [0]; int[] count = new int[1]; /* IShellItemArray::GetCount() */ OS.VtblCall (7, shellItemArray, count); fileNames = new String [count [0]]; for (int i = 0; i < count [0]; i++) { /* IShellItemArray->GetItemAt() */ if (OS.VtblCall(8, shellItemArray, i, ppv) == OS.S_OK) { int /*long*/ psi = ppv[0]; /*IShellItem::GetDisplayName*/ if (OS.VtblCall (5, psi, OS.SIGDN_FILESYSPATH, ppv) == OS.S_OK) { int /*long*/ wstr = ppv [0]; int length = OS.wcslen (wstr); char [] buffer = new char [length]; OS.MoveMemory (buffer, wstr, length * 2); OS.CoTaskMemFree (wstr); String path = new String (buffer); int index = path.lastIndexOf('\\'); fileNames[i] = path.substring(index + 1); if (fullPath == null) { fullPath = path; fileName = fileNames[i]; filterPath = path.substring(0, index); } } /* Release ()*/ OS.VtblCall (2, psi); } } /* Release ()*/ OS.VtblCall (2, shellItemArray); } } if (filterExtensions.length > 0) { int [] piFileType = new int [1]; /* GetFileTypeIndex() */ if (OS.VtblCall(6, mFileDialog, piFileType) == OS.S_OK) { filterIndex = piFileType [0] - 1; } } } /* Release ()*/ OS.VtblCall (2, mFileDialog); } return fullPath; } it seems to be works, but needs a lot more testing (double check for leaks, etc).
Created attachment 199744 [details] Patch v02 Filepe, Thanks for your suggestions. I have slightly modified code to your working code changes and attached the latest patch. I thought of re-using the existing code (old file dialog) for the new File Dialog, and that is the reason I end up using string elements on heap instead of on stack. Wouldn't we need the code to display.setModalDialog (this); for display class to make a note of the existing FileDialog as the current model dialog ? I will test it out on 64-bit machine on Monday and will let you know the results. As you said, I completely agree with you that more intensive testing need to be done for Bidi, async, Modal dialogs etc.
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie.
This bug was marked as stalebug a while ago. Marking as worksforme. If this report is still relevant for the current release, please reopen and remove the stalebug whiteboard tag.
This bug has been marked as stalebug a while ago without any further interaction. If this report is still relevant for the current release, please reopen and remove the stalebug whiteboard flag.
