bug#8181: Patch to fix bug#8181: 23.2; Dired on Windows 7

[Juanma Barranquero <lekktu@gmail.com>]

On Mon, Mar 7, 2011 at 05:15, Ben Key <bkey76@gmail.com> wrote:

> The attached patch fixes bug #8181: 23.2; Dired on Windows 7.

Thanks.

> The code that was attempting to set the text of the file name text field
> when processing the CDN_INITDONE WM_NOTIFY message does not work.  This is
> because the code that processes the lpstrFile member of the OPENFILENAME
> structure to set the initial value of the window is called after the
> CDN_INITDONE WM_NOTIFY message is processed.  The correct way to set the
> text of the file name text field to "Current Directory" is to set the
> initial value of the lpstrFile member of the OPENFILENAME structure to
> "Current Directory" in the only_dir_p case if it does not already have
> another value.

There's no need to duplicate the code in both branches of "if (STRINGP
(default_filename))"; "Current Directory" is shorter than MAX_PATH, so
there are no worries about buffer overflow. You can just add

  /* The code in file_dialog_callback that attempts to set the text
     of the file name edit window when handling the CDN_INITDONE
     WM_NOTIFY message does not work.  Setting filename to "Current
     Directory" in the only_dir_p case here does work however.  */
  if (filename[0] == 0 && ! NILP (only_dir_p))
    strcpy (filename, "Current Directory");

afterwards.

> The attempt to find the window handle of the file name text field failed, at
> least on Windows XP and Windows 7.  By using Microsoft Spy++ I was able to
> discover the correct way to obtain this Window handle.  I modified the code
> in file_dialog_callback that initializes the edit_control using this
> information.

Having to go fishing for the window handles is hackish, but not your
fault, but Microsoft's. Unless someone knows of a better way, that's
what we're stuck with.

> My solution was to set focus to the list box if the
> file name text field is disabled during dialog box initialization.  This
> part could use a little more work.

I don't see anything wrong with this.

Here's your patch, slighty reworked to be more on line with current
Emacs coding conventions (the "0 == X" and "NULL == X" styles are not
frequently used) and with a proposed ChangeLog entry (feel free to
rewrite it, of course).

    Juanma



2011-03-07  Ben Key  <bkey76@gmail.com>

	* w32fns.c (FILE_NAME_COMBO_BOX, FILE_NAME_LIST): Define.
	(file_dialog_callback): Fix locating the window handle of the File Name
	text field.  After disabling it, set focus on the list control.
	(Fx_file_dialog): If only_dir_p is non-nil, set the text of the File
	Name text field to "Current Directory" if it does not already have
	another value.  (Bug#8181)


=== modified file 'src/w32fns.c'
--- src/w32fns.c	2011-02-16 18:39:46 +0000
+++ src/w32fns.c	2011-03-07 05:01:46 +0000
@@ -60,6 +60,8 @@
 #include <dlgs.h>
 #include <imm.h>
 #define FILE_NAME_TEXT_FIELD edt1
+#define FILE_NAME_COMBO_BOX cmb13
+#define FILE_NAME_LIST lst1

 #include "font.h"
 #include "w32font.h"
@@ -5868,13 +5870,37 @@
 	{
 	  HWND dialog = GetParent (hwnd);
 	  HWND edit_control = GetDlgItem (dialog, FILE_NAME_TEXT_FIELD);
-
-	  /* Directories is in index 2.  */
+	  HWND list = GetDlgItem (dialog, FILE_NAME_LIST);
+
+	  /* At least on Windows 7, the above attempt to get the window handle
+	     to the File Name Text Field fails.	 The following code does the
+	     job though.  Note that this code is based on my examination of the
+	     window hierarchy using Microsoft Spy++.  bk */
+	  if (edit_control == NULL)
+	    {
+	      HWND tmp = GetDlgItem (dialog, FILE_NAME_COMBO_BOX);
+	      if (tmp)
+		{
+		  tmp = GetWindow (tmp, GW_CHILD);
+		  if (tmp)
+		    edit_control = GetWindow (tmp, GW_CHILD);
+		}
+	    }
+
+	  /* Directories is in index 2.	 */
 	  if (notify->lpOFN->nFilterIndex == 2)
 	    {
 	      CommDlg_OpenSave_SetControlText (dialog, FILE_NAME_TEXT_FIELD,
 					       "Current Directory");
 	      EnableWindow (edit_control, FALSE);
+	      /* Note that at least on Windows 7, the above call to EnableWindow
+		 disables the window that would ordinarily have focus.	If we
+		 do not set focus to some other window here, focus will land in
+		 no man's land and the user will be unable to tab through the
+		 dialog box (pressing tab will only result in a beep).
+		 Avoid that problem by setting focus to the list here.	*/
+	      if (CDN_INITDONE == notify->hdr.code)
+		SetFocus (list);
 	    }
 	  else
 	    {
@@ -5951,6 +5977,13 @@
   else
     filename[0] = '\0';

+  /* The code in file_dialog_callback that attempts to set the text
+     of the file name edit window when handling the CDN_INITDONE
+     WM_NOTIFY message does not work.  Setting filename to "Current
+     Directory" in the only_dir_p case here does work however.  */
+  if (filename[0] == 0 && ! NILP (only_dir_p))
+    strcpy (filename, "Current Directory");
+
   {
     NEWOPENFILENAME new_file_details;
     BOOL file_opened = FALSE;