bug#20481: 24.5; , Newlines in message-box output don't work on Windows

[Cecilio Pardo <cpardo@imayhem.com>]

[-- Attachment #1: Type: text/plain, Size: 646 bytes --]

On 12/09/2024 4:49, Po Lu wrote:
> Thanks.  Following are a number of minor stylistic comments.

Sorry I missed those. They are corrected in the attached patch.

> Lastly, I observe that you have implemented a bespoke dialog parser for
> Windows, the likes of which have been a source of difficulties in the
> past.  Is there any particular reason that you decided against
> implementing the w32_dialog_show function called in the "#ifdef
> HAVE_DIALOGS" version of w32_popup_dialog?

I left w32_dialog_show as it was in case an implementation for

older versions of windows became available. I can rewrite it

if you think is better that way.


[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 7051 bytes --]

diff --git a/src/menu.c b/src/menu.c
index de4d0964e9c..6b4aaef1715 100644
--- a/src/menu.c
+++ b/src/menu.c
@@ -1594,9 +1594,10 @@ DEFUN ("x-popup-dialog", Fx_popup_dialog, Sx_popup_dialog, 2, 3, 0,
       Lisp_Object selection
 	= FRAME_TERMINAL (f)->popup_dialog_hook (f, header, contents);
 #ifdef HAVE_NTGUI
-      /* NTGUI supports only simple dialogs with Yes/No choices.  For
-	 other dialogs, it returns the symbol 'unsupported--w32-dialog',
-	 as a signal for the caller to fall back to the emulation code.  */
+      /* NTGUI on Windows versions before Vista supports only simple
+	 dialogs with Yes/No choices.  For other dialogs, it returns the
+	 symbol 'unsupported--w32-dialog', as a signal for the caller to
+	 fall back to the emulation code.  */
       if (!EQ (selection, Qunsupported__w32_dialog))
 #endif
 	return selection;
diff --git a/src/w32menu.c b/src/w32menu.c
index cea4f4892a4..8ecf7e8e8a7 100644
--- a/src/w32menu.c
+++ b/src/w32menu.c
@@ -52,6 +52,9 @@
 
 #include "w32common.h"	/* for osinfo_cache */
 
+#include "commctrl.h"
+
+/* This only applies to OS versions prior to Vista.  */
 #undef HAVE_DIALOGS /* TODO: Implement native dialogs.  */
 
 #ifndef TRUE
@@ -76,6 +79,11 @@ #define FALSE 0
     IN const WCHAR *text,
     IN const WCHAR *caption,
     IN UINT type);
+typedef HRESULT (WINAPI *TaskDialogIndirect_Proc) (
+    IN const TASKDIALOGCONFIG *pTaskConfig,
+    OUT int *pnButton,
+    OUT int *pnRadioButton,
+    OUT BOOL *pfVerificationFlagChecked);
 
 #ifdef NTGUI_UNICODE
 GetMenuItemInfoA_Proc get_menu_item_info = GetMenuItemInfoA;
@@ -89,6 +97,8 @@ #define FALSE 0
 MessageBoxW_Proc unicode_message_box = NULL;
 #endif /* NTGUI_UNICODE */
 
+TaskDialogIndirect_Proc task_dialog_indirect;
+
 #ifdef HAVE_DIALOGS
 static Lisp_Object w32_dialog_show (struct frame *, Lisp_Object, Lisp_Object, char **);
 #else
@@ -101,14 +111,157 @@ #define FALSE 0
 
 void w32_free_menu_strings (HWND);
 
+#define TASK_DIALOG_MAX_BUTTONS 10
+
+static HRESULT
+task_dialog_callback (HWND hwnd, UINT msg, WPARAM wParam,
+		      LPARAM lParam, LONG_PTR callback_data)
+{
+  switch (msg)
+    {
+    case TDN_CREATED:
+      /* Disable all buttons with ID >= 2000  */
+      for (int i = 0; i < TASK_DIALOG_MAX_BUTTONS; i++)
+        SendMessage (hwnd, TDM_ENABLE_BUTTON, 2000 + i, FALSE);
+      break;
+    }
+  return S_OK;
+}
+
 Lisp_Object
 w32_popup_dialog (struct frame *f, Lisp_Object header, Lisp_Object contents)
 {
-
   check_window_system (f);
 
-#ifndef HAVE_DIALOGS
+  if (task_dialog_indirect)
+    {
+      int wide_len;
+
+      CHECK_CONS (contents);
+
+      /* Get the title as an UTF-16 string.  */
+      char *title = SSDATA (ENCODE_UTF_8 (XCAR (contents)));
+      wide_len = (sizeof (WCHAR)
+		  * pMultiByteToWideChar (CP_UTF8, 0, title, -1, NULL, 0));
+      WCHAR *title_w = alloca (wide_len);
+      pMultiByteToWideChar (CP_UTF8, 0, title, -1, title_w, wide_len);
 
+      /* Prepare the arrays with the dialog's buttons and return values.  */
+      TASKDIALOG_BUTTON buttons[TASK_DIALOG_MAX_BUTTONS];
+      Lisp_Object button_values[TASK_DIALOG_MAX_BUTTONS];
+      int button_count = 0;
+      Lisp_Object b = XCDR (contents);
+
+      while (!NILP (b))
+	{
+	  if (button_count >= TASK_DIALOG_MAX_BUTTONS)
+	    {
+	      /* We have too many buttons. We ignore the rest.  */
+	      break;
+	    }
+	  
+	  Lisp_Object item = XCAR (b);
+	  
+	  if (CONSP (item))
+	    {
+	      /* A normal item (text . value)  */
+	      Lisp_Object item_name = XCAR (item);
+	      Lisp_Object item_value = XCDR (item);
+	      
+	      CHECK_STRING (item_name);
+	      
+	      item_name = ENCODE_UTF_8 (item_name);
+	      wide_len = (sizeof (WCHAR)
+			  * pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item_name),
+						  -1, NULL, 0));
+	      buttons[button_count].pszButtonText = alloca (wide_len);
+	      pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item_name), -1,
+				    (LPWSTR)
+				    buttons[button_count].pszButtonText,
+				    wide_len);
+	      buttons[button_count].nButtonID = 1000 + button_count;
+	      button_values[button_count++] = item_value;
+	    }
+	  else if (NILP (item))
+	    {
+	      /* A nil item means to put all following items on the
+		 right.  We ignore this.  */
+	    }
+	  else if (STRINGP (item))
+	    {
+	      /* A string item means an unselectable button.  We add a
+	       button, an then need to disable it on the callback.  We
+	       use ids based on 2000 to mark these buttons.  */
+	      Lisp_Object item_name = ENCODE_UTF_8 (item);
+	      wide_len = (sizeof (WCHAR)
+			  * pMultiByteToWideChar (CP_UTF8, 0,
+						  SSDATA (item_name),
+						  -1, NULL, 0));
+	      buttons[button_count].pszButtonText = alloca (wide_len);
+	      pMultiByteToWideChar (CP_UTF8, 0, SSDATA (item_name), -1,
+				    (LPWSTR)
+				    buttons[button_count].pszButtonText,
+				    wide_len);
+	      buttons[button_count].nButtonID = 2000 + button_count;
+	      button_values[button_count++] = Qnil;
+	    }
+	  else
+	    {
+	      error ("Incorrect dialog button specification");
+	      return Qnil;
+	    }
+	  
+	  b = XCDR (b);
+	}
+      
+      int pressed_button = 0;
+      
+      TASKDIALOGCONFIG config = { 0 };
+      config.hwndParent = FRAME_W32_WINDOW (f);
+      config.cbSize = sizeof (config);
+      config.hInstance = hinst;
+      config.dwFlags = TDF_ALLOW_DIALOG_CANCELLATION;
+      config.pfCallback = task_dialog_callback;
+      
+      config.pszWindowTitle = L"Question";
+      if (!NILP (header))
+	{
+	  config.pszWindowTitle = L"Information";
+	  config.pszMainIcon = TD_INFORMATION_ICON;
+	}
+
+      config.pszMainInstruction = title_w;
+      config.pButtons = buttons;
+      config.cButtons = button_count;
+      
+      if (!SUCCEEDED (task_dialog_indirect (&config, &pressed_button,
+					    NULL, NULL)))
+	quit ();
+      
+
+      switch (pressed_button)
+	{
+	case IDOK:
+	  /* This can only happen if no buttons were provided. An OK
+	     button is automatically added by TaskDialogIndirect in that
+	     case.  */
+	  return Qt;
+	case IDCANCEL:
+	  /* The user closed the dialog without using the buttons.  */
+	  return quit ();
+	default:
+	  /* One of the specified buttons.  */
+	  int button_index = pressed_button - 1000;
+	  if (button_index >= 0 && button_index < button_count)
+	    return button_values[button_index];
+	  return quit ();
+	}
+    }
+
+  /* If we get here, TaskDialog is not supported. Use MessageBox/Menu.  */
+
+
+#ifndef HAVE_DIALOGS
   /* Handle simple Yes/No choices as MessageBox popups.  */
   if (is_simple_dialog (contents))
     return simple_dialog_show (f, contents, header);
@@ -1618,6 +1771,10 @@ syms_of_w32menu (void)
 void
 globals_of_w32menu (void)
 {
+  HMODULE comctrl32 = GetModuleHandle ("comctl32.dll");
+  task_dialog_indirect = (TaskDialogIndirect_Proc)
+    get_proc_addr (comctrl32, "TaskDialogIndirect");
+
 #ifndef NTGUI_UNICODE
   /* See if Get/SetMenuItemInfo functions are available.  */
   HMODULE user32 = GetModuleHandle ("user32.dll");