Subject: bug#3468: drag and drop text

From: Eli Zaretskii <[email-masked]>
Date: Sat, 05 Oct 2024 14:07:52 +0300

> Date: Mon, 30 Sep 2024 23:20:32 +0200
> From: Cecilio Pardo <[email-masked]>
> Cc: [email-masked]
> 
> 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?