bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Paul Magwene, Ph.D.]

* Drag and drop events no longer appear to work properly in Emacs 27.1 on
Mac OS X.

* When I run Emacs from the command line I see the message: `Invalid data
type in dragging pasteboard` when trying to drag text or URLs from a webbrowser
(firefox or safari) into an Emacs buffer.

* I've tested this with two different builds of 27.1 -- from
https://emacsformacosx.com and the "emacs plus" formula under homebrew
(https://github.com/d12frosted/homebrew-emacs-plus) on both OS X 10.14.6
(Mavericks) and 10.15.6 (Catalina)

* Reverting to Emacs 26.3 resolves this issue


In GNU Emacs 27.1 (build 1, x86_64-apple-darwin19.5.0, NS appkit-1894.50 Version 10.15.5 (Build 19F101))
of 2020-08-27 built on d12frosted.local
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.6

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Composing main Info directory...done
Beginning of buffer [18 times]
Mark saved where search started
Beginning of buffer [24 times]
Making completion list...
user-error: End of history; no default available

Configured using:
'configure --disable-dependency-tracking --disable-silent-rules
--enable-locallisppath=/usr/local/share/emacs/site-lisp
--infodir=/usr/local/Cellar/emacs-plus@27/27.1/share/info/emacs
--prefix=/usr/local/Cellar/emacs-plus@27/27.1 --with-xml2 --with-gnutls
--without-dbus --with-imagemagick --with-modules --with-rsvg --with-ns
--disable-ns-self-contained'

Configured features:
RSVG IMAGEMAGICK GLIB NOTIFY KQUEUE ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS NS MODULES THREADS JSON PDUMPER LCMS2 GMP

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec password-cache epa derived epg epg-config
gnus-util rmail rmail-loaddefs text-property-search seq byte-opt gv
bytecomp byte-compile cconv mm-decode mm-bodies mm-encode mail-parse
rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045
ietf-drums mm-util mail-prsvr mail-utils thingatpt jka-compr time-date
subr-x misearch multi-isearch info help-mode easymenu cl-loaddefs cl-lib
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/ns-win ns-win ucs-normalize mule-util term/common-win
tool-bar dnd fontset image regexp-opt fringe tabulated-list replace
newcomment text-mode elisp-mode lisp-mode prog-mode register page
tab-bar menu-bar rfn-eshadow isearch timer select scroll-bar mouse
jit-lock font-lock syntax facemenu font-core term/tty-colors frame
minibuffer cl-generic cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite charscript charprop case-table epa-hook jka-cmpr-hook help
simple abbrev obarray cl-preloaded nadvice loaddefs button faces
cus-face macroexp files text-properties overlay sha1 md5 base64 format
env code-pages mule custom widget hashtable-print-readable backquote
threads kqueue cocoa ns lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 77707 9630)
(symbols 48 6474 1)
(strings 32 20721 1816)
(string-bytes 1 627785)
(vectors 16 11181)
(vector-slots 8 157770 17782)
(floats 8 31 51)
(intervals 56 4895 0)
(buffers 1000 15))


Paul Magwene
Associate Professor
Department of Biology
Duke University

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Thu, Sep 17, 2020 at 01:27:38PM +0000, Paul Magwene, Ph.D. wrote:
> 
> 
> * Drag and drop events no longer appear to work properly in Emacs 27.1 on
> Mac OS X.
> 
> * When I run Emacs from the command line I see the message: `Invalid data
> type in dragging pasteboard` when trying to drag text or URLs from a webbrowser
> (firefox or safari) into an Emacs buffer.
> 
> * I've tested this with two different builds of 27.1 -- from
> https://emacsformacosx.com and the "emacs plus" formula under homebrew
> (https://github.com/d12frosted/homebrew-emacs-plus) on both OS X 10.14.6
> (Mavericks) and 10.15.6 (Catalina)
> 
> * Reverting to Emacs 26.3 resolves this issue

Confirmed.

Annoyingly I could've sworn this worked just a few months ago. I
wonder if Apple have changed something in their libraries to
completely deprecate the old methods.

Anyway, It'll need to be almost completely rewritten (again!).
-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

Alan Third <alan@idiocy.org> writes:
>
> Confirmed.
>
> Annoyingly I could've sworn this worked just a few months ago. I
> wonder if Apple have changed something in their libraries to
> completely deprecate the old methods.
>
> Anyway, It'll need to be almost completely rewritten (again!).

Hi, Alan:

Could it be caused by an incomplete rename done at
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9624f609493da7c08016ba00d6895bad0fe26a0e?

The following patch fixes the reported bug for me on 10.15.6 and Emacs
28.0.50, and also fixes some compilation warnings as well:

diff --git a/src/nsterm.m b/src/nsterm.m
index 26059ab67c..c40f790e0f 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -5648,7 +5648,7 @@ Needs to be here because ns_initialize_display_info () uses AppKit classes.
   ns_drag_types = [[NSArray arrayWithObjects:
                             NSPasteboardTypeString,
                             NSPasteboardTypeTabularText,
-                            NSFilenamesPboardType,
+                            NSPasteboardTypeFileURL,
                             NSPasteboardTypeURL, nil] retain];
 
   /* If fullscreen is in init/default-frame-alist, focus isn't set
@@ -8592,10 +8592,7 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
     {
       return NO;
     }
-  /* FIXME: NSFilenamesPboardType is deprecated in 10.14, but the
-     NSURL method can only handle one file at a time.  Stick with the
-     existing code at the moment.  */
-  else if ([type isEqualToString: NSFilenamesPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeFileURL])
     {
       NSArray *files;
       NSEnumerator *fenum;
@@ -8610,7 +8607,7 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
       while ( (file = [fenum nextObject]) )
         strings = Fcons ([file lispString], strings);
     }
-  else if ([type isEqualToString: NSURLPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeURL])
     {
       NSURL *url = [NSURL URLFromPasteboard: pb];
       if (url == nil) return NO;
@@ -8619,8 +8616,8 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
 
       strings = list1 ([[url absoluteString] lispString]);
     }
-  else if ([type isEqualToString: NSStringPboardType]
-           || [type isEqualToString: NSTabularTextPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeString]
+           || [type isEqualToString: NSPasteboardTypeTabularText])
     {
       NSString *data;
 

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Thu, Sep 17, 2020 at 08:12:56PM +0200, Daniel Mart�­n via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Alan Third <alan@idiocy.org> writes:
> >
> > Confirmed.
> >
> > Annoyingly I could've sworn this worked just a few months ago. I
> > wonder if Apple have changed something in their libraries to
> > completely deprecate the old methods.
> >
> > Anyway, It'll need to be almost completely rewritten (again!).
> 
> Hi, Alan:
> 
> Could it be caused by an incomplete rename done at
> https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9624f609493da7c08016ba00d6895bad0fe26a0e?
> 
> The following patch fixes the reported bug for me on 10.15.6 and Emacs
> 28.0.50, and also fixes some compilation warnings as well:

IIRC (and I could definitely be wrong here) a simple find/replace
breaks the ability to handle more than one file being dragged at a
time, which is why I left it (it used to work fine even though it was
flagged as deprecated).

Obviously that doesn't matter if it now no longer works at all, but we
need to retain backwards compatibility so any patch will have to work
on GNUstep and older versions of macOS. The bottom of nsterm.h shows
how we've handled that with previous deprecations.

Can you check whether your patch handles the multi-file case?
-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

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

Alan Third <alan@idiocy.org> writes:

> IIRC (and I could definitely be wrong here) a simple find/replace
> breaks the ability to handle more than one file being dragged at a
> time, which is why I left it (it used to work fine even though it was
> flagged as deprecated).

You're right. I also misread the FIXME, which mentions this particular
problem. That part of the patch is not correct.

I think we need to use the same constant names as those that are
included in the ns_drag_types array, at least until we implement proper
support for the new pasteboard API. This means that three constant names
should be renamed in nsterm.m.

I've attached a new patch that implements this idea. I've tested the
following drag and drop operations work correctly after the patch is
applied:

- Drag a URL.
- Drag arbitrary text from another application.
- Drag tabular text from another application.
- Drag a couple of files from the OS file manager. It correctly creates
a couple of buffers in Emacs that visit those files.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-modern-constant-names-for-the-NS-pasteboard.patch --]
[-- Type: text/x-patch, Size: 1504 bytes --]

From 8d50e8c08600306a6355014d90eb21b1b15af59e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Fri, 18 Sep 2020 13:36:47 +0200
Subject: [PATCH] Use modern constant names for the NS pasteboard

Use the same pasteboard constant names defined in
ns_drag_types. (Bug#43470).

* src/nsterm.m: Rename NSURLPboardType to NSPasteboardTypeURL,
NSStringPboardType to NSPasteboardTypeString, and
NSTabularTextPboardType to NSPasteboardTypeTabularText
---
 src/nsterm.m | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 26059ab67c..bab7a0abb9 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8610,7 +8610,7 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
       while ( (file = [fenum nextObject]) )
         strings = Fcons ([file lispString], strings);
     }
-  else if ([type isEqualToString: NSURLPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeURL])
     {
       NSURL *url = [NSURL URLFromPasteboard: pb];
       if (url == nil) return NO;
@@ -8619,8 +8619,8 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
 
       strings = list1 ([[url absoluteString] lispString]);
     }
-  else if ([type isEqualToString: NSStringPboardType]
-           || [type isEqualToString: NSTabularTextPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeString]
+           || [type isEqualToString: NSPasteboardTypeTabularText])
     {
       NSString *data;
 
-- 
2.28.0

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Fri, Sep 18, 2020 at 02:00:16PM +0200, Daniel Martín wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > IIRC (and I could definitely be wrong here) a simple find/replace
> > breaks the ability to handle more than one file being dragged at a
> > time, which is why I left it (it used to work fine even though it was
> > flagged as deprecated).
> 
> You're right. I also misread the FIXME, which mentions this particular
> problem. That part of the patch is not correct.
> 
> I think we need to use the same constant names as those that are
> included in the ns_drag_types array, at least until we implement proper
> support for the new pasteboard API. This means that three constant names
> should be renamed in nsterm.m.

I'll have to have a look at the new API again, I can't remember
anything about it or why I didn't use it the last time I worked on
this. If you fancy giving it a go, feel free.

> I've attached a new patch that implements this idea. I've tested the
> following drag and drop operations work correctly after the patch is
> applied:
> 
> - Drag a URL.
> - Drag arbitrary text from another application.
> - Drag tabular text from another application.
> - Drag a couple of files from the OS file manager. It correctly creates
> a couple of buffers in Emacs that visit those files.

This looks good to me. I think we'll want to apply it to Emacs 27
since this is a regression from Emacs 26 and the fix is minimal, so
can you please rebase against emacs-27 and I'll push it there.

Thanks!

-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

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

Alan Third <alan@idiocy.org> writes:
>
> This looks good to me. I think we'll want to apply it to Emacs 27
> since this is a regression from Emacs 26 and the fix is minimal, so
> can you please rebase against emacs-27 and I'll push it there.
>
> Thanks!

Here's the same patch applied on top of the emacs-27 branch. Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Use-modern-constant-names-for-the-NS-pasteboard.patch --]
[-- Type: text/x-patch, Size: 1534 bytes --]

From 4e3dc31ab100467fb39bce6e687aef029f0d3225 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Fri, 18 Sep 2020 13:36:47 +0200
Subject: [PATCH] Use modern constant names for the NS pasteboard

Use the same pasteboard constant names defined in
ns_drag_types. (Bug#43470).

* src/nsterm.m: Rename NSURLPboardType to NSPasteboardTypeURL,
NSStringPboardType to NSPasteboardTypeString, and
NSTabularTextPboardType to NSPasteboardTypeTabularText
---
 src/nsterm.m | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index ac467840a2..3dd915e370 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -8363,7 +8363,7 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
       while ( (file = [fenum nextObject]) )
         strings = Fcons (build_string ([file UTF8String]), strings);
     }
-  else if ([type isEqualToString: NSURLPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeURL])
     {
       NSURL *url = [NSURL URLFromPasteboard: pb];
       if (url == nil) return NO;
@@ -8372,8 +8372,8 @@ -(BOOL)performDragOperation: (id <NSDraggingInfo>) sender
 
       strings = list1 (build_string ([[url absoluteString] UTF8String]));
     }
-  else if ([type isEqualToString: NSStringPboardType]
-           || [type isEqualToString: NSTabularTextPboardType])
+  else if ([type isEqualToString: NSPasteboardTypeString]
+           || [type isEqualToString: NSPasteboardTypeTabularText])
     {
       NSString *data;
 
-- 
2.28.0

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Fri, Sep 18, 2020 at 08:34:30PM +0200, Daniel Mart�­n via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
> Alan Third <alan@idiocy.org> writes:
> >
> > This looks good to me. I think we'll want to apply it to Emacs 27
> > since this is a regression from Emacs 26 and the fix is minimal, so
> > can you please rebase against emacs-27 and I'll push it there.
> >
> > Thanks!
> 
> Here's the same patch applied on top of the emacs-27 branch. Thanks.

Thanks! I've pushed it to emacs-27 and it will be merged into master
in due course.

I'll close this bug report now and if anyone wants to modernise the
drag and drop code they can create a new one with the patch or
whatever.

-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Paul Magwene]

On 9/18/2020 3:11 PM, Alan Third wrote:
> On Fri, Sep 18, 2020 at 08:34:30PM +0200, Daniel Mart�­n via Bug reports for GNU Emacs, the Swiss army knife of text editors wrote:
>> Alan Third <alan@idiocy.org> writes:
>>>
>>> This looks good to me. I think we'll want to apply it to Emacs 27
>>> since this is a regression from Emacs 26 and the fix is minimal, so
>>> can you please rebase against emacs-27 and I'll push it there.
>>>
>>> Thanks!
>>
>> Here's the same patch applied on top of the emacs-27 branch. Thanks.
> 
> Thanks! I've pushed it to emacs-27 and it will be merged into master
> in due course.
> 
> I'll close this bug report now and if anyone wants to modernise the
> drag and drop code they can create a new one with the patch or
> whatever.
> 

Hi Alan and Daniel,

I can confirm this patch restores basic drag-and-drop functionality -- 
for example I can drag URLs from a browser into emacs.

However, there still seems to be regression with respect to the behavior 
of the package org-download (https://github.com/abo-abo/org-download) -- 
images dragged from a web browser are no longer recognized as 
attachments, only their URLs are getting pasted.

Best,
Paul

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Fri, Sep 18, 2020 at 05:27:17PM -0400, Paul Magwene wrote:
> I can confirm this patch restores basic drag-and-drop functionality -- for
> example I can drag URLs from a browser into emacs.
> 
> However, there still seems to be regression with respect to the behavior of
> the package org-download (https://github.com/abo-abo/org-download) -- images
> dragged from a web browser are no longer recognized as attachments, only
> their URLs are getting pasted.

Try holding the option key when dragging into the Emacs frame.

Emacs 26 didn't handle drag and drop according to Apple's guidelines,
which meant that different source applications were able to force
Emacs to handle drag and drop in apparently arbitrary ways.

It didn't help that changing which keys worked as meta and super
affected the drag and drop in unexpected ways too!

The result was that there was no way to be able to predict what would
happen when you dragged something into Emacs. For example, dragging
highlighted text from iTerm would result in Emacs doing something
different than when dragging highlighted text from TextEdit.

More info here:

http://emacs.1067599.n8.nabble.com/bug-30929-26-0-91-Text-drag-and-drop-does-not-work-td451899.html

-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Paul Magwene, Ph.D.]

> On Sep 18, 2020, at 6:15 PM, Alan Third <alan@idiocy.org> wrote:
> 
> On Fri, Sep 18, 2020 at 05:27:17PM -0400, Paul Magwene wrote:
>> I can confirm this patch restores basic drag-and-drop functionality -- for
>> example I can drag URLs from a browser into emacs.
>> 
>> However, there still seems to be regression with respect to the behavior of
>> the package org-download (https://urldefense.com/v3/__https://github.com/abo-abo/org-download__;!!OToaGQ!8m3-afgTMTEJM6rEIKhlRQZ4Hu1ZcDjjtoDKEKWvNfsYjq_DrTj5_mBe8gX44ZvuVvg$ ) -- images
>> dragged from a web browser are no longer recognized as attachments, only
>> their URLs are getting pasted.
> 
> Try holding the option key when dragging into the Emacs frame.
> 
> Emacs 26 didn't handle drag and drop according to Apple's guidelines,
> which meant that different source applications were able to force
> Emacs to handle drag and drop in apparently arbitrary ways.
> 
> It didn't help that changing which keys worked as meta and super
> affected the drag and drop in unexpected ways too!
> 
> The result was that there was no way to be able to predict what would
> happen when you dragged something into Emacs. For example, dragging
> highlighted text from iTerm would result in Emacs doing something
> different than when dragging highlighted text from TextEdit.
> 
> More info here:
> 
> https://urldefense.com/v3/__http://emacs.1067599.n8.nabble.com/bug-30929-26-0-91-Text-drag-and-drop-does-not-work-td451899.html__;!!OToaGQ!8m3-afgTMTEJM6rEIKhlRQZ4Hu1ZcDjjtoDKEKWvNfsYjq_DrTj5_mBe8gX42IpwYfI$ 
> 
> -- 
> Alan Third

My testing suggests that there's still source application specific behavior.

* Simple drag of images works when Safari or Chrome is the web browser.

* No combination of Option, Command, or Control seems to work in Firefox; the drag behavior always produces a URL.

So I guess the patch partially fixes the regression.  Given the state of the OS X api I'm not sure what the best way forward is. I'd try and jump in to contribute but I unfortunately have zero experience working with Objective C or programming against Apple's APIs

Best,
Paul

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

"Paul Magwene, Ph.D." <paul.magwene@duke.edu> writes:
>
> My testing suggests that there's still source application specific behavior.
>
> * Simple drag of images works when Safari or Chrome is the web browser.
>
> * No combination of Option, Command, or Control seems to work in Firefox; the drag behavior always produces a URL.
>

Thank you for the additional testing. I can reproduce the drag behavior
differences between 26.3 and 27.1, and also the Firefox vs. other
browsers difference.

I'm familiarizing with that part of the codebase and I'll prepare a
patch that will keep the same default drag behavior as in 26.3.

Thanks.

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

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

"Paul Magwene, Ph.D." <paul.magwene@duke.edu> writes:
>
> My testing suggests that there's still source application specific behavior.
>
> * Simple drag of images works when Safari or Chrome is the web browser.
>
> * No combination of Option, Command, or Control seems to work in Firefox; the drag behavior always produces a URL.
>
> So I guess the patch partially fixes the regression.  Given the state
> of the OS X api I'm not sure what the best way forward is. I'd try and
> jump in to contribute but I unfortunately have zero experience working
> with Objective C or programming against Apple's APIs
>
> Best,
> Paul

OK, I've created a patch on top of emacs-27. I've tested it by dragging
and dropping files, text, URLs, images, from Safari and Firefox, and I
get the same default behavior as in Emacs 26.3. Note that you should
still be able to override the default behavior by pressing Option,
Command, etc. while dragging the content.

Could you test it and see if it completely fixes the drag and drop
behavior on macOS?

Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Give-ns-drag-operation-copy-preference-when-handling.patch --]
[-- Type: text/x-patch, Size: 2361 bytes --]

From 1a41df947bcf102e0884f295d8f394ade8d9eb18 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=ADn?= <mardani29@yahoo.es>
Date: Sat, 19 Sep 2020 14:13:53 +0200
Subject: [PATCH] Give ns-drag-operation-copy preference when handling drag and
 drop

Some applications like Firefox send NSDragOperationEvery when dragging
URLs or images. That constant means "every possible operation (copy,
link, move)". When there's more than one operation, we want to
consider specialized actions like ns-drag-operation-copy before
generic ones like ns-drag-operation-generic.

This change brings back the same drag and drop behavior as in Emacs
26.3. (Bug#43470).

* lisp/term/ns-win.el (ns-drag-n-drop): Move the check for
ns-drag-operation-copy before ns-drag-operation-generic.
---
 lisp/term/ns-win.el | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index 90024b001f..fc20de5a05 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -513,19 +513,19 @@ ns-drag-n-drop
     (set-frame-selected-window nil window)
     (raise-frame)
     (setq window (selected-window))
-    (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)
+    (cond ((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))))
+          ((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)))
           (t
            ;; Insert the text as is.
            (dnd-insert-text window 'private string)))))
-- 
2.28.0

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Fri, Sep 18, 2020 at 11:22:46PM +0000, Paul Magwene, Ph.D. wrote:
> 
> My testing suggests that there's still source application specific behavior.
> 
> * Simple drag of images works when Safari or Chrome is the web browser.
> 
> * No combination of Option, Command, or Control seems to work in Firefox; the drag behavior always produces a URL.

That's how Apple designed it, what we wanted to avoid was where the
source application defined which modifiers Emacs saw in use as that
just turned into a nightmare of random behaviour.

Probably what's happening is that Firefox is setting some drag
operations that don't include NSDragOperationCopy, which is the one we
chose to always try opening the text as a URL.

It would be interesting to know what it's setting. You could try
adding something like

    NSLog ("%lx", op);

into performDragOperation in nsterm.m and then we can compare it to
the constants listed here:

    https://developer.apple.com/documentation/appkit/nsdragoperation?language=objc

> So I guess the patch partially fixes the regression. Given the state
> of the OS X api I'm not sure what the best way forward is. I'd try
> and jump in to contribute but I unfortunately have zero experience
> working with Objective C or programming against Apple's APIs

The ns-drag-n-drop function is customisable, so if you wanted to
ALWAYS force it to try and open ANY text as a URL you could do
something like

(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."
  (interactive "e")
  (let* ((window (posn-window (event-start event)))
         (arg (car (cdr (cdr event))))
         (type (car arg))
         (objects (cdr (cdr arg))))
    (set-frame-selected-window nil window)
    (raise-frame)
    (setq window (selected-window))
    (princ operations)
    (dolist (data objects)
      (dnd-handle-one-url window 'private (if (eq type 'file)
                                              (concat "file:" data)
                                            data)))))

It's quite possible we should have more logic in ns-drag-n-drop to
work out what to do given the data type and available dragging
options.

BTW, Apple's documentation seems to be inconsistent. One page[1] has
this table:

    | Modifier Key       | Dragging Operation  |
    |--------------------+---------------------|
    | Option             | NSDragOperationCopy | 
    | Command            | NSDragOperationMove | 
    | Option and Command | NSDragOperationLink | 

and another[2] has this:

    | Modifier Key | Dragging Operation     |
    |--------------+------------------------|
    | Control      | NSDragOperationLink    |
    | Option       | NSDragOperationCopy    |
    | Command      | NSDragOperationGeneric |

The latter seems to be the correct one.

[1] https://developer.apple.com/documentation/appkit/nsdragginginfo/1415966-draggingsourceoperationmask?language=objc
[2] https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/DragandDrop/Concepts/dragsource.html#//apple_ref/doc/uid/20000976-104936
-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Sat, Sep 19, 2020 at 02:45:19PM +0200, Daniel Martín wrote:
> 
> Some applications like Firefox send NSDragOperationEvery when dragging
> URLs or images. That constant means "every possible operation (copy,
> link, move)". When there's more than one operation, we want to
> consider specialized actions like ns-drag-operation-copy before
> generic ones like ns-drag-operation-generic.

Hi Daniel,

I don't think this is correct, we should always choose generic when
it's available as that is the "default" action taken by the
application. The more specialised copy, move, etc., are there for the
sending application (or user via modifiers) to force the receiving
application to do something else.

The big decision is what should Emacs do by default? My opinion is
that when receiving a URL we should insert the link as text rather
than try to open it: Emacs isn't a web browser. Perhaps that's wrong,
I'm not sure what Emacs does on other platforms.

> This change brings back the same drag and drop behavior as in Emacs
> 26.3. (Bug#43470).

This is untrue as the Emacs 26 behaviour was almost entirely arbitrary
and broken. What it does is fix this one particular use-case.
-- 
Alan Third

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

Alan Third <alan@idiocy.org> writes:
>
> The big decision is what should Emacs do by default? My opinion is
> that when receiving a URL we should insert the link as text rather
> than try to open it: Emacs isn't a web browser. Perhaps that's wrong,
> I'm not sure what Emacs does on other platforms.

I've tested on GNU/Linux (Emacs 27.1 and Emacs 24) and the behavior is
that dragging an dropping a URL from Firefox opens the web page source,
just like Emacs 26 on macOS. The GNU/Linux version does not seem to
support overriding the destination action with Ctrl, Shift, etc.

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

Alan Third <alan@idiocy.org> writes:
>
> The big decision is what should Emacs do by default? My opinion is
> that when receiving a URL we should insert the link as text rather
> than try to open it: Emacs isn't a web browser. Perhaps that's wrong,
> I'm not sure what Emacs does on other platforms.

I've tested on GNU/Linux (Emacs 27.1 and Emacs 24) and the behavior is
that dragging an dropping a URL from Firefox opens the web page source,
just like Emacs 26 on macOS. The GNU/Linux version does not seem to
support overriding the destination action with Ctrl, Shift, etc.

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

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

On Sat, Sep 26, 2020 at 01:39:03PM +0200, Daniel Martín wrote:
> Alan Third <alan@idiocy.org> writes:
> >
> > The big decision is what should Emacs do by default? My opinion is
> > that when receiving a URL we should insert the link as text rather
> > than try to open it: Emacs isn't a web browser. Perhaps that's wrong,
> > I'm not sure what Emacs does on other platforms.
> 
> I've tested on GNU/Linux (Emacs 27.1 and Emacs 24) and the behavior is
> that dragging an dropping a URL from Firefox opens the web page source,
> just like Emacs 26 on macOS. The GNU/Linux version does not seem to
> support overriding the destination action with Ctrl, Shift, etc.

I've been fiddling with this more and I've realised that (at least on
my machine today) Chrome and Safari never actually send the URLs as
NSPasteboardTypeURL, they just send them as plain text.

I suspect this means that if we want to support the NS drag and drop
process correctly (and I think we do) then we really need to look into
a larger rewrite, unfortunately. And any larger rewrite will need to
wait for Emacs 28, I think.

In the meantime I suppose the simplest work-around is something like
the attached, which is near enough the same as what you did, Daniel,
but making explicit that "copy" and "generic" are doing the same
thing.

-- 
Alan Third

[-- Attachment #2: 0001-Make-drag-and-drop-on-NS-open-all-URLs-bug-43470.patch --]
[-- Type: text/plain, Size: 1638 bytes --]

From 908a5fade4afc77467ef61b771c4e3c46c8cd798 Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 27 Sep 2020 10:55:32 +0100
Subject: [PATCH] Make drag and drop on NS open all URLs (bug#43470)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/term/ns-win.el (ns-drag-n-drop): Merge generic and copy
actions.

Co-authored-by: Daniel Martín <mardani29@yahoo.es>
---
 lisp/term/ns-win.el | 12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/lisp/term/ns-win.el b/lisp/term/ns-win.el
index 90024b001f..184271d9e6 100644
--- a/lisp/term/ns-win.el
+++ b/lisp/term/ns-win.el
@@ -513,15 +513,9 @@ ns-drag-n-drop
     (set-frame-selected-window nil window)
     (raise-frame)
     (setq window (selected-window))
-    (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.
+    (cond ((or (memq 'ns-drag-operation-generic operations)
+               (memq 'ns-drag-operation-copy operations))
+           ;; Perform the default/copy action.
            (dolist (data objects)
              (dnd-handle-one-url window 'private (if (eq type 'file)
                                                      (concat "file:" data)
-- 
2.26.1

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Unknown]

Alan Third <alan@idiocy.org> writes:

> In the meantime I suppose the simplest work-around is something like
> the attached, which is near enough the same as what you did, Daniel,
> but making explicit that "copy" and "generic" are doing the same
> thing.

I've tested the patch and it works fine for the use cases mentioned by
the OP.

Thanks.

bug#43470: 27.1; Drag and Drop not working properly in 27.1 on OSX

[Alan Third]

On Mon, Sep 28, 2020 at 12:22:35AM +0200, Daniel Martín wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > In the meantime I suppose the simplest work-around is something like
> > the attached, which is near enough the same as what you did, Daniel,
> > but making explicit that "copy" and "generic" are doing the same
> > thing.
> 
> I've tested the patch and it works fine for the use cases mentioned by
> the OP.

Thanks.

I've pushed it to the emacs-27 branch.
-- 
Alan Third