bug#30929: 26.0.91; Text drag and drop does not work

[Alan Third <alan@idiocy.org>]

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

On Sun, Jan 06, 2019 at 02:05:57AM +1300, Nick Helm wrote:
> On Sat, 2019-01-05 at 10:27 +0000, Alan Third wrote:
> > Hi Nick, did you make any progress on the drag and drop stuff?
> 
> Hi Alan. Sorry, no I never managed to get it working they way I wanted.

No problem. Can you have a look at the attached. It’s not as ambitious
as we discussed, but it does pretty much the minimum required to make
drag and drop usable.

If you think the defaults I’ve chosen don’t make sense, I’m happy to
change them. I don’t ever use drag and drop, so I don’t know what’s
really useful for people.
-- 
Alan Third

[-- Attachment #2: 0001-Fix-drag-and-drop-behaviour-on-NS.patch --]
[-- Type: text/plain, Size: 10646 bytes --]

From 1d4cbb250bb0f8fd75a3c749b454c9b177c4dae7 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 5 Jan 2019 16:11:37 +0000
Subject: [PATCH] Fix drag and drop behaviour on NS

* doc/emacs/macos.texi (Mac / GNUstep Events): Describe the new drag
and drop behaviour.
* lisp/term/ns-win.el (ns-drag-n-drop): Handle the new event format.
(ns-drag-n-drop-other-frame):
(ns-drag-n-drop-as-text):
(ns-drag-n-drop-as-text-other-frame): Remove functions and key
bindings.
* src/nsterm.m ([EmacsView performDragOperation:]): Send Emacs event
in new format without setting any modifiers.
---
 doc/emacs/macos.texi | 21 ++++++++++--
 etc/NEWS             |  6 ++++
 lisp/term/ns-win.el  | 54 ++++++++++++-----------------
 src/nsterm.m         | 81 ++++++++++++++++++++------------------------
 4 files changed, 84 insertions(+), 78 deletions(-)

diff --git a/doc/emacs/macos.texi b/doc/emacs/macos.texi
index 6d27e97821..d9920957ad 100644
--- a/doc/emacs/macos.texi
+++ b/doc/emacs/macos.texi
@@ -170,8 +170,25 @@ Mac / GNUstep Events
 This event occurs when a user drags an object from another application
 into an Emacs frame.  The default behavior is to open a file in the
 window under the mouse, or to insert text at point of the window under
-the mouse.  It may sometimes be necessary to use the @key{Meta} key in
-conjunction with dragging to force text insertion.
+the mouse.
+
+The sending application has some limited ability to decide how Emacs
+handles the sent object, but the user may override the default
+behaviour by holding one or more modifier key.
+
+@table @kbd
+@item control
+Insert as text in the current buffer.  If the object is a file, this
+will insert the filename.
+@item alt/option
+Attempt to open the object as though it is a file or URL.
+@item super/command
+Perform the default action for the type.  This can be useful when an
+application is overriding the default behaviour.
+@end table
+
+The modifier keys listed above are defined by macOS and are unaffected
+by user changes to the modifiers in Emacs.
 
 @item ns-change-font
 This event occurs when the user selects a font in a Nextstep font
diff --git a/etc/NEWS b/etc/NEWS
index b316aecbfa..3b3c4483e9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1480,6 +1480,12 @@ versions of MS-Windows.  Set this variable to 50 if for some reason
 you need the old behavior (and please report such situations to Emacs
 developers).
 
++++
+** On NS the behaviour of drag and drop can now be modified by use of
+modifier keys in line with Apples guidelines.  This makes the drag and
+drop behaviour more consistent, as previously the sending application
+was able to 'set' modifiers without the knowledge of the user.
+
 \f
 ----------------------------------------------------------------------
 This file is part of GNU Emacs.
diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index c9f5bfef52..6a668b213d 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -501,48 +501,38 @@ ns-find-file
       (find-file f)))))
 
 
-(defun ns-drag-n-drop (event &optional new-frame force-text)
+(defun ns-drag-n-drop (event)
   "Edit the files listed in the drag-n-drop EVENT.
-Switch to a buffer editing the last file dropped."
+Switch to a buffer editing the last file dropped, or insert the
+string dropped into the current buffer."
   (interactive "e")
   (let* ((window (posn-window (event-start event)))
          (arg (car (cdr (cdr event))))
          (type (car arg))
-         (data (car (cdr arg)))
-         (url-or-string (cond ((eq type 'file)
-                               (concat "file:" data))
-                              (t data))))
+         (operations (car (cdr arg)))
+         (objects (cdr (cdr arg)))
+         (string (mapconcat 'identity objects "\n")))
     (set-frame-selected-window nil window)
-    (when new-frame
-      (select-frame (make-frame)))
     (raise-frame)
     (setq window (selected-window))
-    (if force-text
-        (dnd-insert-text window 'private data)
-      (dnd-handle-one-url window 'private url-or-string))))
-
-
-(defun ns-drag-n-drop-other-frame (event)
-  "Edit the files listed in the drag-n-drop EVENT, in other frames.
-May create new frames, or reuse existing ones.  The frame editing
-the last file dropped is selected."
-  (interactive "e")
-  (ns-drag-n-drop event t))
-
-(defun ns-drag-n-drop-as-text (event)
-  "Drop the data in EVENT as text."
-  (interactive "e")
-  (ns-drag-n-drop event nil t))
-
-(defun ns-drag-n-drop-as-text-other-frame (event)
-  "Drop the data in EVENT as text in a new frame."
-  (interactive "e")
-  (ns-drag-n-drop event t t))
+    (cond ((memq 'ns-drag-operation-generic operations)
+           ;; Perform the default action for the type.
+           (if (eq type 'file)
+               (dolist (data objects)
+                 (dnd-handle-one-url window 'private (concat "file:" data)))
+             (dnd-insert-text window 'private string)))
+          ((memq 'ns-drag-operation-copy operations)
+           ;; Try to open the file/URL.  If type is nil, try to open
+           ;; it as a URL anyway.
+           (dolist (data objects)
+             (dnd-handle-one-url window 'private (if (eq type 'file)
+                                                     (concat "file:" data)
+                                                   data))))
+          (t
+           ;; Insert the text as is.
+           (dnd-insert-text window 'private string)))))
 
 (global-set-key [drag-n-drop] 'ns-drag-n-drop)
-(global-set-key [C-drag-n-drop] 'ns-drag-n-drop-other-frame)
-(global-set-key [M-drag-n-drop] 'ns-drag-n-drop-as-text)
-(global-set-key [C-M-drag-n-drop] 'ns-drag-n-drop-as-text-other-frame)
 
 ;;;; Frame-related functions.
 
diff --git a/src/nsterm.m b/src/nsterm.m
index 016c044760..2bce4a89ae 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8230,7 +8230,9 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
   NSEvent *theEvent = [[self window] currentEvent];
   NSPoint position;
   NSDragOperation op = [sender draggingSourceOperationMask];
-  int modifiers = 0;
+  Lisp_Object operations = Qnil;
+  Lisp_Object strings = Qnil;
+  Lisp_Object type_sym;
 
   NSTRACE ("[EmacsView performDragOperation:]");
 
@@ -8243,19 +8245,17 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
   pb = [sender draggingPasteboard];
   type = [pb availableTypeFromArray: ns_drag_types];
 
-  if (! (op & (NSDragOperationMove|NSDragOperationDelete)) &&
-      // URL drags contain all operations (0xf), don't allow all to be set.
-      (op & 0xf) != 0xf)
-    {
-      if (op & NSDragOperationLink)
-        modifiers |= NSEventModifierFlagControl;
-      if (op & NSDragOperationCopy)
-        modifiers |= NSEventModifierFlagOption;
-      if (op & NSDragOperationGeneric)
-        modifiers |= NSEventModifierFlagCommand;
-    }
+  /* We used to convert these drag operations to keyboard modifiers,
+     but because they can be set by the sending program as well as the
+     keyboard modifiers it was difficult to work out a sensible key
+     mapping for drag and drop.  */
+  if (op & NSDragOperationLink)
+    operations = Fcons (Qns_drag_operation_link, operations);
+  if (op & NSDragOperationCopy)
+    operations = Fcons (Qns_drag_operation_copy, operations);
+  if (op & NSDragOperationGeneric || NILP (operations))
+    operations = Fcons (Qns_drag_operation_generic, operations);
 
-  modifiers = EV_MODIFIERS2 (modifiers);
   if (type == 0)
     {
       return NO;
@@ -8269,39 +8269,20 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
       if (!(files = [pb propertyListForType: type]))
         return NO;
 
+      type_sym = Qfile;
+
       fenum = [files objectEnumerator];
       while ( (file = [fenum nextObject]) )
-        {
-          emacs_event->kind = DRAG_N_DROP_EVENT;
-          XSETINT (emacs_event->x, x);
-          XSETINT (emacs_event->y, y);
-          emacs_event->modifiers = modifiers;
-          emacs_event->arg =  list2 (Qfile, build_string ([file UTF8String]));
-          EV_TRAILER (theEvent);
-        }
-      return YES;
+        strings = Fcons (build_string ([file UTF8String]), strings);
     }
   else if ([type isEqualToString: NSURLPboardType])
     {
       NSURL *url = [NSURL URLFromPasteboard: pb];
       if (url == nil) return NO;
 
-      emacs_event->kind = DRAG_N_DROP_EVENT;
-      XSETINT (emacs_event->x, x);
-      XSETINT (emacs_event->y, y);
-      emacs_event->modifiers = modifiers;
-      emacs_event->arg =  list2 (Qurl,
-                                 build_string ([[url absoluteString]
-                                                 UTF8String]));
-      EV_TRAILER (theEvent);
+      type_sym = Qurl;
 
-      if ([url isFileURL] != NO)
-        {
-          NSString *file = [url path];
-          ns_input_file = append2 (ns_input_file,
-                                   build_string ([file UTF8String]));
-        }
-      return YES;
+      strings = Fcons (build_string ([[url absoluteString] UTF8String]), Qnil);
     }
   else if ([type isEqualToString: NSStringPboardType]
            || [type isEqualToString: NSTabularTextPboardType])
@@ -8311,19 +8292,27 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
       if (! (data = [pb stringForType: type]))
         return NO;
 
-      emacs_event->kind = DRAG_N_DROP_EVENT;
-      XSETINT (emacs_event->x, x);
-      XSETINT (emacs_event->y, y);
-      emacs_event->modifiers = modifiers;
-      emacs_event->arg =  list2 (Qnil, build_string ([data UTF8String]));
-      EV_TRAILER (theEvent);
-      return YES;
+      type_sym = Qnil;
+
+      strings = Fcons (build_string ([data UTF8String]), Qnil);
     }
   else
     {
       fprintf (stderr, "Invalid data type in dragging pasteboard");
       return NO;
     }
+
+  emacs_event->kind = DRAG_N_DROP_EVENT;
+  XSETINT (emacs_event->x, x);
+  XSETINT (emacs_event->y, y);
+  emacs_event->modifiers = 0;
+
+  emacs_event->arg = Fcons (type_sym,
+                            Fcons (operations,
+                                   strings));
+  EV_TRAILER (theEvent);
+
+  return YES;
 }
 
 
@@ -9358,6 +9347,10 @@ Convert an X font name (XLFD) to an NS font name.
   DEFSYM (Qfile, "file");
   DEFSYM (Qurl, "url");
 
+  DEFSYM (Qns_drag_operation_copy, "ns-drag-operation-copy");
+  DEFSYM (Qns_drag_operation_link, "ns-drag-operation-link");
+  DEFSYM (Qns_drag_operation_generic, "ns-drag-operation-generic");
+
   Fput (Qalt, Qmodifier_value, make_fixnum (alt_modifier));
   Fput (Qhyper, Qmodifier_value, make_fixnum (hyper_modifier));
   Fput (Qmeta, Qmodifier_value, make_fixnum (meta_modifier));
-- 
2.19.1