bug#3468: drag and drop text

bug#3468: drag and drop text

[Erdkern Erdkern]

I use windows as operating system. I want to drag text from text editor or browser (say Wordpad or Firefox) and drop it in emacs, but this is not possible, drop is not allowed. On the other side, if I drag an unknown filetype (say, an *.rdp-File) into emacs, it is dropped as text. 

In GNU Emacs 22.3.1 (i386-mingw-nt5.1.2600)
 of 2008-09-06 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4)'

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: DEU
  locale-coding-system: cp1252
  default-enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  cua-mode: t
  encoded-kbd-mode: t
  tooltip-mode: t
  tool-bar-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  unify-8859-on-encoding-mode: t
  utf-translate-cjk-mode: t
  auto-compression-mode: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<up> <up> <up> <up> <up> <up> <up> <up> <up> <up> <down> 
<down-mouse-1> <mouse-1> <down-mouse-1> <mouse-1> <down-mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> <mouse-1> 
<mouse-1> <mouse-1> <mouse-1> <mouse-1> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <tool-bar> 
<kill-buffer> <help-echo> <help-echo> <help-echo> <help-echo> 
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo> 
<menu-bar> <help-menu> <send-emacs-bug-report>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Loading emacsbug...
Loading regexp-opt...done
Loading emacsbug...done
Loading help-mode...done
Auto-saving...done
Undo! [29 times]
Auto-saving...done
*** E-Mail body has been placed on clipboard, please paste them here! ***

-- 
GMX FreeDSL mit DSL 6.000 Flatrate und Telefonanschluss nur 17,95 Euro/mtl.!
http://dslspecial.gmx.de/freedsl-aktionspreis/?ac=OM.AD.PD003K11308T4569a

bug#3468: drag and drop text

[Lars Ingebrigtsen]

"Erdkern Erdkern" <Erdkern2000@gmx.de> writes:

> I use windows as operating system. I want to drag text from text
> editor or browser (say Wordpad or Firefox) and drop it in emacs, but
> this is not possible, drop is not allowed. On the other side, if I
> drag an unknown filetype (say, an *.rdp-File) into emacs, it is
> dropped as text.

(I'm going through old bug reports that unfortunately didn't get any
response at the time.)

I tried this in Emacs 27, but on GNU/Linux, and it worked fine for me
there.  Are you still seeing this problem?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#3468: drag and drop text

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Mon, 30 Sep 2019 17:29:54 +0200
> Cc: 3468@debbugs.gnu.org
> 
> I tried this in Emacs 27, but on GNU/Linux, and it worked fine for me
> there.  Are you still seeing this problem?

Emacs on MS-Windows doesn't support drag-n-drop of arbitrary text, it
only supports drag-n-drop of file names.  This is indicated by the
"you can't do that" icon shown as the mouse pointer when you attempt
to drop the text onto the Emacs frame.

The Emacs User manual says in "Drag and Drop":

     The XDND and Motif drag and drop protocols, and the old KDE 1.x
  protocol, are currently supported.

which doesn't include any MS-Windows protocols.

The reason for not supporting this on MS-Windows is that the relevant
system APIs are only available for C++ programs.  If someone knows how
to work around this limitation, I'm sure we will welcome patches or at
least an explanation for how to do that in plain C.

bug#3468: drag and drop text

[Cecilio Pardo]

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

This patch implements drag-and-drop for w32 for files and text, using C,
not C++.
This should work from Windows 95, but I can't test it.  Because of this,
the prior implementation with WM_DROPFILES has been removed with '#if
0', and can probably be completely removed.

I tested with mingw-w64 and mingw on Windows 11.

Text is inserted at point position, not at drop position.  I don't know
how this works on other platforms.


[-- Attachment #2: 0001-Implement-drag-n-drop-for-w32-with-support-for-files.patch --]
[-- Type: text/plain, Size: 13699 bytes --]

From 1712ab3ba109b2beafa418f0b12d09c836590b2c Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Fri, 27 Sep 2024 23:58:02 +0200
Subject: [PATCH] Implement drag-n-drop for w32 with support for files and text

Implement dnd with IDropTarget

* lisp/term/w32-win.el (w32-drag-n-drop): changed to handle files or strings
* src/w32fns.c
(process_dropfiles): new, convert DROPFILES struct to array of strings
(w32_createwindow): assign an IDropTarget to each new frame
(w32_name_of_message): added new messages
(w32_msg_pump): Changed CoInitialize to OleInitialize, needed by the
drag-n-drop functions
(w32_wnd_proc): new struct w32_drop_target, and w32_drop_target_*
functions to implement the IDropTarget interface
* src/w32term.c (w32_read_socket): handle WM_EMACS_DROPFILES,
WM_EMACS_DROPSTRING, skip WM_EMACS_DROPFILES
* src/w32term.h (): add WM_EMACS_DROPFILES, WM_EMACS_DROPSTRING
---
 lisp/term/w32-win.el |  19 ++--
 src/w32fns.c         | 202 ++++++++++++++++++++++++++++++++++++++++++-
 src/w32term.c        |  46 ++++++++++
 src/w32term.h        |   4 +-
 4 files changed, 260 insertions(+), 11 deletions(-)

diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index b57b3dd3bef..ac85ec8d945 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -131,8 +131,11 @@ w32-dropped-file-to-url
    file-name))
 
 (defun w32-drag-n-drop (event &optional new-frame)
-  "Edit the files listed in the drag-n-drop EVENT.
-Switch to a buffer editing the last file dropped."
+  "If the drag-n-drop EVENT is for a file or files, edit those
+files. Switch to a buffer editing the last file dropped.
+
+If the EVENT is for text, insert the text at point on the buffer shown
+in the target window, or add to kill ring if that buffer is read-only."
   (interactive "e")
   (save-excursion
     ;; Make sure the drop target has positive co-ords
@@ -140,6 +143,7 @@ w32-drag-n-drop
     ;; won't work.  <skx@tardis.ed.ac.uk>
     (let* ((window (posn-window (event-start event)))
 	   (coords (posn-x-y (event-start event)))
+           (arg (car (cdr (cdr event))))
 	   (x (car coords))
 	   (y (cdr coords)))
       (if (and (> x 0) (> y 0))
@@ -150,11 +154,12 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (dnd-handle-multiple-urls
-       window
-       (mapcar #'w32-dropped-file-to-url
-               (car (cdr (cdr event))))
-       'private))))
+      (if (stringp arg)
+          (dnd-insert-text window 'copy arg)
+        (dnd-handle-multiple-urls
+         window
+         (mapcar #'w32-dropped-file-to-url arg)
+         'private)))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
diff --git a/src/w32fns.c b/src/w32fns.c
index 0a3f5c38a58..fdba3524846 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -34,6 +34,12 @@ #define _WIN32_WINNT 0x0600
 
 #include <c-ctype.h>
 
+#define COBJMACROS /* Ask for C definitions for COM.  */
+#include <shlobj.h>
+#include <oleidl.h>
+#include <objidl.h>
+#include <ole2.h>
+
 #include "lisp.h"
 #include "w32term.h"
 #include "frame.h"
@@ -359,6 +365,10 @@ #define WS_EX_NOACTIVATE 0x08000000L
 
 static struct w32_display_info *w32_display_info_for_name (Lisp_Object);
 
+static void my_post_msg (W32Msg*, HWND, UINT, WPARAM, LPARAM);
+static unsigned int w32_get_modifiers (void);
+
+
 /* Let the user specify a display with a frame.
    nil stands for the selected frame--or, if that is not a w32 frame,
    the first display on the list.  */
@@ -2464,6 +2474,168 @@ w32_createhscrollbar (struct frame *f, struct scroll_bar * bar)
   return hwnd;
 }
 
+/* From the DROPFILES struct, extract the list of filenames.  Returns a
+   NULL terminated malloc array of malloc strings that should be freed
+   by the caller.  */
+static char **
+process_dropfiles (DROPFILES *files)
+{
+  char *start_of_files = (char*)files + files->pFiles;
+  int count = 0;
+  char filename[MAX_PATH];
+  char **filenames;
+
+  if (files->fWide)
+    {
+      WCHAR *p = (WCHAR*)start_of_files;
+      for ( ; *p; count ++, p += wcslen (p) + 1)
+	;
+      filenames = malloc ((count+1) * sizeof (char*));
+      filenames [count] = NULL;
+      p = (WCHAR*)start_of_files;
+      for ( int i = 0; *p; p += wcslen (p) + 1, i++)
+	{
+	  filename_from_utf16 (p, filename);
+	  filenames [i] = xstrdup (filename);
+	}
+    }
+  else
+    {
+      char *p = start_of_files;
+      for ( ; *p; count ++, p += strlen (p) + 1)
+	;
+      filenames = malloc ((count + 1) * sizeof (char*));
+      filenames [count] = NULL;
+      p = start_of_files;
+      for ( int i = 0; *p; p += strlen(p) + 1, i++)
+	{
+	  filename_from_ansi (p, filename);
+	  filenames[i] = xstrdup (filename);
+	}
+    }
+  return filenames;
+}
+
+struct w32_drop_target {
+  /* i_drop_target must be the first member.  */
+  IDropTarget i_drop_target;
+  HWND hwnd;
+};
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_QueryInterface (IDropTarget *t, REFIID ri, void **r)
+{
+  return E_NOINTERFACE;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_AddRef (IDropTarget *This)
+{
+  return 1;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_Release (IDropTarget *This)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+  xfree( target->i_drop_target.lpVtbl );
+  xfree( target );
+  return 0;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragEnter( IDropTarget *This, IDataObject *pDataObj, DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragOver ( IDropTarget *This, DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragLeave( IDropTarget *This )
+{
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_Drop (IDropTarget *This, IDataObject *pDataObj,
+		      DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+  *pdwEffect = DROPEFFECT_COPY;
+
+  W32Msg msg = {0};
+  msg.dwModifiers = w32_get_modifiers();
+  msg.msg.time = GetMessageTime ();
+  msg.msg.pt.x = pt.x;
+  msg.msg.pt.y = pt.y;
+
+  STGMEDIUM stgmedium;
+
+  /* Try for dropped files (CF_HDROP).  */
+  FORMATETC formatetc = { CF_HDROP, NULL, DVASPECT_CONTENT, -1, TYMED_HGLOBAL };
+  if (SUCCEEDED (IDataObject_GetData (pDataObj, &formatetc, &stgmedium)))
+    {
+      if (stgmedium.tymed == TYMED_HGLOBAL)
+	{
+	  DROPFILES *files = (DROPFILES*)GlobalLock (stgmedium.hGlobal);
+	  if (files)
+	      my_post_msg (&msg, target->hwnd, WM_EMACS_DROPFILES,
+			   0, (LPARAM)process_dropfiles (files) );
+	  GlobalUnlock (stgmedium.hGlobal);
+	}
+      ReleaseStgMedium (&stgmedium);
+      return S_OK;
+    }
+
+  formatetc.cfFormat = CF_UNICODETEXT;
+  if (SUCCEEDED (IDataObject_GetData (pDataObj, &formatetc, &stgmedium)))
+    {
+      if (stgmedium.tymed == TYMED_HGLOBAL)
+	{
+	  WCHAR *text = (WCHAR*)GlobalLock (stgmedium.hGlobal);
+	  Lisp_Object text_string = from_unicode_buffer (text);
+	  char *utf8 = xstrdup (SSDATA (ENCODE_UTF_8 (text_string)));
+	  my_post_msg (&msg, target->hwnd, WM_EMACS_DROPSTRING,
+		       0, (LPARAM)utf8 );
+	  GlobalUnlock (stgmedium.hGlobal);
+	}
+      ReleaseStgMedium (&stgmedium);
+      return S_OK;
+    }
+
+  formatetc.cfFormat = CF_TEXT;
+  if (SUCCEEDED (IDataObject_GetData (pDataObj, &formatetc, &stgmedium)))
+    {
+      if (stgmedium.tymed == TYMED_HGLOBAL)
+	{
+	  char *text = (char*)GlobalLock (stgmedium.hGlobal);
+
+	  int l = strlen (text);
+	  WCHAR *text_utf16 = xmalloc (sizeof (WCHAR) * l + 1 );;
+	  if (MultiByteToWideChar (CP_ACP, 0, text, l, text_utf16, l + 1))
+	    {
+	      Lisp_Object text_string = from_unicode_buffer (text_utf16);
+	      char *utf8 = xstrdup (SSDATA (ENCODE_UTF_8 (text_string)));
+	      my_post_msg (&msg, target->hwnd, WM_EMACS_DROPSTRING,
+			   0, (LPARAM)utf8 );
+	    }
+	  xfree (text_utf16);
+	  GlobalUnlock (stgmedium.hGlobal);
+	}
+      ReleaseStgMedium (&stgmedium);
+      return S_OK;
+    }
+
+  return S_OK;
+}
+
 static void
 w32_createwindow (struct frame *f, int *coords)
 {
@@ -2548,7 +2720,23 @@ w32_createwindow (struct frame *f, int *coords)
       SetWindowLong (hwnd, WND_BACKGROUND_INDEX, FRAME_BACKGROUND_PIXEL (f));
 
       /* Enable drag-n-drop.  */
+      struct w32_drop_target *drop_target = xmalloc (sizeof (struct w32_drop_target));
+      drop_target-> hwnd = hwnd;
+
+      IDropTargetVtbl *vtbl = xmalloc (sizeof (IDropTargetVtbl));
+      drop_target->i_drop_target.lpVtbl = vtbl;
+      vtbl->QueryInterface = w32_drop_target_QueryInterface;
+      vtbl->AddRef = w32_drop_target_AddRef;
+      vtbl->Release = w32_drop_target_Release;
+      vtbl->DragEnter = w32_drop_target_DragEnter;
+      vtbl->DragOver = w32_drop_target_DragOver;
+      vtbl->DragLeave = w32_drop_target_DragLeave;
+      vtbl->Drop = w32_drop_target_Drop;
+      RegisterDragDrop (hwnd, &drop_target->i_drop_target);
+
+#if 0
       DragAcceptFiles (hwnd, TRUE);
+#endif
 
       /* Enable system light/dark theme.  */
       w32_applytheme (hwnd);
@@ -3399,6 +3587,8 @@ #define M(msg) { msg, # msg }
       M (WM_EMACS_PAINT),
       M (WM_EMACS_IME_STATUS),
       M (WM_CHAR),
+      M (WM_EMACS_DROPFILES),
+      M (WM_EMACS_DROPSTRING),
 #undef M
       { 0, 0 }
   };
@@ -3465,13 +3655,14 @@ w32_msg_pump (deferred_msg * msg_buf)
 	      /* Produced by complete_deferred_msg; just ignore.  */
 	      break;
 	    case WM_EMACS_CREATEWINDOW:
-	      /* Initialize COM for this window. Even though we don't use it,
-		 some third party shell extensions can cause it to be used in
+	      /* Initialize COM for this window. Needed for RegisterDragDrop.
+		 Some third party shell extensions can cause it to be used in
 		 system dialogs, which causes a crash if it is not initialized.
 		 This is a known bug in Windows, which was fixed long ago, but
 		 the patch for XP is not publicly available until XP SP3,
 		 and older versions will never be patched.  */
-	      CoInitialize (NULL);
+	      OleInitialize (NULL);
+
 	      w32_createwindow ((struct frame *) msg.wParam,
 				(int *) msg.lParam);
 	      if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
@@ -5106,7 +5297,9 @@ #define MOUSEEVENTF_FROMTOUCH 0xFF515700
       return 0;
 
     case WM_MOUSEWHEEL:
+#if 0
     case WM_DROPFILES:
+#endif
       wmsg.dwModifiers = w32_get_modifiers ();
       my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
       signal_user_input ();
@@ -5597,7 +5790,10 @@ #define WM_TOUCH 576
       }
 
     case WM_EMACS_DESTROYWINDOW:
+	RevokeDragDrop ((HWND) wParam);
+#if 0
       DragAcceptFiles ((HWND) wParam, FALSE);
+#endif
       return DestroyWindow ((HWND) wParam);
 
     case WM_EMACS_HIDE_CARET:
diff --git a/src/w32term.c b/src/w32term.c
index 62037e3b2cd..e1ff4112a7b 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -3576,6 +3576,7 @@ w32_construct_mouse_wheel (struct input_event *result, W32Msg *msg,
   return Qnil;
 }
 
+#if 0
 static Lisp_Object
 w32_construct_drag_n_drop (struct input_event *result, W32Msg *msg,
                            struct frame *f)
@@ -3651,6 +3652,7 @@ w32_construct_drag_n_drop (struct input_event *result, W32Msg *msg,
   return Qnil;
 }
 
+#endif
 \f
 #if HAVE_W32NOTIFY
 
@@ -5682,12 +5684,56 @@ w32_read_socket (struct terminal *terminal,
 	  }
 	  break;
 
+#if 0
 	case WM_DROPFILES:
 	  f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
 
 	  if (f)
 	    w32_construct_drag_n_drop (&inev, &msg, f);
 	  break;
+#endif
+
+	case WM_EMACS_DROPSTRING:
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f)
+	      break;
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+	    inev.arg = make_string ((char*)msg.msg.lParam, strlen((char*)msg.msg.lParam));
+	    xfree ((void*)msg.msg.lParam);
+	    break;
+
+	case WM_EMACS_DROPFILES:
+	  {
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f)
+	      break;
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+
+	    Lisp_Object files = Qnil;
+	    char **filenames = (char**)msg.msg.lParam;
+	    for (int n = 0; filenames[n]; n++ )
+	      {
+		files = Fcons (DECODE_FILE (build_string (filenames[n])), files );
+		xfree (filenames[n]);
+	      }
+	    xfree (filenames);
+	    inev.arg = files;
+	    break;
+	  }
 
 	case WM_HSCROLL:
 	  {
diff --git a/src/w32term.h b/src/w32term.h
index 47be542f570..c1253af47c4 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -710,7 +710,9 @@ #define WM_EMACS_BRINGTOTOP            (WM_EMACS_START + 23)
 #define WM_EMACS_INPUT_READY           (WM_EMACS_START + 24)
 #define WM_EMACS_FILENOTIFY            (WM_EMACS_START + 25)
 #define WM_EMACS_IME_STATUS            (WM_EMACS_START + 26)
-#define WM_EMACS_END                   (WM_EMACS_START + 27)
+#define WM_EMACS_DROPFILES             (WM_EMACS_START + 27)
+#define WM_EMACS_DROPSTRING            (WM_EMACS_START + 28)
+#define WM_EMACS_END                   (WM_EMACS_START + 29)
 
 #define WND_FONTWIDTH_INDEX    (0)
 #define WND_LINEHEIGHT_INDEX   (4)
-- 
2.35.1.windows.2

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Sat, 28 Sep 2024 23:52:45 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> This patch implements drag-and-drop for w32 for files and text, using C,
> not C++.
> This should work from Windows 95, but I can't test it.  Because of this,
> the prior implementation with WM_DROPFILES has been removed with '#if
> 0', and can probably be completely removed.

I think we should remove those "#if 0" parts, yes.  If and when
someone is able to test this on Windows 9X, they will report problems
they encounter, and we can take it from there.  The removed code is
available in Git, so it is not lost.

> Text is inserted at point position, not at drop position.  I don't know
> how this works on other platforms.

Po Lu, any comments to this particular aspect?

I have some comments to the code below.

> Implement dnd with IDropTarget

This should explicitly mention MS-Windows.

> * lisp/term/w32-win.el (w32-drag-n-drop): changed to handle files or strings
> * src/w32fns.c
> (process_dropfiles): new, convert DROPFILES struct to array of strings
> (w32_createwindow): assign an IDropTarget to each new frame
> (w32_name_of_message): added new messages
> (w32_msg_pump): Changed CoInitialize to OleInitialize, needed by the
> drag-n-drop functions
> (w32_wnd_proc): new struct w32_drop_target, and w32_drop_target_*
> functions to implement the IDropTarget interface
> * src/w32term.c (w32_read_socket): handle WM_EMACS_DROPFILES,
> WM_EMACS_DROPSTRING, skip WM_EMACS_DROPFILES
> * src/w32term.h (): add WM_EMACS_DROPFILES, WM_EMACS_DROPSTRING

Some of these lines are too long (try limiting to 64 columns).  Also,
descriptions of changes should be complete sentences: start with a
capital letter and end in a period, like this:

 (w32_createwindow): Assign an IDropTarget to each new frame.

>  (defun w32-drag-n-drop (event &optional new-frame)
> -  "Edit the files listed in the drag-n-drop EVENT.
> -Switch to a buffer editing the last file dropped."
> +  "If the drag-n-drop EVENT is for a file or files, edit those
> +files. Switch to a buffer editing the last file dropped.

The first line of a doc string should be a single complete sentence.
In this case, I suggest:

    Perform drag-n-drop action according to data in EVENT.
  If EVENT is for one or more files, visit those files in corresponding
  buffers, and switch to the buffer that visits the last dropped file.
  If EVENT is for text, insert that text at point into the buffer
  shown in the window that is the target of the drop; if that buffer is
  read-only, add the dropped text to kill-ring.
  If the optional argument NEW-FRAME is non-nil, perform the
  drag-n-drop action in a newly-created frame using its selected-window
  and that window's buffer.

> +      (if (stringp arg)
> +          (dnd-insert-text window 'copy arg)
> +        (dnd-handle-multiple-urls
> +         window
> +         (mapcar #'w32-dropped-file-to-url arg)
> +         'private)))))

Please add a comment here saying that the payload in the EVENT should
be either a string (meaning the text to drop) or a list of strings
(meaning names of files to drop).  This is important because a file
name is also a string, so it is not obvious how we distinguish between
the two possible payloads.

> +/* From the DROPFILES struct, extract the list of filenames.  Returns a
> +   NULL terminated malloc array of malloc strings that should be freed
                      ^^^^^^          ^^^^^^
"malloced"

> +static char **
> +process_dropfiles (DROPFILES *files)
> +{
> +  char *start_of_files = (char*)files + files->pFiles;
                            ^^^^^^^^^^^^
Style: "(char *) files".

> +  int count = 0;
> +  char filename[MAX_PATH];
                   ^^^^^^^^
This should be MAX_UTF8_PATH, since filename[] is UTF-8 encoded.

> +  if (files->fWide)

What determines whether we get "wide" file names or ANSI file names?
We want to get "wide" file names where possible, but I don't see where
we request that?

> +    {
> +      WCHAR *p = (WCHAR*)start_of_files;
                    ^^^^^^^^^^^^^^^^^^^^^^
"(WCHAR *) start_of_files".

> +      p = (WCHAR*)start_of_files;

Likewise.

> +      for ( int i = 0; *p; p += wcslen (p) + 1, i++)
             ^^
Style: no space there.

> +      filenames = malloc ((count+1) * sizeof (char*));

You call malloc here, but the function which frees uses xfree.
We should probably call xmalloc here, but xmalloc could signal an
error, and this code runs in a non-main thread, doesn't it?  If so, I
think the function which frees needs to call 'free', nor 'xfree'.  And
we need to handle the case malloc returns NULL.

> +	  filenames [i] = xstrdup (filename);

For the same reason, we cannot call xstrdup here, and neither can we
call strdup (because it comes from MSVCRT, and will call a different
version of malloc).  We need to provide a private version of strdup.
But see below about moving all this processing to the main thread.

Also, "filenames[i]", without the space before the brackets.

> +  struct w32_drop_target *target = (struct w32_drop_target *)This;
> +  xfree( target->i_drop_target.lpVtbl );
> +  xfree( target );
         ^^^^
Style: missing and extra space again.

> +static HRESULT STDMETHODCALLTYPE
> +w32_drop_target_DragEnter( IDropTarget *This, IDataObject *pDataObj, DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)

Please break this long list of arguments into several lines.
Also, our style is to write

  w32_drop_target_DragEnter (IDropTarget *This,

That is, one space between the function name and the opening
parenthesis, and no space between the parenthesis and the first
argument.

> +static HRESULT STDMETHODCALLTYPE
> +w32_drop_target_DragOver ( IDropTarget *This, DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)

Likewise here (and elsewhere in the patch).

> +  formatetc.cfFormat = CF_UNICODETEXT;
> +  if (SUCCEEDED (IDataObject_GetData (pDataObj, &formatetc, &stgmedium)))
> +    {
> +      if (stgmedium.tymed == TYMED_HGLOBAL)
> +	{
> +	  WCHAR *text = (WCHAR*)GlobalLock (stgmedium.hGlobal);
> +	  Lisp_Object text_string = from_unicode_buffer (text);
> +	  char *utf8 = xstrdup (SSDATA (ENCODE_UTF_8 (text_string)));
> +	  my_post_msg (&msg, target->hwnd, WM_EMACS_DROPSTRING,
> +		       0, (LPARAM)utf8 );
> +	  GlobalUnlock (stgmedium.hGlobal);
> +	}
> +      ReleaseStgMedium (&stgmedium);
> +      return S_OK;
> +    }

If this runs in a non-main thread, then we cannot do this: calling
functions that manipulate Lisp data in non-main threads is a no-no.
Can we instead delegate the actual processing to where
WM_EMACS_DROPSTRING is processed?  If not, why not?

To tell the truth, I'd be happier if we could do the same with
WM_EMACS_DROPFILES, i.e. convert the file names and allocate memory
for them in the main thread: that would remove all the issues
mentioned above with xmalloc, xstrdup, etc., and be in general much
safer.

Also, there's no need to call ENCODE_UTF_8, since from_unicode_buffer
already returns the text in the internal representation.  You are
basically decoding strings from UTF-16, then encode them in UTF-8,
only to decode them again in the main thread.  (This will also be
avoided if we do everything in the main thread.)

> +  formatetc.cfFormat = CF_TEXT;
> +  if (SUCCEEDED (IDataObject_GetData (pDataObj, &formatetc, &stgmedium)))
> +    {
> +      if (stgmedium.tymed == TYMED_HGLOBAL)
> +	{
> +	  char *text = (char*)GlobalLock (stgmedium.hGlobal);
> +
> +	  int l = strlen (text);
> +	  WCHAR *text_utf16 = xmalloc (sizeof (WCHAR) * l + 1 );;
> +	  if (MultiByteToWideChar (CP_ACP, 0, text, l, text_utf16, l + 1))

We cannot call MultiByteToWideChar directly: it will prevent Emacs
from running on Windows 9X, where that function is available only
after loading a special DLL.  We need to call this function via a
function pointer, pMultiByteToWideChar.

> +	    {
> +	      Lisp_Object text_string = from_unicode_buffer (text_utf16);
> +	      char *utf8 = xstrdup (SSDATA (ENCODE_UTF_8 (text_string)));
> +	      my_post_msg (&msg, target->hwnd, WM_EMACS_DROPSTRING,
> +			   0, (LPARAM)utf8 );

This is a very convoluted way of converting ANSI encoded string to
UTF-8.  I would simply use DECODE_SYSTEM here (subject to the same
restriction regarding doing that in a non-main thread).

> @@ -2548,7 +2720,23 @@ w32_createwindow (struct frame *f, int *coords)
>        SetWindowLong (hwnd, WND_BACKGROUND_INDEX, FRAME_BACKGROUND_PIXEL (f));
>  
>        /* Enable drag-n-drop.  */
> +      struct w32_drop_target *drop_target = xmalloc (sizeof (struct w32_drop_target));
> +      drop_target-> hwnd = hwnd;
> +
> +      IDropTargetVtbl *vtbl = xmalloc (sizeof (IDropTargetVtbl));
                                 ^^^^^^^
w32_createwindow runs in the input thread, so it cannot safely call
xmalloc.

>  	    case WM_EMACS_CREATEWINDOW:
> -	      /* Initialize COM for this window. Even though we don't use it,
> -		 some third party shell extensions can cause it to be used in
> +	      /* Initialize COM for this window. Needed for RegisterDragDrop.
                                               ^^
Two spaces between sentences, please.

> +	case WM_EMACS_DROPSTRING:
> +	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
> +	    if (!f)
> +	      break;
> +	    XSETFRAME (inev.frame_or_window, f);
> +	    inev.kind = DRAG_N_DROP_EVENT;
> +	    inev.code = 0;
> +	    inev.timestamp = msg.msg.time;
> +	    inev.modifiers = msg.dwModifiers;
> +	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
> +	    XSETINT (inev.x, msg.msg.pt.x);
> +	    XSETINT (inev.y, msg.msg.pt.y);
> +	    inev.arg = make_string ((char*)msg.msg.lParam, strlen((char*)msg.msg.lParam));

Since we are (hopefully) going to move the prcessing and decoding of
the dropped string to this place, just decoding it from UTF-18 or ANSI
(via DECODE_SYSTEM) should be enough.

> +	    xfree ((void*)msg.msg.lParam);

This may need to be 'free', see above.

> +	case WM_EMACS_DROPFILES:
> +	  {
> +	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
> +	    if (!f)
> +	      break;
> +	    XSETFRAME (inev.frame_or_window, f);
> +	    inev.kind = DRAG_N_DROP_EVENT;
> +	    inev.code = 0;
> +	    inev.timestamp = msg.msg.time;
> +	    inev.modifiers = msg.dwModifiers;
> +	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
> +	    XSETINT (inev.x, msg.msg.pt.x);
> +	    XSETINT (inev.y, msg.msg.pt.y);
> +
> +	    Lisp_Object files = Qnil;
> +	    char **filenames = (char**)msg.msg.lParam;
> +	    for (int n = 0; filenames[n]; n++ )
> +	      {
> +		files = Fcons (DECODE_FILE (build_string (filenames[n])), files );
> +		xfree (filenames[n]);
> +	      }
> +	    xfree (filenames);

Similarly here.

Finally, we need a NEWS entry (near the end of NEWS) announcing this
new capability.

Thanks!

bug#3468: drag and drop text

[Cecilio Pardo]

On 29/09/2024 9:19, Eli Zaretskii wrote:

>> +  if (files->fWide)
> 
> What determines whether we get "wide" file names or ANSI file names?
> We want to get "wide" file names where possible, but I don't see where
> we request that?

We can't request it, I suppose it depends on the OS version or the 
program sending the drop.

> If this runs in a non-main thread, then we cannot do this: calling
> functions that manipulate Lisp data in non-main threads is a no-no.
> Can we instead delegate the actual processing to where
> WM_EMACS_DROPSTRING is processed?  If not, why not?

It runs indeed on the "shell thread". I'll move all the processing to 
the message handler.

Thanks a lot for your review, and sorry for all the style mistakes.

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Sun, 29 Sep 2024 13:17:26 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 29/09/2024 9:19, Eli Zaretskii wrote:
> 
> >> +  if (files->fWide)
> > 
> > What determines whether we get "wide" file names or ANSI file names?
> > We want to get "wide" file names where possible, but I don't see where
> > we request that?
> 
> We can't request it, I suppose it depends on the OS version or the 
> program sending the drop.

What did you see on Windows 11?  Do we always receive "wide" file
names?  Did you try dragging file names whose characters are not
supported by the current ANSI codepage, and if so, did it work?

> > If this runs in a non-main thread, then we cannot do this: calling
> > functions that manipulate Lisp data in non-main threads is a no-no.
> > Can we instead delegate the actual processing to where
> > WM_EMACS_DROPSTRING is processed?  If not, why not?
> 
> It runs indeed on the "shell thread". I'll move all the processing to 
> the message handler.

Thanks.  By "message handler" you mean w32_read_socket, yes?

> Thanks a lot for your review, and sorry for all the style mistakes.

No need to apologize, it takes time to learn the subtleties of our
conventions.

bug#3468: drag and drop text

[Cecilio Pardo]

On 29/09/2024 13:36, Eli Zaretskii wrote:

> What did you see on Windows 11?  Do we always receive "wide" file
> names?  

Yes, always wide on Windows 11 on my tests.

 > Did you try dragging file names whose characters are not supported by 
 > the current ANSI codepage, and if so, did it work?

I just tried and it works fine.

>> It runs indeed on the "shell thread". I'll move all the processing to
>> the message handler.
> 
> Thanks.  By "message handler" you mean w32_read_socket, yes?

Yes.

bug#3468: drag and drop text

[Cecilio Pardo]

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

Here is the reworked version.

Now the processing of the drop data is delayed until we process the 
WM_EMACS_DROP message at w32_read_socket.

There is no malloc/strdup.




[-- Attachment #2: 0001-Implement-drag-n-drop-for-w32-with-support-for-files.patch --]
[-- Type: text/plain, Size: 15200 bytes --]

From c20e8e3e4880eb65257020ee88298f5697b1dfc3 Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Fri, 27 Sep 2024 23:58:02 +0200
Subject: [PATCH] Implement drag-n-drop for w32 with support for files and text

Implement drag-n-drop with IDropTarget for MS-Windows. This
allows for dropping files or text.

* lisp/term/w32-win.el (w32-drag-n-drop): Changed to handle
files or strings.
* src/w32fns.c (process_dropfiles): New function to convert
DROPFILES struct to array of strings.
(w32_process_dnd_data): New function to process drop data.
(w32_try_get_data): Extract data from IDataObject.
(w32_createwindow): Assign an IDropTarget to each new frame.
(w32_name_of_message): Added new message.
(w32_msg_pump): Changed CoInitialize to OleInitialize, needed
by the drag-n-drop functions.
(w32_wnd_proc): New struct w32_drop_target, and
w32_drop_target_* functions to implement the IDropTarget
interface.
* src/w32term.c (w32_read_socket): Handle WM_EMACS_DROP and
remove WM_EMACS_DROPFILES.
* src/w32term.h (): Added new message WM_EMACS_DROP.

..
---
 etc/NEWS             |   5 ++
 lisp/term/w32-win.el |  25 ++++--
 src/w32fns.c         | 203 +++++++++++++++++++++++++++++++++++++++++--
 src/w32term.c        |  98 ++++-----------------
 src/w32term.h        |   4 +-
 5 files changed, 242 insertions(+), 93 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 37568ffdbea..7c9f6f559ec 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -584,6 +584,11 @@ In particular, it is now possible to show text with embedded newlines in
 a dialog popped by 'message-box'.  This is supported on Windows Vista
 and later versions.
 
+---
+** Emacs on MS-Windows now supports drag-n-drop of text into a buffer.
+This is in addition to drag-n-drop of files, that was already supported.
+
+
 \f
 ----------------------------------------------------------------------
 This file is part of GNU Emacs.
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index b57b3dd3bef..541fef2ced3 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -131,8 +131,15 @@ w32-dropped-file-to-url
    file-name))
 
 (defun w32-drag-n-drop (event &optional new-frame)
-  "Edit the files listed in the drag-n-drop EVENT.
-Switch to a buffer editing the last file dropped."
+  "Perform drag-n-drop action according to data in EVENT.
+If EVENT is for one or more files, visit those files in corresponding
+buffers, and switch to the buffer that visits the last dropped file.
+If EVENT is for text, insert that text at point into the buffer
+shown in the window that is the target of the drop; if that buffer is
+read-only, add the dropped text to kill-ring.
+If the optional argument NEW-FRAME is non-nil, perform the
+drag-n-drop action in a newly-created frame using its selected-window
+and that window's buffer."
   (interactive "e")
   (save-excursion
     ;; Make sure the drop target has positive co-ords
@@ -140,6 +147,7 @@ w32-drag-n-drop
     ;; won't work.  <skx@tardis.ed.ac.uk>
     (let* ((window (posn-window (event-start event)))
 	   (coords (posn-x-y (event-start event)))
+           (arg (car (cdr (cdr event))))
 	   (x (car coords))
 	   (y (cdr coords)))
       (if (and (> x 0) (> y 0))
@@ -150,11 +158,14 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (dnd-handle-multiple-urls
-       window
-       (mapcar #'w32-dropped-file-to-url
-               (car (cdr (cdr event))))
-       'private))))
+      ;; arg (the payload of the event) is a string when the drop is
+      ;; text, and a list of strings when the drop is one or more files.
+      (if (stringp arg)
+          (dnd-insert-text window 'copy arg)
+        (dnd-handle-multiple-urls
+         window
+         (mapcar #'w32-dropped-file-to-url arg)
+         'private)))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
diff --git a/src/w32fns.c b/src/w32fns.c
index 0a3f5c38a58..dc69a398019 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -34,6 +34,12 @@ #define _WIN32_WINNT 0x0600
 
 #include <c-ctype.h>
 
+#define COBJMACROS /* Ask for C definitions for COM.  */
+#include <shlobj.h>
+#include <oleidl.h>
+#include <objidl.h>
+#include <ole2.h>
+
 #include "lisp.h"
 #include "w32term.h"
 #include "frame.h"
@@ -359,6 +365,9 @@ #define WS_EX_NOACTIVATE 0x08000000L
 
 static struct w32_display_info *w32_display_info_for_name (Lisp_Object);
 
+static void my_post_msg (W32Msg*, HWND, UINT, WPARAM, LPARAM);
+static unsigned int w32_get_modifiers (void);
+
 /* Let the user specify a display with a frame.
    nil stands for the selected frame--or, if that is not a w32 frame,
    the first display on the list.  */
@@ -2464,6 +2473,174 @@ w32_createhscrollbar (struct frame *f, struct scroll_bar * bar)
   return hwnd;
 }
 
+/* From the DROPFILES struct, extract the filenames and return as a list
+   of strings.  */
+static Lisp_Object
+process_dropfiles (DROPFILES *files)
+{
+  char *start_of_files = (char *) files + files->pFiles;
+  char filename[MAX_UTF8_PATH];
+  Lisp_Object lisp_files = Qnil;
+
+  if (files->fWide)
+    {
+      WCHAR *p = (WCHAR *) start_of_files;
+      for (; *p; p += wcslen (p) + 1)
+	{
+	  filename_from_utf16(p, filename);
+	  lisp_files = Fcons (DECODE_FILE (build_string (filename)),
+			      lisp_files );
+	}
+    }
+  else
+    {
+      char *p = start_of_files;
+      for (; *p; p += strlen(p) + 1)
+	{
+	  filename_from_ansi (p, filename);
+	  lisp_files = Fcons (DECODE_FILE (build_string (filename)),
+			      lisp_files );
+	}
+    }
+  return lisp_files;
+}
+
+Lisp_Object
+w32_process_dnd_data (int format, void *hGlobal)
+{
+  Lisp_Object result = Qnil;
+  HGLOBAL hg = (HGLOBAL) hGlobal;
+
+  switch (format)
+    {
+    case CF_HDROP:
+      {
+	DROPFILES *files = (DROPFILES *) GlobalLock (hg);
+	if (files)
+	  result = process_dropfiles (files);
+	GlobalUnlock (hg);
+	break;
+      }
+    case CF_UNICODETEXT:
+      {
+	WCHAR *text = (WCHAR *) GlobalLock (hg);
+	result = from_unicode_buffer (text);
+	GlobalUnlock (hg);
+	break;
+      }
+    case CF_TEXT:
+      {
+	char *text = (char *) GlobalLock (hg);
+	result = DECODE_SYSTEM (build_unibyte_string (text));
+	GlobalUnlock (hg);
+	break;
+      }
+    }
+
+  GlobalFree (hg);
+
+  return result;
+}
+
+struct w32_drop_target {
+  /* i_drop_target must be the first member.  */
+  IDropTarget i_drop_target;
+  HWND hwnd;
+};
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_QueryInterface (IDropTarget *t, REFIID ri, void **r)
+{
+  return E_NOINTERFACE;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_AddRef (IDropTarget *This)
+{
+  return 1;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_Release (IDropTarget *This)
+{
+  struct w32_drop_target *target = (struct w32_drop_target * ) This;
+  free (target->i_drop_target.lpVtbl);
+  free (target);
+  return 0;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
+			   DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragOver (IDropTarget *This, DWORD grfKeyState, POINTL pt,
+			  DWORD *pdwEffect)
+{
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragLeave (IDropTarget *This)
+{
+  return S_OK;
+}
+
+static HGLOBAL w32_try_get_data (IDataObject *pDataObj, int format)
+{
+  FORMATETC formatetc = { format, NULL, DVASPECT_CONTENT, -1,
+			  TYMED_HGLOBAL };
+  STGMEDIUM stgmedium;
+  HRESULT r = IDataObject_GetData (pDataObj, &formatetc, &stgmedium);
+  if (SUCCEEDED (r))
+    {
+      if (stgmedium.tymed == TYMED_HGLOBAL)
+	return stgmedium.hGlobal;
+      ReleaseStgMedium (&stgmedium);
+    }
+  return NULL;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_Drop (IDropTarget *This, IDataObject *pDataObj,
+		      DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+  *pdwEffect = DROPEFFECT_COPY;
+
+  W32Msg msg = {0};
+  msg.dwModifiers = w32_get_modifiers();
+  msg.msg.time = GetMessageTime ();
+  msg.msg.pt.x = pt.x;
+  msg.msg.pt.y = pt.y;
+
+  int format = CF_HDROP;
+  HGLOBAL hGlobal = w32_try_get_data (pDataObj, format);
+
+  if (!hGlobal)
+    {
+      format = CF_UNICODETEXT;
+      hGlobal = w32_try_get_data (pDataObj, format);
+    }
+
+  if (!hGlobal)
+    {
+      format = CF_TEXT;
+      hGlobal = w32_try_get_data (pDataObj, format);
+    }
+
+  if (hGlobal)
+      my_post_msg (&msg, target->hwnd, WM_EMACS_DROP, format,
+		   (LPARAM) hGlobal);
+
+  return S_OK;
+}
+
 static void
 w32_createwindow (struct frame *f, int *coords)
 {
@@ -2548,7 +2725,20 @@ w32_createwindow (struct frame *f, int *coords)
       SetWindowLong (hwnd, WND_BACKGROUND_INDEX, FRAME_BACKGROUND_PIXEL (f));
 
       /* Enable drag-n-drop.  */
-      DragAcceptFiles (hwnd, TRUE);
+      struct w32_drop_target *drop_target =
+	malloc (sizeof (struct w32_drop_target));
+      drop_target->hwnd = hwnd;
+
+      IDropTargetVtbl *vtbl = malloc (sizeof (IDropTargetVtbl));
+      drop_target->i_drop_target.lpVtbl = vtbl;
+      vtbl->QueryInterface = w32_drop_target_QueryInterface;
+      vtbl->AddRef = w32_drop_target_AddRef;
+      vtbl->Release = w32_drop_target_Release;
+      vtbl->DragEnter = w32_drop_target_DragEnter;
+      vtbl->DragOver = w32_drop_target_DragOver;
+      vtbl->DragLeave = w32_drop_target_DragLeave;
+      vtbl->Drop = w32_drop_target_Drop;
+      RegisterDragDrop (hwnd, &drop_target->i_drop_target);
 
       /* Enable system light/dark theme.  */
       w32_applytheme (hwnd);
@@ -3399,6 +3589,7 @@ #define M(msg) { msg, # msg }
       M (WM_EMACS_PAINT),
       M (WM_EMACS_IME_STATUS),
       M (WM_CHAR),
+      M (WM_EMACS_DROP),
 #undef M
       { 0, 0 }
   };
@@ -3465,13 +3656,14 @@ w32_msg_pump (deferred_msg * msg_buf)
 	      /* Produced by complete_deferred_msg; just ignore.  */
 	      break;
 	    case WM_EMACS_CREATEWINDOW:
-	      /* Initialize COM for this window. Even though we don't use it,
-		 some third party shell extensions can cause it to be used in
+	      /* Initialize COM for this window.  Needed for RegisterDragDrop.
+		 Some third party shell extensions can cause it to be used in
 		 system dialogs, which causes a crash if it is not initialized.
 		 This is a known bug in Windows, which was fixed long ago, but
 		 the patch for XP is not publicly available until XP SP3,
 		 and older versions will never be patched.  */
-	      CoInitialize (NULL);
+	      OleInitialize (NULL);
+
 	      w32_createwindow ((struct frame *) msg.wParam,
 				(int *) msg.lParam);
 	      if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
@@ -5106,7 +5298,6 @@ #define MOUSEEVENTF_FROMTOUCH 0xFF515700
       return 0;
 
     case WM_MOUSEWHEEL:
-    case WM_DROPFILES:
       wmsg.dwModifiers = w32_get_modifiers ();
       my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
       signal_user_input ();
@@ -5597,7 +5788,7 @@ #define WM_TOUCH 576
       }
 
     case WM_EMACS_DESTROYWINDOW:
-      DragAcceptFiles ((HWND) wParam, FALSE);
+      RevokeDragDrop ((HWND) wParam);
       return DestroyWindow ((HWND) wParam);
 
     case WM_EMACS_HIDE_CARET:
diff --git a/src/w32term.c b/src/w32term.c
index 62037e3b2cd..3a627308137 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -3576,81 +3576,6 @@ w32_construct_mouse_wheel (struct input_event *result, W32Msg *msg,
   return Qnil;
 }
 
-static Lisp_Object
-w32_construct_drag_n_drop (struct input_event *result, W32Msg *msg,
-                           struct frame *f)
-{
-  Lisp_Object files;
-  Lisp_Object frame;
-  HDROP hdrop;
-  POINT p;
-  WORD num_files;
-  wchar_t name_w[MAX_PATH];
-#ifdef NTGUI_UNICODE
-  const int use_unicode = 1;
-#else
-  int use_unicode = w32_unicode_filenames;
-  char name_a[MAX_PATH];
-  char file[MAX_UTF8_PATH];
-#endif
-  int i;
-
-  result->kind = DRAG_N_DROP_EVENT;
-  result->code = 0;
-  result->timestamp = msg->msg.time;
-  result->modifiers = msg->dwModifiers;
-
-  hdrop = (HDROP) msg->msg.wParam;
-  DragQueryPoint (hdrop, &p);
-
-#if 0
-  p.x = LOWORD (msg->msg.lParam);
-  p.y = HIWORD (msg->msg.lParam);
-  ScreenToClient (msg->msg.hwnd, &p);
-#endif
-
-  XSETINT (result->x, p.x);
-  XSETINT (result->y, p.y);
-
-  num_files = DragQueryFile (hdrop, 0xFFFFFFFF, NULL, 0);
-  files = Qnil;
-
-  for (i = 0; i < num_files; i++)
-    {
-      if (use_unicode)
-	{
-	  eassert (DragQueryFileW (hdrop, i, NULL, 0) < MAX_PATH);
-	  /* If DragQueryFile returns zero, it failed to fetch a file
-	     name.  */
-	  if (DragQueryFileW (hdrop, i, name_w, MAX_PATH) == 0)
-	    continue;
-#ifdef NTGUI_UNICODE
-	  files = Fcons (from_unicode_buffer (name_w), files);
-#else
-	  filename_from_utf16 (name_w, file);
-	  files = Fcons (DECODE_FILE (build_unibyte_string (file)), files);
-#endif /* NTGUI_UNICODE */
-	}
-#ifndef NTGUI_UNICODE
-      else
-	{
-	  eassert (DragQueryFileA (hdrop, i, NULL, 0) < MAX_PATH);
-	  if (DragQueryFileA (hdrop, i, name_a, MAX_PATH) == 0)
-	    continue;
-	  filename_from_ansi (name_a, file);
-	  files = Fcons (DECODE_FILE (build_unibyte_string (file)), files);
-	}
-#endif
-    }
-
-  DragFinish (hdrop);
-
-  XSETFRAME (frame, f);
-  result->frame_or_window = frame;
-  result->arg = files;
-  return Qnil;
-}
-
 \f
 #if HAVE_W32NOTIFY
 
@@ -5682,11 +5607,26 @@ w32_read_socket (struct terminal *terminal,
 	  }
 	  break;
 
-	case WM_DROPFILES:
-	  f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	case WM_EMACS_DROP:
+	  {
+	    int format = msg.msg.wParam;
+	    Lisp_Object drop_object =
+	      w32_process_dnd_data (format, (void *) msg.msg.lParam);
 
-	  if (f)
-	    w32_construct_drag_n_drop (&inev, &msg, f);
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f || NILP (drop_object))
+	      break;
+
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+	    inev.arg = drop_object;
+	  }
 	  break;
 
 	case WM_HSCROLL:
diff --git a/src/w32term.h b/src/w32term.h
index 47be542f570..39e2262e2a8 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -272,6 +272,7 @@ #define CP_DEFAULT 1004
 
 /* w32fns.c */
 extern void w32_default_font_parameter (struct frame* f, Lisp_Object parms);
+extern Lisp_Object w32_process_dnd_data (int format, void *pDataObj);
 
 \f
 #define PIX_TYPE COLORREF
@@ -710,7 +711,8 @@ #define WM_EMACS_BRINGTOTOP            (WM_EMACS_START + 23)
 #define WM_EMACS_INPUT_READY           (WM_EMACS_START + 24)
 #define WM_EMACS_FILENOTIFY            (WM_EMACS_START + 25)
 #define WM_EMACS_IME_STATUS            (WM_EMACS_START + 26)
-#define WM_EMACS_END                   (WM_EMACS_START + 27)
+#define WM_EMACS_DROP                  (WM_EMACS_START + 27)
+#define WM_EMACS_END                   (WM_EMACS_START + 28)
 
 #define WND_FONTWIDTH_INDEX    (0)
 #define WND_LINEHEIGHT_INDEX   (4)
-- 
2.35.1.windows.2

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Mon, 30 Sep 2024 23:20:32 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> Cc: 3468@debbugs.gnu.org
> 
> Here is the reworked version.
> 
> Now the processing of the drop data is delayed until we process the 
> WM_EMACS_DROP message at w32_read_socket.
> 
> There is no malloc/strdup.

Well, there are still 2 calls to malloc in w32_createwindow.  Any
reasons not to call xmalloc instead?  Can w32_createwindow be called
before we call init_heap?

And I have a few minor comments below.

> +  if (files->fWide)
> +    {
> +      WCHAR *p = (WCHAR *) start_of_files;
> +      for (; *p; p += wcslen (p) + 1)
> +	{
> +	  filename_from_utf16(p, filename);
> +	  lisp_files = Fcons (DECODE_FILE (build_string (filename)),
                                           ^^^^^^^^^^^^
This should use build_unibyte_string instead, since we know we want a
unibyte string here, which we then decode.

> +      for (; *p; p += strlen(p) + 1)
> +	{
> +	  filename_from_ansi (p, filename);
> +	  lisp_files = Fcons (DECODE_FILE (build_string (filename)),
> +			      lisp_files );

Same here.

> +Lisp_Object
> +w32_process_dnd_data (int format, void *hGlobal)
> +{
> +  Lisp_Object result = Qnil;
> +  HGLOBAL hg = (HGLOBAL) hGlobal;
> +
> +  switch (format)
> +    {
> +    case CF_HDROP:
> +      {
> +	DROPFILES *files = (DROPFILES *) GlobalLock (hg);
> +	if (files)
> +	  result = process_dropfiles (files);
> +	GlobalUnlock (hg);
> +	break;
> +      }
> +    case CF_UNICODETEXT:
> +      {
> +	WCHAR *text = (WCHAR *) GlobalLock (hg);
> +	result = from_unicode_buffer (text);
> +	GlobalUnlock (hg);
> +	break;
> +      }
> +    case CF_TEXT:
> +      {
> +	char *text = (char *) GlobalLock (hg);
> +	result = DECODE_SYSTEM (build_unibyte_string (text));
> +	GlobalUnlock (hg);
> +	break;
> +      }
> +    }
> +
> +  GlobalFree (hg);

This function should call block_input at entry and unblock_input
before it exits, because we must make sure all the GlobalLock calls
are paired with GlobalUnlock and GlobalFree, while functions we call
here, process_dropfiles etc., all cons Lisp object, and therefore
could potentially QUIT, which throws to top-level.

> +static HRESULT STDMETHODCALLTYPE
> +w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
> +			   DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
> +{
> +  *pdwEffect = DROPEFFECT_COPY;
> +  return S_OK;
> +}
> +
> +static HRESULT STDMETHODCALLTYPE
> +w32_drop_target_DragOver (IDropTarget *This, DWORD grfKeyState, POINTL pt,
> +			  DWORD *pdwEffect)
> +{
> +  *pdwEffect = DROPEFFECT_COPY;
> +  return S_OK;

It would be good here to have comments that explain the meaning of
DROPEFFECT_COPY and the rationale for returning this value.

> @@ -2548,7 +2725,20 @@ w32_createwindow (struct frame *f, int *coords)
>        SetWindowLong (hwnd, WND_BACKGROUND_INDEX, FRAME_BACKGROUND_PIXEL (f));
>  
>        /* Enable drag-n-drop.  */
> -      DragAcceptFiles (hwnd, TRUE);
> +      struct w32_drop_target *drop_target =
> +	malloc (sizeof (struct w32_drop_target));
> +      drop_target->hwnd = hwnd;
> +
> +      IDropTargetVtbl *vtbl = malloc (sizeof (IDropTargetVtbl));
> +      drop_target->i_drop_target.lpVtbl = vtbl;
> +      vtbl->QueryInterface = w32_drop_target_QueryInterface;
> +      vtbl->AddRef = w32_drop_target_AddRef;
> +      vtbl->Release = w32_drop_target_Release;
> +      vtbl->DragEnter = w32_drop_target_DragEnter;
> +      vtbl->DragOver = w32_drop_target_DragOver;
> +      vtbl->DragLeave = w32_drop_target_DragLeave;
> +      vtbl->Drop = w32_drop_target_Drop;
> +      RegisterDragDrop (hwnd, &drop_target->i_drop_target);

Do we ever have to free the two memory chunks we allocate here?  Like
when the window is disposed of?

bug#3468: drag and drop text

[Cecilio Pardo]

On 05/10/2024 13:07, Eli Zaretskii wrote:

>> There is no malloc/strdup.
> 
> Well, there are still 2 calls to malloc in w32_createwindow.  Any
> reasons not to call xmalloc instead?  Can w32_createwindow be called
> before we call init_heap?

[...]

> Do we ever have to free the two memory chunks we allocate here?  Like
> when the window is disposed of?

They are freed in w32_drop_target_Release, which is called internally 
when we call RevokeDragDrop while handling the WW_EMACS_DESTROYWINDOW on 
w32_wnd_proc.

As this runs on the input thread, I used malloc/free instead of 
xmalloc/xfree.  I could send a message to handle on w32_read_socket and 
then use xmalloc.

> This function should call block_input at entry and unblock_input
> before it exits, because we must make sure all the GlobalLock calls
> are paired with GlobalUnlock and GlobalFree, while functions we call
> here, process_dropfiles etc., all cons Lisp object, and therefore
> could potentially QUIT, which throws to top-level.

The function (w32_process_dnd_data) is called from within the 
block_input/unblock_input on w32_read_socket.

Should I add its own calls for clarity? Maybe just a comment?

Thanks.

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Sat, 5 Oct 2024 14:07:01 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 05/10/2024 13:07, Eli Zaretskii wrote:
> 
> >> There is no malloc/strdup.
> > 
> > Well, there are still 2 calls to malloc in w32_createwindow.  Any
> > reasons not to call xmalloc instead?  Can w32_createwindow be called
> > before we call init_heap?
> 
> [...]
> 
> > Do we ever have to free the two memory chunks we allocate here?  Like
> > when the window is disposed of?
> 
> They are freed in w32_drop_target_Release, which is called internally 
> when we call RevokeDragDrop while handling the WW_EMACS_DESTROYWINDOW on 
> w32_wnd_proc.
> 
> As this runs on the input thread, I used malloc/free instead of 
> xmalloc/xfree.  I could send a message to handle on w32_read_socket and 
> then use xmalloc.

It's okay to use malloc/free in this case, but we must check the
return value of malloc to not be NULL.

> > This function should call block_input at entry and unblock_input
> > before it exits, because we must make sure all the GlobalLock calls
> > are paired with GlobalUnlock and GlobalFree, while functions we call
> > here, process_dropfiles etc., all cons Lisp object, and therefore
> > could potentially QUIT, which throws to top-level.
> 
> The function (w32_process_dnd_data) is called from within the 
> block_input/unblock_input on w32_read_socket.

Ah, okay.

> Should I add its own calls for clarity? Maybe just a comment?

Yes, please.

bug#3468: drag and drop text

[Eli Zaretskii]

> Cc: 3468@debbugs.gnu.org
> Date: Sat, 05 Oct 2024 15:30:45 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Should I add its own calls for clarity? Maybe just a comment?
> 
> Yes, please.

I meant to say that a comment will be enough.

bug#3468: drag and drop text

[Cecilio Pardo]

Here is the new version of the patch, with everything addressed.

Looking at the manual, in the "Drag and Drop" chapter it says:

 >    It can be difficult to scroll a window or determine where dropped
 > text will be inserted while dragging text onto an Emacs window.
 > Setting the option ‘dnd-indicate-insertion-point’ to a non-‘nil’ value
 > makes point move to the location any dropped text will be inserted
 > when the mouse moves in a window during drag, and setting
 > ‘dnd-scroll-margin’ to an integer value causes a window to be scrolled
 > if the mouse moves within that many lines of the top or bottom of the
 > window during drag.

This should be implemented on Windows too, but it will take me some 
time. Maybe you prefer to wait before installing the changes.

Also, do you think the manual should be updated too?

bug#3468: drag and drop text

[Cecilio Pardo]

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

This time with the attached patch.

[-- Attachment #2: 0001-Implement-drag-n-drop-for-w32-with-support-for-files.patch --]
[-- Type: text/plain, Size: 15776 bytes --]

From 8ba7afb223afd5c81eb07991141a229d41a9de03 Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Fri, 27 Sep 2024 23:58:02 +0200
Subject: [PATCH] Implement drag-n-drop for w32 with support for files and text

Implement drag-n-drop with IDropTarget for MS-Windows. This
allows for dropping files or text.

* lisp/term/w32-win.el (w32-drag-n-drop): Changed to handle
files or strings.
* src/w32fns.c (process_dropfiles): New function to convert
DROPFILES struct to array of strings.
(w32_process_dnd_data): New function to process drop data.
(w32_try_get_data): Extract data from IDataObject.
(w32_createwindow): Assign an IDropTarget to each new frame.
(w32_name_of_message): Added new message.
(w32_msg_pump): Changed CoInitialize to OleInitialize, needed
by the drag-n-drop functions.
(w32_wnd_proc): New struct w32_drop_target, and
w32_drop_target_* functions to implement the IDropTarget
interface.
* src/w32term.c (w32_read_socket): Handle WM_EMACS_DROP and
remove WM_EMACS_DROPFILES.
* src/w32term.h (): Added new message WM_EMACS_DROP.
---
 etc/NEWS             |   5 +
 lisp/term/w32-win.el |  25 +++--
 src/w32fns.c         | 221 +++++++++++++++++++++++++++++++++++++++++--
 src/w32term.c        |  98 ++++---------------
 src/w32term.h        |   4 +-
 5 files changed, 260 insertions(+), 93 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index d1bd469435f..8d01e03d036 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -635,6 +635,11 @@ In particular, it is now possible to show text with embedded newlines in
 a dialog popped by 'message-box'.  This is supported on Windows Vista
 and later versions.
 
+---
+** Emacs on MS-Windows now supports drag-n-drop of text into a buffer.
+This is in addition to drag-n-drop of files, that was already supported.
+
+
 \f
 ----------------------------------------------------------------------
 This file is part of GNU Emacs.
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index b57b3dd3bef..541fef2ced3 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -131,8 +131,15 @@ w32-dropped-file-to-url
    file-name))
 
 (defun w32-drag-n-drop (event &optional new-frame)
-  "Edit the files listed in the drag-n-drop EVENT.
-Switch to a buffer editing the last file dropped."
+  "Perform drag-n-drop action according to data in EVENT.
+If EVENT is for one or more files, visit those files in corresponding
+buffers, and switch to the buffer that visits the last dropped file.
+If EVENT is for text, insert that text at point into the buffer
+shown in the window that is the target of the drop; if that buffer is
+read-only, add the dropped text to kill-ring.
+If the optional argument NEW-FRAME is non-nil, perform the
+drag-n-drop action in a newly-created frame using its selected-window
+and that window's buffer."
   (interactive "e")
   (save-excursion
     ;; Make sure the drop target has positive co-ords
@@ -140,6 +147,7 @@ w32-drag-n-drop
     ;; won't work.  <skx@tardis.ed.ac.uk>
     (let* ((window (posn-window (event-start event)))
 	   (coords (posn-x-y (event-start event)))
+           (arg (car (cdr (cdr event))))
 	   (x (car coords))
 	   (y (cdr coords)))
       (if (and (> x 0) (> y 0))
@@ -150,11 +158,14 @@ w32-drag-n-drop
       (raise-frame)
       (setq window (selected-window))
 
-      (dnd-handle-multiple-urls
-       window
-       (mapcar #'w32-dropped-file-to-url
-               (car (cdr (cdr event))))
-       'private))))
+      ;; arg (the payload of the event) is a string when the drop is
+      ;; text, and a list of strings when the drop is one or more files.
+      (if (stringp arg)
+          (dnd-insert-text window 'copy arg)
+        (dnd-handle-multiple-urls
+         window
+         (mapcar #'w32-dropped-file-to-url arg)
+         'private)))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
diff --git a/src/w32fns.c b/src/w32fns.c
index 0a3f5c38a58..b3d26b841e4 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -34,6 +34,12 @@ #define _WIN32_WINNT 0x0600
 
 #include <c-ctype.h>
 
+#define COBJMACROS /* Ask for C definitions for COM.  */
+#include <shlobj.h>
+#include <oleidl.h>
+#include <objidl.h>
+#include <ole2.h>
+
 #include "lisp.h"
 #include "w32term.h"
 #include "frame.h"
@@ -359,6 +365,9 @@ #define WS_EX_NOACTIVATE 0x08000000L
 
 static struct w32_display_info *w32_display_info_for_name (Lisp_Object);
 
+static void my_post_msg (W32Msg*, HWND, UINT, WPARAM, LPARAM);
+static unsigned int w32_get_modifiers (void);
+
 /* Let the user specify a display with a frame.
    nil stands for the selected frame--or, if that is not a w32 frame,
    the first display on the list.  */
@@ -2464,6 +2473,182 @@ w32_createhscrollbar (struct frame *f, struct scroll_bar * bar)
   return hwnd;
 }
 
+/* From the DROPFILES struct, extract the filenames and return as a list
+   of strings.  */
+static Lisp_Object
+process_dropfiles (DROPFILES *files)
+{
+  char *start_of_files = (char *) files + files->pFiles;
+  char filename[MAX_UTF8_PATH];
+  Lisp_Object lisp_files = Qnil;
+
+  if (files->fWide)
+    {
+      WCHAR *p = (WCHAR *) start_of_files;
+      for (; *p; p += wcslen (p) + 1)
+	{
+	  filename_from_utf16(p, filename);
+	  lisp_files = Fcons (DECODE_FILE (build_unibyte_string (filename)),
+			      lisp_files );
+	}
+    }
+  else
+    {
+      char *p = start_of_files;
+      for (; *p; p += strlen(p) + 1)
+	{
+	  filename_from_ansi (p, filename);
+	  lisp_files = Fcons (DECODE_FILE (build_unibyte_string (filename)),
+			      lisp_files );
+	}
+    }
+  return lisp_files;
+}
+
+
+/* This function can be called ONLY between calls to
+   block_input/unblock_input.  It is used in w32_read_socket.  */
+Lisp_Object
+w32_process_dnd_data (int format, void *hGlobal)
+{
+  Lisp_Object result = Qnil;
+  HGLOBAL hg = (HGLOBAL) hGlobal;
+
+  switch (format)
+    {
+    case CF_HDROP:
+      {
+	DROPFILES *files = (DROPFILES *) GlobalLock (hg);
+	if (files)
+	  result = process_dropfiles (files);
+	GlobalUnlock (hg);
+	break;
+      }
+    case CF_UNICODETEXT:
+      {
+	WCHAR *text = (WCHAR *) GlobalLock (hg);
+	result = from_unicode_buffer (text);
+	GlobalUnlock (hg);
+	break;
+      }
+    case CF_TEXT:
+      {
+	char *text = (char *) GlobalLock (hg);
+	result = DECODE_SYSTEM (build_unibyte_string (text));
+	GlobalUnlock (hg);
+	break;
+      }
+    }
+
+  GlobalFree (hg);
+
+  return result;
+}
+
+struct w32_drop_target {
+  /* i_drop_target must be the first member.  */
+  IDropTarget i_drop_target;
+  HWND hwnd;
+};
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_QueryInterface (IDropTarget *t, REFIID ri, void **r)
+{
+  return E_NOINTERFACE;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_AddRef (IDropTarget *This)
+{
+  return 1;
+}
+
+static ULONG STDMETHODCALLTYPE
+w32_drop_target_Release (IDropTarget *This)
+{
+  struct w32_drop_target *target = (struct w32_drop_target * ) This;
+  free (target->i_drop_target.lpVtbl);
+  free (target);
+  return 0;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
+			   DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  /* Possible 'effect' values are COPY, MOVE, LINK or NONE.  This choice
+     changes the mouse pointer shape to inform the user of what will
+     happen on drop.  We send COPY because our use cases don't modify
+     or link to the original data.  */
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragOver (IDropTarget *This, DWORD grfKeyState, POINTL pt,
+			  DWORD *pdwEffect)
+{
+  /* See comment in w32_drop_target_DragEnter.  */
+  *pdwEffect = DROPEFFECT_COPY;
+  return S_OK;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_DragLeave (IDropTarget *This)
+{
+  return S_OK;
+}
+
+static HGLOBAL w32_try_get_data (IDataObject *pDataObj, int format)
+{
+  FORMATETC formatetc = { format, NULL, DVASPECT_CONTENT, -1,
+			  TYMED_HGLOBAL };
+  STGMEDIUM stgmedium;
+  HRESULT r = IDataObject_GetData (pDataObj, &formatetc, &stgmedium);
+  if (SUCCEEDED (r))
+    {
+      if (stgmedium.tymed == TYMED_HGLOBAL)
+	return stgmedium.hGlobal;
+      ReleaseStgMedium (&stgmedium);
+    }
+  return NULL;
+}
+
+static HRESULT STDMETHODCALLTYPE
+w32_drop_target_Drop (IDropTarget *This, IDataObject *pDataObj,
+		      DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+  *pdwEffect = DROPEFFECT_COPY;
+
+  W32Msg msg = {0};
+  msg.dwModifiers = w32_get_modifiers();
+  msg.msg.time = GetMessageTime ();
+  msg.msg.pt.x = pt.x;
+  msg.msg.pt.y = pt.y;
+
+  int format = CF_HDROP;
+  HGLOBAL hGlobal = w32_try_get_data (pDataObj, format);
+
+  if (!hGlobal)
+    {
+      format = CF_UNICODETEXT;
+      hGlobal = w32_try_get_data (pDataObj, format);
+    }
+
+  if (!hGlobal)
+    {
+      format = CF_TEXT;
+      hGlobal = w32_try_get_data (pDataObj, format);
+    }
+
+  if (hGlobal)
+      my_post_msg (&msg, target->hwnd, WM_EMACS_DROP, format,
+		   (LPARAM) hGlobal);
+
+  return S_OK;
+}
+
 static void
 w32_createwindow (struct frame *f, int *coords)
 {
@@ -2548,7 +2733,30 @@ w32_createwindow (struct frame *f, int *coords)
       SetWindowLong (hwnd, WND_BACKGROUND_INDEX, FRAME_BACKGROUND_PIXEL (f));
 
       /* Enable drag-n-drop.  */
-      DragAcceptFiles (hwnd, TRUE);
+      struct w32_drop_target *drop_target =
+	malloc (sizeof (struct w32_drop_target));
+
+      if (drop_target != NULL)
+	{
+	  IDropTargetVtbl *vtbl = malloc (sizeof (IDropTargetVtbl));
+	  if (vtbl != NULL)
+	    {
+	      drop_target->hwnd = hwnd;
+	      drop_target->i_drop_target.lpVtbl = vtbl;
+	      vtbl->QueryInterface = w32_drop_target_QueryInterface;
+	      vtbl->AddRef = w32_drop_target_AddRef;
+	      vtbl->Release = w32_drop_target_Release;
+	      vtbl->DragEnter = w32_drop_target_DragEnter;
+	      vtbl->DragOver = w32_drop_target_DragOver;
+	      vtbl->DragLeave = w32_drop_target_DragLeave;
+	      vtbl->Drop = w32_drop_target_Drop;
+	      RegisterDragDrop (hwnd, &drop_target->i_drop_target);
+	    }
+	  else
+	    {
+	      free (drop_target);
+	    }
+	}
 
       /* Enable system light/dark theme.  */
       w32_applytheme (hwnd);
@@ -3399,6 +3607,7 @@ #define M(msg) { msg, # msg }
       M (WM_EMACS_PAINT),
       M (WM_EMACS_IME_STATUS),
       M (WM_CHAR),
+      M (WM_EMACS_DROP),
 #undef M
       { 0, 0 }
   };
@@ -3465,13 +3674,14 @@ w32_msg_pump (deferred_msg * msg_buf)
 	      /* Produced by complete_deferred_msg; just ignore.  */
 	      break;
 	    case WM_EMACS_CREATEWINDOW:
-	      /* Initialize COM for this window. Even though we don't use it,
-		 some third party shell extensions can cause it to be used in
+	      /* Initialize COM for this window.  Needed for RegisterDragDrop.
+		 Some third party shell extensions can cause it to be used in
 		 system dialogs, which causes a crash if it is not initialized.
 		 This is a known bug in Windows, which was fixed long ago, but
 		 the patch for XP is not publicly available until XP SP3,
 		 and older versions will never be patched.  */
-	      CoInitialize (NULL);
+	      OleInitialize (NULL);
+
 	      w32_createwindow ((struct frame *) msg.wParam,
 				(int *) msg.lParam);
 	      if (!PostThreadMessage (dwMainThreadId, WM_EMACS_DONE, 0, 0))
@@ -5106,7 +5316,6 @@ #define MOUSEEVENTF_FROMTOUCH 0xFF515700
       return 0;
 
     case WM_MOUSEWHEEL:
-    case WM_DROPFILES:
       wmsg.dwModifiers = w32_get_modifiers ();
       my_post_msg (&wmsg, hwnd, msg, wParam, lParam);
       signal_user_input ();
@@ -5597,7 +5806,7 @@ #define WM_TOUCH 576
       }
 
     case WM_EMACS_DESTROYWINDOW:
-      DragAcceptFiles ((HWND) wParam, FALSE);
+      RevokeDragDrop ((HWND) wParam);
       return DestroyWindow ((HWND) wParam);
 
     case WM_EMACS_HIDE_CARET:
diff --git a/src/w32term.c b/src/w32term.c
index 62037e3b2cd..3a627308137 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -3576,81 +3576,6 @@ w32_construct_mouse_wheel (struct input_event *result, W32Msg *msg,
   return Qnil;
 }
 
-static Lisp_Object
-w32_construct_drag_n_drop (struct input_event *result, W32Msg *msg,
-                           struct frame *f)
-{
-  Lisp_Object files;
-  Lisp_Object frame;
-  HDROP hdrop;
-  POINT p;
-  WORD num_files;
-  wchar_t name_w[MAX_PATH];
-#ifdef NTGUI_UNICODE
-  const int use_unicode = 1;
-#else
-  int use_unicode = w32_unicode_filenames;
-  char name_a[MAX_PATH];
-  char file[MAX_UTF8_PATH];
-#endif
-  int i;
-
-  result->kind = DRAG_N_DROP_EVENT;
-  result->code = 0;
-  result->timestamp = msg->msg.time;
-  result->modifiers = msg->dwModifiers;
-
-  hdrop = (HDROP) msg->msg.wParam;
-  DragQueryPoint (hdrop, &p);
-
-#if 0
-  p.x = LOWORD (msg->msg.lParam);
-  p.y = HIWORD (msg->msg.lParam);
-  ScreenToClient (msg->msg.hwnd, &p);
-#endif
-
-  XSETINT (result->x, p.x);
-  XSETINT (result->y, p.y);
-
-  num_files = DragQueryFile (hdrop, 0xFFFFFFFF, NULL, 0);
-  files = Qnil;
-
-  for (i = 0; i < num_files; i++)
-    {
-      if (use_unicode)
-	{
-	  eassert (DragQueryFileW (hdrop, i, NULL, 0) < MAX_PATH);
-	  /* If DragQueryFile returns zero, it failed to fetch a file
-	     name.  */
-	  if (DragQueryFileW (hdrop, i, name_w, MAX_PATH) == 0)
-	    continue;
-#ifdef NTGUI_UNICODE
-	  files = Fcons (from_unicode_buffer (name_w), files);
-#else
-	  filename_from_utf16 (name_w, file);
-	  files = Fcons (DECODE_FILE (build_unibyte_string (file)), files);
-#endif /* NTGUI_UNICODE */
-	}
-#ifndef NTGUI_UNICODE
-      else
-	{
-	  eassert (DragQueryFileA (hdrop, i, NULL, 0) < MAX_PATH);
-	  if (DragQueryFileA (hdrop, i, name_a, MAX_PATH) == 0)
-	    continue;
-	  filename_from_ansi (name_a, file);
-	  files = Fcons (DECODE_FILE (build_unibyte_string (file)), files);
-	}
-#endif
-    }
-
-  DragFinish (hdrop);
-
-  XSETFRAME (frame, f);
-  result->frame_or_window = frame;
-  result->arg = files;
-  return Qnil;
-}
-
 \f
 #if HAVE_W32NOTIFY
 
@@ -5682,11 +5607,26 @@ w32_read_socket (struct terminal *terminal,
 	  }
 	  break;
 
-	case WM_DROPFILES:
-	  f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	case WM_EMACS_DROP:
+	  {
+	    int format = msg.msg.wParam;
+	    Lisp_Object drop_object =
+	      w32_process_dnd_data (format, (void *) msg.msg.lParam);
 
-	  if (f)
-	    w32_construct_drag_n_drop (&inev, &msg, f);
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f || NILP (drop_object))
+	      break;
+
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+	    inev.arg = drop_object;
+	  }
 	  break;
 
 	case WM_HSCROLL:
diff --git a/src/w32term.h b/src/w32term.h
index 47be542f570..39e2262e2a8 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -272,6 +272,7 @@ #define CP_DEFAULT 1004
 
 /* w32fns.c */
 extern void w32_default_font_parameter (struct frame* f, Lisp_Object parms);
+extern Lisp_Object w32_process_dnd_data (int format, void *pDataObj);
 
 \f
 #define PIX_TYPE COLORREF
@@ -710,7 +711,8 @@ #define WM_EMACS_BRINGTOTOP            (WM_EMACS_START + 23)
 #define WM_EMACS_INPUT_READY           (WM_EMACS_START + 24)
 #define WM_EMACS_FILENOTIFY            (WM_EMACS_START + 25)
 #define WM_EMACS_IME_STATUS            (WM_EMACS_START + 26)
-#define WM_EMACS_END                   (WM_EMACS_START + 27)
+#define WM_EMACS_DROP                  (WM_EMACS_START + 27)
+#define WM_EMACS_END                   (WM_EMACS_START + 28)
 
 #define WND_FONTWIDTH_INDEX    (0)
 #define WND_LINEHEIGHT_INDEX   (4)
-- 
2.35.1.windows.2

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Sat, 5 Oct 2024 23:41:55 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> Looking at the manual, in the "Drag and Drop" chapter it says:
> 
>  >    It can be difficult to scroll a window or determine where dropped
>  > text will be inserted while dragging text onto an Emacs window.
>  > Setting the option ‘dnd-indicate-insertion-point’ to a non-‘nil’ value
>  > makes point move to the location any dropped text will be inserted
>  > when the mouse moves in a window during drag, and setting
>  > ‘dnd-scroll-margin’ to an integer value causes a window to be scrolled
>  > if the mouse moves within that many lines of the top or bottom of the
>  > window during drag.
> 
> This should be implemented on Windows too, but it will take me some 
> time. Maybe you prefer to wait before installing the changes.

Up to you.  From where I stand, we can install what you have now, and
add support for the above as a separate changeset.

> Also, do you think the manual should be updated too?

If the manual becomes inaccurate or incomplete with your changes, then
yes, of course.

bug#3468: drag and drop text

[Cecilio Pardo]

On 06/10/2024 8:10, Eli Zaretskii wrote:
>> This should be implemented on Windows too, but it will take me some
>> time. Maybe you prefer to wait before installing the changes.
> 
> Up to you.  From where I stand, we can install what you have now, and
> add support for the above as a separate changeset.

Let's do this then.


>> Also, do you think the manual should be updated too?
> 
> If the manual becomes inaccurate or incomplete with your changes, then
> yes, of course.

I was thinking about changing this:

    The XDND and Motif drag and drop protocols, and the old KDE 1.x
protocol, are currently supported.


To this:

    The XDND and Motif drag and drop protocols, and the old KDE 1.x
protocol, are currently supported. There is also support on MS-Windows.


And adding later:

The 'dnd-indicate-insertion-point’ and ‘dnd-scroll-margin’ variables are 
not supported on MS-Windows.

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Mon, 7 Oct 2024 12:28:20 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 06/10/2024 8:10, Eli Zaretskii wrote:
> >> This should be implemented on Windows too, but it will take me some
> >> time. Maybe you prefer to wait before installing the changes.
> > 
> > Up to you.  From where I stand, we can install what you have now, and
> > add support for the above as a separate changeset.
> 
> Let's do this then.

Will install when I have time.

> >> Also, do you think the manual should be updated too?
> > 
> > If the manual becomes inaccurate or incomplete with your changes, then
> > yes, of course.
> 
> I was thinking about changing this:
> 
>     The XDND and Motif drag and drop protocols, and the old KDE 1.x
> protocol, are currently supported.
> 
> 
> To this:
> 
>     The XDND and Motif drag and drop protocols, and the old KDE 1.x
> protocol, are currently supported. There is also support on MS-Windows.

OK.

> And adding later:
> 
> The 'dnd-indicate-insertion-point’ and ‘dnd-scroll-margin’ variables are 
> not supported on MS-Windows.

If you intend to work on supporting this, there's no need to add this
now.

bug#3468: drag and drop text

[Eli Zaretskii]

> Cc: 3468@debbugs.gnu.org
> Date: Mon, 07 Oct 2024 15:00:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > Date: Mon, 7 Oct 2024 12:28:20 +0200
> > Cc: 3468@debbugs.gnu.org
> > From: Cecilio Pardo <cpardo@imayhem.com>
> > 
> > On 06/10/2024 8:10, Eli Zaretskii wrote:
> > >> This should be implemented on Windows too, but it will take me some
> > >> time. Maybe you prefer to wait before installing the changes.
> > > 
> > > Up to you.  From where I stand, we can install what you have now, and
> > > add support for the above as a separate changeset.
> > 
> > Let's do this then.
> 
> Will install when I have time.

Done.

I'm not closing the bug, because you said you want to add support for
the dnd-* variables.

bug#3468: drag and drop text

[Cecilio Pardo]

> I'm not closing the bug, because you said you want to add support for
> the dnd-* variables.

Thanks, I will.

bug#3468: drag and drop text

[Cecilio Pardo]

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

On 08/10/2024 15:15, Eli Zaretskii wrote:

> I'm not closing the bug, because you said you want to add support for
> the dnd-* variables.

This add support for dnd-scroll-margin and dnd-indicate-insertion-point 
on MS-Windows.

BTW, I think this variables should be enabled by default. I'd say that 
is the expected behaviour for drag and drop.

[-- Attachment #2: 0001-Improve-drag-and-drop-on-MS-Windows-bug-3468.patch --]
[-- Type: text/plain, Size: 6973 bytes --]

From 5c0f2a1b40931824df75a620d2494f2031bf0f83 Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Wed, 23 Oct 2024 14:41:24 +0200
Subject: [PATCH] Improve drag and drop on MS-Windows (bug#3468)

Add support for dnd-scroll-margin and dnd-indicate-insertion-point by
calling dnd-handle-movement when dragging the mouse.

* lisp/term/w32-win.el (w32-drag-n-drop): Call dnd-handle-movement
when applicable.
* src/w32fns.c (w32_handle_drag_movement): New function, sends a
WM_EMACS_DRAGOVER message.
(w32_drop_target_DragEnter): Call w32_handle_drag_movement.
(w32_drop_target_DragOver): Call w32_handle_drag_movement.
* src/w32term.c: (w32_read_socket): Handle WM_EMACS_DRAGOVER,
sending an drag-n-drop event.
* src/w32term.h (): Define new WM_EMACS_DRAGOVER message.
---
 lisp/term/w32-win.el | 54 ++++++++++++++++++++++++--------------------
 src/w32fns.c         | 17 +++++++++++++-
 src/w32term.c        | 18 +++++++++++++++
 src/w32term.h        |  5 ++--
 4 files changed, 66 insertions(+), 28 deletions(-)

diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 541fef2ced3..b47b5f21f05 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -137,35 +137,39 @@ w32-drag-n-drop
 If EVENT is for text, insert that text at point into the buffer
 shown in the window that is the target of the drop; if that buffer is
 read-only, add the dropped text to kill-ring.
+If EVENT payload is nil, then this is a drag event.
 If the optional argument NEW-FRAME is non-nil, perform the
 drag-n-drop action in a newly-created frame using its selected-window
 and that window's buffer."
   (interactive "e")
-  (save-excursion
-    ;; Make sure the drop target has positive co-ords
-    ;; before setting the selected frame - otherwise it
-    ;; won't work.  <skx@tardis.ed.ac.uk>
-    (let* ((window (posn-window (event-start event)))
-	   (coords (posn-x-y (event-start event)))
-           (arg (car (cdr (cdr event))))
-	   (x (car coords))
-	   (y (cdr coords)))
-      (if (and (> x 0) (> y 0))
-	  (set-frame-selected-window nil window))
-
-      (when new-frame
-        (select-frame (make-frame)))
-      (raise-frame)
-      (setq window (selected-window))
-
-      ;; arg (the payload of the event) is a string when the drop is
-      ;; text, and a list of strings when the drop is one or more files.
-      (if (stringp arg)
-          (dnd-insert-text window 'copy arg)
-        (dnd-handle-multiple-urls
-         window
-         (mapcar #'w32-dropped-file-to-url arg)
-         'private)))))
+  ;; Make sure the drop target has positive co-ords
+  ;; before setting the selected frame - otherwise it
+  ;; won't work.  <skx@tardis.ed.ac.uk>
+  (let* ((window (posn-window (event-start event)))
+	 (coords (posn-x-y (event-start event)))
+         (arg (car (cdr (cdr event))))
+	 (x (car coords))
+	 (y (cdr coords)))
+
+    (if (and (> x 0) (> y 0) (window-live-p window))
+        (set-frame-selected-window nil window))
+    ;; Don't create new frame if we are just dragging
+    (and arg new-frame
+         (select-frame (make-frame)))
+    (raise-frame)
+    (setq window (selected-window))
+
+    ;; arg (the payload of the event) is a string when the drop is
+    ;; text, and a list of strings when the drop is one or more files.
+    ;; It is nil if the event is a drag event.
+    (if arg
+        (if (stringp arg)
+            (dnd-insert-text window 'copy arg)
+          (dnd-handle-multiple-urls
+           window
+           (mapcar #'w32-dropped-file-to-url arg)
+           'private))
+      (dnd-handle-movement (event-start event)))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
diff --git a/src/w32fns.c b/src/w32fns.c
index 3ee13dcbbdd..eb42d3b61b2 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -2505,7 +2505,6 @@ process_dropfiles (DROPFILES *files)
   return lisp_files;
 }
 
-
 /* This function can be called ONLY between calls to
    block_input/unblock_input.  It is used in w32_read_socket.  */
 Lisp_Object
@@ -2572,6 +2571,19 @@ w32_drop_target_Release (IDropTarget *This)
   return 0;
 }
 
+static void
+w32_handle_drag_movement (IDropTarget *This, POINTL pt)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+
+  W32Msg msg = {0};
+  msg.dwModifiers = w32_get_modifiers ();
+  msg.msg.time = GetMessageTime ();
+  msg.msg.pt.x = pt.x;
+  msg.msg.pt.y = pt.y;
+  my_post_msg (&msg, target->hwnd, WM_EMACS_DRAGOVER, 0, 0 );
+}
+
 static HRESULT STDMETHODCALLTYPE
 w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
 			   DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
@@ -2581,6 +2593,7 @@ w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
      happen on drop.  We send COPY because our use cases don't modify
      or link to the original data.  */
   *pdwEffect = DROPEFFECT_COPY;
+  w32_handle_drag_movement (This, pt);
   return S_OK;
 }
 
@@ -2590,6 +2603,7 @@ w32_drop_target_DragOver (IDropTarget *This, DWORD grfKeyState, POINTL pt,
 {
   /* See comment in w32_drop_target_DragEnter.  */
   *pdwEffect = DROPEFFECT_COPY;
+  w32_handle_drag_movement (This, pt);
   return S_OK;
 }
 
@@ -3607,6 +3621,7 @@ #define M(msg) { msg, # msg }
       M (WM_EMACS_PAINT),
       M (WM_EMACS_IME_STATUS),
       M (WM_CHAR),
+      M (WM_EMACS_DRAGOVER),
       M (WM_EMACS_DROP),
 #undef M
       { 0, 0 }
diff --git a/src/w32term.c b/src/w32term.c
index 3a627308137..88622700386 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -5629,6 +5629,24 @@ w32_read_socket (struct terminal *terminal,
 	  }
 	  break;
 
+	case WM_EMACS_DRAGOVER:
+	  {
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f)
+	      break;
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+	    /* This is a drag movement.  */
+	    inev.arg = Qnil;
+	    break;
+	  }
+
 	case WM_HSCROLL:
 	  {
 	    struct scroll_bar *bar =
diff --git a/src/w32term.h b/src/w32term.h
index 39e2262e2a8..cad9fcf8cb1 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -711,8 +711,9 @@ #define WM_EMACS_BRINGTOTOP            (WM_EMACS_START + 23)
 #define WM_EMACS_INPUT_READY           (WM_EMACS_START + 24)
 #define WM_EMACS_FILENOTIFY            (WM_EMACS_START + 25)
 #define WM_EMACS_IME_STATUS            (WM_EMACS_START + 26)
-#define WM_EMACS_DROP                  (WM_EMACS_START + 27)
-#define WM_EMACS_END                   (WM_EMACS_START + 28)
+#define WM_EMACS_DRAGOVER              (WM_EMACS_START + 27)
+#define WM_EMACS_DROP                  (WM_EMACS_START + 28)
+#define WM_EMACS_END                   (WM_EMACS_START + 29)
 
 #define WND_FONTWIDTH_INDEX    (0)
 #define WND_LINEHEIGHT_INDEX   (4)
-- 
2.35.1.windows.2

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Wed, 23 Oct 2024 19:16:51 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> This add support for dnd-scroll-margin and dnd-indicate-insertion-point 
> on MS-Windows.

Thanks.

I guess I'm missing some higher-level understanding of how this work,
because I don't think I understand why you needed to add a special
message WM_EMACS_DRAGOVER for this.  Can you explain?

> BTW, I think this variables should be enabled by default. I'd say that 
> is the expected behaviour for drag and drop.

Maybe.  But that is a separate issue.

> * lisp/term/w32-win.el (w32-drag-n-drop): Call dnd-handle-movement
> when applicable.
> * src/w32fns.c (w32_handle_drag_movement): New function, sends a
> WM_EMACS_DRAGOVER message.
> (w32_drop_target_DragEnter): Call w32_handle_drag_movement.
> (w32_drop_target_DragOver): Call w32_handle_drag_movement.
> * src/w32term.c: (w32_read_socket): Handle WM_EMACS_DRAGOVER,
> sending an drag-n-drop event.

Please quote symbols 'like this'.

> * src/w32term.h (): Define new WM_EMACS_DRAGOVER message.
                 ^^^
Please remove these parentheses.

> --- a/lisp/term/w32-win.el
> +++ b/lisp/term/w32-win.el
> @@ -137,35 +137,39 @@ w32-drag-n-drop
>  If EVENT is for text, insert that text at point into the buffer
>  shown in the window that is the target of the drop; if that buffer is
>  read-only, add the dropped text to kill-ring.
> +If EVENT payload is nil, then this is a drag event.
>  If the optional argument NEW-FRAME is non-nil, perform the
>  drag-n-drop action in a newly-created frame using its selected-window
>  and that window's buffer."
>    (interactive "e")
> -  (save-excursion

What are the reasons for removing save-excursion?

I guess this also needs a NEWS entry, to announce support for these
two variables on MS-Windows?

bug#3468: drag and drop text

[Cecilio Pardo]

On 24/10/2024 9:37, Eli Zaretskii wrote:
> I guess I'm missing some higher-level understanding of how this work,
> because I don't think I understand why you needed to add a special
> message WM_EMACS_DRAGOVER for this.  Can you explain?

I could reuse WM_EMACS_DROP, maybe rename to WM_EMACS_DRAGDROP. Added a 
new one just for clarity.

> What are the reasons for removing save-excursion?

dnd-handle-movement needs to move the point to apply
dnd-indicate-insertion-point-

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Thu, 24 Oct 2024 09:56:16 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 24/10/2024 9:37, Eli Zaretskii wrote:
> > I guess I'm missing some higher-level understanding of how this work,
> > because I don't think I understand why you needed to add a special
> > message WM_EMACS_DRAGOVER for this.  Can you explain?
> 
> I could reuse WM_EMACS_DROP, maybe rename to WM_EMACS_DRAGDROP. Added a 
> new one just for clarity.

I don't mind a new message, I just didn't understand why it was
needed.  What is the purpose of WM_EMACS_DROP, and how it differs from
this new message?

> > What are the reasons for removing save-excursion?
> 
> dnd-handle-movement needs to move the point to apply
> dnd-indicate-insertion-point-

Then maybe use save-excursion when dnd-handle-movement is NOT called?

bug#3468: drag and drop text

[Cecilio Pardo]

On 24/10/2024 10:16, Eli Zaretskii wrote:
>> On 24/10/2024 9:37, Eli Zaretskii wrote:
>>> I guess I'm missing some higher-level understanding of how this work,
>>> because I don't think I understand why you needed to add a special
>>> message WM_EMACS_DRAGOVER for this.  Can you explain?
>>
>> I could reuse WM_EMACS_DROP, maybe rename to WM_EMACS_DRAGDROP. Added a
>> new one just for clarity.
> 
> I don't mind a new message, I just didn't understand why it was
> needed.  What is the purpose of WM_EMACS_DROP, and how it differs from
> this new message?

We receive the drag or drop information in the functions
w32_drop_target_Drop and w32_drop_target_DragOver, which run on another 
thread. We need to notify that to the input thread, sending a message to 
the window. We use those to custom messages, WM_EMACS_DROP and 
WM_EMACS_DRAGDROP. One for the drop, the other for when the mouse moves 
over the window.
> 
>>> What are the reasons for removing save-excursion?
>>
>> dnd-handle-movement needs to move the point to apply
>> dnd-indicate-insertion-point-
> 
> Then maybe use save-excursion when dnd-handle-movement is NOT called?

Ok, I'll do that.

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Thu, 24 Oct 2024 10:46:12 +0200
> Cc: 3468@debbugs.gnu.org
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 24/10/2024 10:16, Eli Zaretskii wrote:
> >> On 24/10/2024 9:37, Eli Zaretskii wrote:
> >>> I guess I'm missing some higher-level understanding of how this work,
> >>> because I don't think I understand why you needed to add a special
> >>> message WM_EMACS_DRAGOVER for this.  Can you explain?
> >>
> >> I could reuse WM_EMACS_DROP, maybe rename to WM_EMACS_DRAGDROP. Added a
> >> new one just for clarity.
> > 
> > I don't mind a new message, I just didn't understand why it was
> > needed.  What is the purpose of WM_EMACS_DROP, and how it differs from
> > this new message?
> 
> We receive the drag or drop information in the functions
> w32_drop_target_Drop and w32_drop_target_DragOver, which run on another 
> thread. We need to notify that to the input thread, sending a message to 
> the window. We use those to custom messages, WM_EMACS_DROP and 
> WM_EMACS_DRAGDROP. One for the drop, the other for when the mouse moves 
> over the window.

Ah, okay, thanks.  So moving point and/or scrolling use different
message from actually dropping the payload?  Makes sense.

bug#3468: drag and drop text

[Cecilio Pardo]

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

On 24/10/2024 9:37, Eli Zaretskii wrote:

> What are the reasons for removing save-excursion?
> 
> I guess this also needs a NEWS entry, to announce support for these
> two variables on MS-Windows?

All fixed now.

Thanks for your time.



[-- Attachment #2: 0001-Improve-drag-and-drop-on-MS-Windows-bug-3468.patch --]
[-- Type: text/plain, Size: 7619 bytes --]

From 9a2c2002aca81ad718db4b165aa5012dd2120e4a Mon Sep 17 00:00:00 2001
From: Cecilio Pardo <cpardo@imayhem.com>
Date: Wed, 23 Oct 2024 14:41:24 +0200
Subject: [PATCH] Improve drag and drop on MS-Windows (bug#3468)

Add support for 'dnd-scroll-margin' and 'dnd-indicate-insertion-point'
by calling 'dnd-handle-movement' when dragging the mouse.

* lisp/term/w32-win.el (w32-drag-n-drop): Call 'dnd-handle-movement'
when applicable.
* src/w32fns.c (w32_handle_drag_movement): New function, sends a
WM_EMACS_DRAGOVER message.
(w32_drop_target_DragEnter): Call 'w32_handle_drag_movement'.
(w32_drop_target_DragOver): Call 'w32_handle_drag_movement'.
* src/w32term.c: (w32_read_socket): Handle WM_EMACS_DRAGOVER,
sending a drag-n-drop event.
* src/w32term.h: Define new WM_EMACS_DRAGOVER message.
---
 etc/NEWS             |  4 +++-
 lisp/term/w32-win.el | 55 ++++++++++++++++++++++++--------------------
 src/w32fns.c         | 17 +++++++++++++-
 src/w32term.c        | 18 +++++++++++++++
 src/w32term.h        |  5 ++--
 5 files changed, 70 insertions(+), 29 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index f7ad41b1b9f..ffa128cc2fb 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -762,7 +762,9 @@ and later versions.
 
 ---
 ** Emacs on MS-Windows now supports drag-n-drop of text into a buffer.
-This is in addition to drag-n-drop of files, that was already supported.
+This is in addition to drag-n-drop of files, that was already
+supported. The configuration variables 'dnd-scroll-margin' and
+'dnd-indicate-insertion-point' can be used to customize the process.
 
 \f
 ----------------------------------------------------------------------
diff --git a/lisp/term/w32-win.el b/lisp/term/w32-win.el
index 541fef2ced3..75f8530010c 100644
--- a/lisp/term/w32-win.el
+++ b/lisp/term/w32-win.el
@@ -137,35 +137,40 @@ w32-drag-n-drop
 If EVENT is for text, insert that text at point into the buffer
 shown in the window that is the target of the drop; if that buffer is
 read-only, add the dropped text to kill-ring.
+If EVENT payload is nil, then this is a drag event.
 If the optional argument NEW-FRAME is non-nil, perform the
 drag-n-drop action in a newly-created frame using its selected-window
 and that window's buffer."
   (interactive "e")
-  (save-excursion
-    ;; Make sure the drop target has positive co-ords
-    ;; before setting the selected frame - otherwise it
-    ;; won't work.  <skx@tardis.ed.ac.uk>
-    (let* ((window (posn-window (event-start event)))
-	   (coords (posn-x-y (event-start event)))
-           (arg (car (cdr (cdr event))))
-	   (x (car coords))
-	   (y (cdr coords)))
-      (if (and (> x 0) (> y 0))
-	  (set-frame-selected-window nil window))
-
-      (when new-frame
-        (select-frame (make-frame)))
-      (raise-frame)
-      (setq window (selected-window))
-
-      ;; arg (the payload of the event) is a string when the drop is
-      ;; text, and a list of strings when the drop is one or more files.
-      (if (stringp arg)
-          (dnd-insert-text window 'copy arg)
-        (dnd-handle-multiple-urls
-         window
-         (mapcar #'w32-dropped-file-to-url arg)
-         'private)))))
+  ;; Make sure the drop target has positive co-ords
+  ;; before setting the selected frame - otherwise it
+  ;; won't work.  <skx@tardis.ed.ac.uk>
+  (let* ((window (posn-window (event-start event)))
+	 (coords (posn-x-y (event-start event)))
+         (arg (car (cdr (cdr event))))
+	 (x (car coords))
+	 (y (cdr coords)))
+
+    (if (and (> x 0) (> y 0) (window-live-p window))
+        (set-frame-selected-window nil window))
+    ;; Don't create new frame if we are just dragging
+    (and arg new-frame
+         (select-frame (make-frame)))
+    (raise-frame)
+    (setq window (selected-window))
+
+    ;; arg (the payload of the event) is a string when the drop is
+    ;; text, and a list of strings when the drop is one or more files.
+    ;; It is nil if the event is a drag event.
+    (if arg
+        (save-excursion
+          (if (stringp arg)
+              (dnd-insert-text window 'copy arg)
+            (dnd-handle-multiple-urls
+             window
+             (mapcar #'w32-dropped-file-to-url arg)
+             'private)))
+      (dnd-handle-movement (event-start event)))))
 
 (defun w32-drag-n-drop-other-frame (event)
   "Edit the files listed in the drag-n-drop EVENT, in other frames.
diff --git a/src/w32fns.c b/src/w32fns.c
index 3ee13dcbbdd..eb42d3b61b2 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -2505,7 +2505,6 @@ process_dropfiles (DROPFILES *files)
   return lisp_files;
 }
 
-
 /* This function can be called ONLY between calls to
    block_input/unblock_input.  It is used in w32_read_socket.  */
 Lisp_Object
@@ -2572,6 +2571,19 @@ w32_drop_target_Release (IDropTarget *This)
   return 0;
 }
 
+static void
+w32_handle_drag_movement (IDropTarget *This, POINTL pt)
+{
+  struct w32_drop_target *target = (struct w32_drop_target *)This;
+
+  W32Msg msg = {0};
+  msg.dwModifiers = w32_get_modifiers ();
+  msg.msg.time = GetMessageTime ();
+  msg.msg.pt.x = pt.x;
+  msg.msg.pt.y = pt.y;
+  my_post_msg (&msg, target->hwnd, WM_EMACS_DRAGOVER, 0, 0 );
+}
+
 static HRESULT STDMETHODCALLTYPE
 w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
 			   DWORD grfKeyState, POINTL pt, DWORD *pdwEffect)
@@ -2581,6 +2593,7 @@ w32_drop_target_DragEnter (IDropTarget *This, IDataObject *pDataObj,
      happen on drop.  We send COPY because our use cases don't modify
      or link to the original data.  */
   *pdwEffect = DROPEFFECT_COPY;
+  w32_handle_drag_movement (This, pt);
   return S_OK;
 }
 
@@ -2590,6 +2603,7 @@ w32_drop_target_DragOver (IDropTarget *This, DWORD grfKeyState, POINTL pt,
 {
   /* See comment in w32_drop_target_DragEnter.  */
   *pdwEffect = DROPEFFECT_COPY;
+  w32_handle_drag_movement (This, pt);
   return S_OK;
 }
 
@@ -3607,6 +3621,7 @@ #define M(msg) { msg, # msg }
       M (WM_EMACS_PAINT),
       M (WM_EMACS_IME_STATUS),
       M (WM_CHAR),
+      M (WM_EMACS_DRAGOVER),
       M (WM_EMACS_DROP),
 #undef M
       { 0, 0 }
diff --git a/src/w32term.c b/src/w32term.c
index 3a627308137..88622700386 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -5629,6 +5629,24 @@ w32_read_socket (struct terminal *terminal,
 	  }
 	  break;
 
+	case WM_EMACS_DRAGOVER:
+	  {
+	    f = w32_window_to_frame (dpyinfo, msg.msg.hwnd);
+	    if (!f)
+	      break;
+	    XSETFRAME (inev.frame_or_window, f);
+	    inev.kind = DRAG_N_DROP_EVENT;
+	    inev.code = 0;
+	    inev.timestamp = msg.msg.time;
+	    inev.modifiers = msg.dwModifiers;
+	    ScreenToClient (msg.msg.hwnd, &msg.msg.pt);
+	    XSETINT (inev.x, msg.msg.pt.x);
+	    XSETINT (inev.y, msg.msg.pt.y);
+	    /* This is a drag movement.  */
+	    inev.arg = Qnil;
+	    break;
+	  }
+
 	case WM_HSCROLL:
 	  {
 	    struct scroll_bar *bar =
diff --git a/src/w32term.h b/src/w32term.h
index 39e2262e2a8..cad9fcf8cb1 100644
--- a/src/w32term.h
+++ b/src/w32term.h
@@ -711,8 +711,9 @@ #define WM_EMACS_BRINGTOTOP            (WM_EMACS_START + 23)
 #define WM_EMACS_INPUT_READY           (WM_EMACS_START + 24)
 #define WM_EMACS_FILENOTIFY            (WM_EMACS_START + 25)
 #define WM_EMACS_IME_STATUS            (WM_EMACS_START + 26)
-#define WM_EMACS_DROP                  (WM_EMACS_START + 27)
-#define WM_EMACS_END                   (WM_EMACS_START + 28)
+#define WM_EMACS_DRAGOVER              (WM_EMACS_START + 27)
+#define WM_EMACS_DROP                  (WM_EMACS_START + 28)
+#define WM_EMACS_END                   (WM_EMACS_START + 29)
 
 #define WND_FONTWIDTH_INDEX    (0)
 #define WND_LINEHEIGHT_INDEX   (4)
-- 
2.35.1.windows.2

bug#3468: drag and drop text

[Eli Zaretskii]

> Date: Thu, 24 Oct 2024 18:52:41 +0200
> From: Cecilio Pardo <cpardo@imayhem.com>
> 
> On 24/10/2024 9:37, Eli Zaretskii wrote:
> 
> > What are the reasons for removing save-excursion?
> > 
> > I guess this also needs a NEWS entry, to announce support for these
> > two variables on MS-Windows?
> 
> All fixed now.

Thanks, installed on the master branch, and closing the bug.