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

From: Cecilio Pardo <[email-masked]>
Date: Wed, 11 Sep 2024 15:44:28 +0200

On 19/08/2024 19:44, Eli Zaretskii wrote:
>> Date: Mon, 19 Aug 2024 18:13:31 +0200
>> From: Cecilio Pardo <[email-masked]>
>>
>> This patch adds support on Windows Vista an later for dialog boxes using
>> TaskDialog.
> Thanks.
>
> First, to accept a contribution of this size we'll need a
> copyright-assignment paperwork from you.  Should I send you the form
> to fill with instructions to go with it, so you could start the
> paperwork rolling?
>
> A few comments about the patch:

Hello,

The copyright assignment is ready. Here is the patch with your

comments addressed. I also attach a couple of manual tests.

--

Cecilio Pardo

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..3c7ebf64abe 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,155 @@ #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 (Fconsp (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 = { };
+      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)))
+	return 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 +1769,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");