Fix X11 copy-paste and drag-and-drop target selection (#182)

Select the "best" target rather than a random one out of a list of
suitable targets. The old target selection algorithm would sometimes
select the wrong target and hence retrieve unexpected data.

This could happen in both copy-paste and drag-and-drop operations.

Note: backported to 1.3.6 (git current) as well
      (commit 7ce6d2cf5d).

Closes #182.

CHANGES.txt

@@ -127,6 +127,8 @@ Changes in FLTK 1.4.0                            Released: ??? ?? 2021
   Other Improvements
 
   - (add new items here)
+  - Fixed X11 copy-paste and drag-and-drop target selection (issue #182).
+    This fix has been backported to 1.3.6 as well.
   - Added support for macOS 11.0 "Big Sur" and for building for
     the arm64 architecture.
   - Add optional argument to Fl_Printer::begin_job() to receive

src/Fl_x.cxx

@@ -1,7 +1,7 @@
 //
 // X specific code for the Fast Light Tool Kit (FLTK).
 //
-// Copyright 1998-2020 by Bill Spitzak and others.
+// Copyright 1998-2021 by Bill Spitzak and others.
 //
 // This library is free software. Distribution and use rights are outlined in
 // the file "COPYING" which should have been included with this file.  If this
@@ -344,7 +344,6 @@ Atom fl_XdndDrop;
 Atom fl_XdndStatus;
 Atom fl_XdndActionCopy;
 Atom fl_XdndFinished;
-//Atom fl_XdndProxy;
 Atom fl_XdndURIList;
 static Atom fl_Xatextplainutf;
 static Atom fl_Xatextplainutf2;         // STR#2930
@@ -367,6 +366,98 @@ Atom fl_NET_WORKAREA;
 static Atom fl_NET_WM_ICON;
 static Atom fl_NET_ACTIVE_WINDOW;
 
+/*
+  Debug: translate Atom (number) to name: enable (1) if used below
+*/
+#if (0)
+static void debug_atom_name(const char *fun, int line, Atom atom) {
+  char *name = XGetAtomName(fl_display, atom);
+  fprintf(stderr, "[%s:%d] debug_atom_name (%4lu) = %s\n", fun, line, atom, name);
+  XFree(name);
+}
+#endif
+
+/*
+  Find the best target in a list of available copy/paste or dnd targets.
+
+  This function searches all available targets [avail, na] by comparing
+  these with an ordered list of suitable targets [targets, nt].
+  The size of the list of available targets is determined by 'na'.
+
+  The list of suitable targets must be ordered by preference with the
+  best target first. This ensures that we always find our preferred
+  target, no matter how the list of available targets is ordered.
+  The list size is max. 'nt' entries but can also be terminated
+  by a zero (0) entry.
+
+  The list of suitable targets is provided by FLTK (see below); the
+  list of available targets is provided by the data source program.
+
+  Returns: (Atom) the "best target" or 0 (zero) if no target matches.
+*/
+
+static Atom find_target(Atom *targets, int nt, Atom *avail, int na) {
+  Atom retval = 0;
+  Atom mt, at;
+  int i = 0, m = 0;
+#if (0) // Debug
+  fprintf(stderr, "find_target: looking for:\n");
+  for (i = 0; i < nt; i++)
+    debug_atom_name("  find_target", i, targets[i]);
+  for (i = 0; i < na; i++)
+    debug_atom_name("  available  ", i, avail[i]);
+#endif // Debug
+  int n = nt;
+  for (i = 0; i < na; i++) { // search all available targets
+    at = avail[i];
+    // search only *better* targets: m < n !
+    for (m = 0; m < n; m++) { // try to match usable targets
+      mt = targets[m];
+      if (!mt) break; // end of list
+      if (mt == at) {
+        n = m;
+        retval = mt;
+        break;
+      }
+    }
+    if (n == 0) break; // found the *best* target (0)
+  }
+  // debug_atom_name("find_target: FOUND", n, retval);
+  return retval;
+}
+
+/*
+  Find a suitable target for text insertion in a list of available targets.
+*/
+static Atom find_target_text(Atom *avail, int na) {
+  static Atom text_targets[] = {
+    fl_XaUtf8String,      // "UTF8_STRING"
+    fl_Xatextplainutf2,   // "text/plain;charset=utf-8" (correct: lowercase)
+    fl_Xatextplainutf,    // "text/plain;charset=UTF-8" (non-standard: compat.)
+    fl_Xatextplain,       // "text/plain"
+    XA_STRING,            // "STRING"
+    fl_XaText,            // "TEXT"
+    fl_XaCompoundText,    // "COMPOUND_TEXT"
+    fl_XaTextUriList,     // "text/uri-list" (e.g. file manager)
+    (Atom)0               // list terminator (optional, but don't remove)
+  };
+  static int nt = sizeof(text_targets) / sizeof(Atom) - 1; // list size w/o terminator
+  return find_target(text_targets, nt, avail, na);
+}
+
+/*
+  Find a suitable target for image insertion in a list of available targets.
+*/
+static Atom find_target_image(Atom *avail, int na) {
+  static Atom image_targets[] = {
+    fl_XaImageBmp,        // "image/bmp"
+    fl_XaImagePNG,        // "image/png"
+    (Atom)0               // list terminator (optional, but don't remove)
+  };
+  static int ni = sizeof(image_targets) / sizeof(Atom) - 1; // list size w/o terminator
+  return find_target(image_targets, ni, avail, na);
+}
+
 /*
   X defines 32-bit-entities to have a format value of max. 32,
   although sizeof(atom) can be 8 (64 bits) on a 64-bit OS.
@@ -645,7 +736,6 @@ void open_display_i(Display* d) {
   fl_XdndStatus         = XInternAtom(d, "XdndStatus",          0);
   fl_XdndActionCopy     = XInternAtom(d, "XdndActionCopy",      0);
   fl_XdndFinished       = XInternAtom(d, "XdndFinished",        0);
-  //fl_XdndProxy        = XInternAtom(d, "XdndProxy",           0);
   fl_XdndEnter          = XInternAtom(d, "XdndEnter",           0);
   fl_XdndURIList        = XInternAtom(d, "text/uri-list",       0);
   fl_Xatextplainutf     = XInternAtom(d, "text/plain;charset=UTF-8",0);
@@ -839,31 +929,13 @@ int Fl_X11_System_Driver::clipboard_contains(const char *type)
                      0, 4000, 0, 0,
                      &actual, &format, &count, &remaining, &portion);
   if (actual != XA_ATOM) return 0;
-  Atom t;
+  Atom t = (Atom)0;
-  int retval = 0;
+  if (strcmp(type, Fl::clipboard_plain_text) == 0)
-  if (strcmp(type, Fl::clipboard_plain_text) == 0) {
+    t = find_target_text((Atom*)portion, count);
-    for (i = 0; i<count; i++) { // searching for text data
+  else if (strcmp(type, Fl::clipboard_image) == 0)
-      t = ((Atom*)portion)[i];
+    t = find_target_image((Atom*)portion, count);
-      if (t == fl_Xatextplainutf ||
-          t == fl_Xatextplainutf2 ||
-          t == fl_Xatextplain ||
-          t == fl_XaUtf8String) {
-        retval = 1;
-        break;
-      }
-    }
-  }
-  else if (strcmp(type, Fl::clipboard_image) == 0) {
-    for (i = 0; i<count; i++) { // searching for image data
-      t = ((Atom*)portion)[i];
-      if (t == fl_XaImageBmp || t == fl_XaImagePNG) {
-        retval = 1;
-        break;
-      }
-    }
-  }
   XFree(portion);
-  return retval;
+  return (t ? 1 : 0);
 }
 
 static Window fl_dnd_source_window;
@@ -1417,7 +1489,6 @@ int fl_handle(const XEvent& thisevent)
 
   case SelectionNotify: {
     static unsigned char* sn_buffer = 0;
-    //static const char *buffer_format = 0;
     if (sn_buffer) {XFree(sn_buffer); sn_buffer = 0;}
     long bytesread = 0;
     if (fl_xevent->xselection.property) for (;;) {
@@ -1447,64 +1518,43 @@ int fl_handle(const XEvent& thisevent)
       }
 
       if (actual == TARGETS || actual == XA_ATOM) {
-        /*for (unsigned i = 0; i<count; i++) {
+        Atom type;
-            fprintf(stderr," %s", XGetAtomName(fl_display, ((Atom*)portion)[i]) );
-          }
-          fprintf(stderr,"\n");
-        */
-        Atom t, type = XA_STRING;
         if (Fl::e_clipboard_type == Fl::clipboard_image) { // searching for image data
-          for (unsigned i = 0; i<count; i++) {
+          type = find_target_image((Atom *)portion, count);
-            t = ((Atom*)portion)[i];
-            if (t == fl_XaImageBmp || t == fl_XaImagePNG) {
-              type = t;
-              goto found;
-            }
-          }
-          XFree(portion);
-          return true;
         }
-        for (unsigned i = 0; i<count; i++) { // searching for text data
+        else { // searching for text data. *FIXME* - there may be other data types!
-          t = ((Atom*)portion)[i];
+          type = find_target_text((Atom *)portion, count);
-          if (t == fl_Xatextplainutf ||
-              t == fl_Xatextplainutf2 ||
-              t == fl_Xatextplain ||
-              t == fl_XaUtf8String) {
-            type = t;
-            break;
-          }
-          // rest are only used if no UTF-8 available:
-          if (t == fl_XaText ||
-              t == fl_XaTextUriList ||
-              t == fl_XaCompoundText) type = t;
         }
-      found:
         XFree(portion); portion = 0;
+
+        if (!type) { // not found
+          if (Fl::e_clipboard_type == Fl::clipboard_image)
+            return true;
+          else
+            type = fl_XaUtf8String; // text: try this anyway (was: XA_STRING)
+        } // not found
+
         Atom property = xevent.xselection.property;
         XConvertSelection(fl_display, property, type, property,
               fl_xid(Fl::first_window()),
               fl_event_time);
         if (type == fl_XaImageBmp) {
           Fl::e_clipboard_type = Fl::clipboard_image;
-          //buffer_format = "image/bmp";
+        }
-          }
         else if (type == fl_XaImagePNG) {
           Fl::e_clipboard_type = Fl::clipboard_image;
-          //buffer_format = "image/png";
+        }
-          }
         else {
           Fl::e_clipboard_type = Fl::clipboard_plain_text;
-          // buffer_format = Fl::clipboard_plain_text;
+        }
-          }
-          // fprintf(stderr,"used format=%s\n", buffer_format);
         return true;
       }
-        if (actual == fl_INCR) {
+      if (actual == fl_INCR) {
-          bytesread = getIncrData(sn_buffer, xevent.xselection, *(long*)portion);
+        bytesread = getIncrData(sn_buffer, xevent.xselection, *(long*)portion);
-          XFree(portion);
+        XFree(portion);
-          break;
+        break;
-        }
+      }
-        // Make sure we got something sane...
+      // Make sure we got something sane...
       if ((portion == NULL) || (format != 8) || (count == 0)) {
         if (portion) { XFree(portion); portion = 0; }
         return true;
@@ -1549,12 +1599,12 @@ int fl_handle(const XEvent& thisevent)
     else if (Fl::e_clipboard_type == Fl::clipboard_plain_text) {
       Fl::e_text = sn_buffer ? (char*)sn_buffer : (char *)"";
       Fl::e_length = bytesread;
-      }
+    }
     int old_event = Fl::e_number;
     int retval = fl_selection_requestor->handle(Fl::e_number = FL_PASTE);
     if (!retval && Fl::e_clipboard_type == Fl::clipboard_image) {
       delete (Fl_RGB_Image*)Fl::e_clipboard_data;
-      Fl::e_clipboard_data= NULL;
+      Fl::e_clipboard_data = NULL;
     }
     Fl::e_number = old_event;
     // Detect if this paste is due to Xdnd by the property name (I use
@@ -1567,13 +1617,15 @@ int fl_handle(const XEvent& thisevent)
                            fl_xevent->xselection.requestor);
       fl_dnd_source_window = 0; // don't send a second time
     }
-    return 1;}
+    return 1;
+  } // SelectionNotify
 
   case SelectionClear: {
     int clipboard = fl_xevent->xselectionclear.selection == CLIPBOARD;
     fl_i_own_selection[clipboard] = 0;
     poll_clipboard_owner();
-    return 1;}
+    return 1;
+  }
 
   case SelectionRequest: {
     XSelectionEvent e;
@@ -1590,7 +1642,7 @@ int fl_handle(const XEvent& thisevent)
         XChangeProperty(fl_display, e.requestor, e.property,
                         XA_ATOM, atom_bits, 0, (unsigned char*)a, 3);
       } else {
-        if (/*e.target == XA_STRING &&*/ fl_selection_length[clipboard]) {
+        if (fl_selection_length[clipboard]) { // data available
           if (e.target == fl_XaUtf8String ||
               e.target == XA_STRING ||
               e.target == fl_XaCompoundText ||
@@ -1607,10 +1659,7 @@ int fl_handle(const XEvent& thisevent)
                             (unsigned char *)fl_selection_buffer[clipboard],
                             fl_selection_length[clipboard]);
           }
-        } else {
+        } else { // no data available
-          //    char* x = XGetAtomName(fl_display,e.target);
-          //    fprintf(stderr,"selection request of %s\n",x);
-          //    XFree(x);
           e.property = 0;
         }
       }
@@ -1630,8 +1679,9 @@ int fl_handle(const XEvent& thisevent)
         }
       }
     }
-    XSendEvent(fl_display, e.requestor, 0, 0, (XEvent *)&e);}
+    XSendEvent(fl_display, e.requestor, 0, 0, (XEvent *)&e);
     return 1;
+  } // SelectionRequest
 
   // events where interesting window id is in a different place:
   case CirculateNotify:
@@ -1647,7 +1697,7 @@ int fl_handle(const XEvent& thisevent)
   case UnmapNotify:
     xid = xevent.xmaprequest.window;
     break;
-  }
+  } // switch (xevent.type)
 
   int event = 0;
   Fl_Window* window = fl_find(xid);
@@ -1664,7 +1714,7 @@ int fl_handle(const XEvent& thisevent)
         window->position(0, 0);
         window->position(oldx, oldy);
         window->show(); // recreate the X11 window in support of the FLTK window
-        }
+      }
       return 1;
     }
   case ClientMessage: {
@@ -1677,7 +1727,7 @@ int fl_handle(const XEvent& thisevent)
       in_a_window = true;
       fl_dnd_source_window = data[0];
       // version number is data[1]>>24
-//      printf("XdndEnter, version %ld\n", data[1] >> 24);
+      // fprintf(stderr, "XdndEnter, version %ld\n", data[1] >> 24);
       if (data[1]&1) {
         // get list of data types:
         Atom actual; int format; unsigned long count, remaining;
@@ -1706,25 +1756,16 @@ int fl_handle(const XEvent& thisevent)
         fl_dnd_source_types[3] = 0;
       }
 
-      // Loop through the source types and pick the first text type...
+      // Pick the "best" source (text) type...
-      unsigned i;
+      // *FIXME* what if we don't find a suitable type? (see below: first type?)
-      Atom type = ((Atom*)fl_dnd_source_types)[0];
+      // *FIXME* count (zero terminated) dnd sources (must be at least 1)
-      for (i = 0; fl_dnd_source_types[i]; i ++) {
+      int dnd_sources;
-        Atom t = ((Atom*)fl_dnd_source_types)[i];
+      for (dnd_sources = 0; fl_dnd_source_types[dnd_sources]; dnd_sources++) {
-        //printf("fl_dnd_source_types[%d]=%ld(%s)\n",i,t,XGetAtomName(fl_display,t));
+        // empty
-        if (t == fl_Xatextplainutf ||           // "text/plain;charset=UTF-8"
-            t == fl_Xatextplainutf2 ||          // "text/plain;charset=utf-8" -- See STR#2930
-            t == fl_Xatextplain ||              // "text/plain"
-            t == fl_XaUtf8String) {             // "UTF8_STRING"
-          type = t;
-          break;
-        }
-        // rest are only used if no UTF-8 available:
-        if (t == fl_XaText ||                   // "TEXT"
-            t == fl_XaTextUriList ||            // "text/uri-list"
-            t == fl_XaCompoundText) type = t;   // "COMPOUND_TEXT"
       }
-      fl_dnd_type = type;
+      fl_dnd_type = find_target_text(fl_dnd_source_types, dnd_sources);
+      if (!fl_dnd_type) // not found: pick first type
+        fl_dnd_type = fl_dnd_source_types[0];
 
       event = FL_DND_ENTER;
       Fl::e_text = unknown;
@@ -1791,7 +1832,8 @@ int fl_handle(const XEvent& thisevent)
       return 1;
 
     }
-    break;}
+    break;
+  } // case ClientMessage
 
   case UnmapNotify:
     event = FL_HIDE;
@@ -2223,8 +2265,8 @@ int fl_handle(const XEvent& thisevent)
       window->position(rint(xpos/s), rint(ypos/s));
     }
     break;
-    }
+  } // ReparentNotify
-  }
+  } // if (window) switch (xevent.type)
 
 #if HAVE_XFIXES
   switch (xevent.type - xfixes_event_base) {

src/fl_dnd_x.cxx

@@ -1,7 +1,7 @@
 //
 // Drag & Drop code for the Fast Light Tool Kit (FLTK).
 //
-// Copyright 1998-2018 by Bill Spitzak and others.
+// Copyright 1998-2021 by Bill Spitzak and others.
 //
 // This library is free software. Distribution and use rights are outlined in
 // the file "COPYING" which should have been included with this file.  If this
@@ -21,7 +21,6 @@
 #include "drivers/X11/Fl_X11_Screen_Driver.H"
 #include "Fl_Window_Driver.H"
 
-
 extern Atom fl_XdndAware;
 extern Atom fl_XdndSelection;
 extern Atom fl_XdndEnter;
@@ -32,7 +31,6 @@ extern Atom fl_XdndDrop;
 extern Atom fl_XdndStatus;
 extern Atom fl_XdndActionCopy;
 extern Atom fl_XdndFinished;
-//extern Atom fl_XdndProxy;
 extern Atom fl_XdndURIList;
 extern Atom fl_XaUtf8String;
 
@@ -90,7 +88,6 @@ int Fl_X11_Screen_Driver::dnd(int unused) {
   XSetSelectionOwner(fl_display, fl_XdndSelection, fl_message_window, fl_event_time);
 
   while (Fl::pushed()) {
-
     // figure out what window we are pointing at:
     Window new_window = 0; int new_version = 0;
     Fl_Window* new_local_window = 0;
@@ -145,12 +142,12 @@ int Fl_X11_Screen_Driver::dnd(int unused) {
             !strchr(fl_selection_buffer[0], ' ') &&
             strstr(fl_selection_buffer[0], "\r\n")) {
           // Send file/URI list...
-          fl_sendClientMessage(target_window, fl_XdndEnter, source_window,
+          fl_sendClientMessage(target_window, fl_XdndEnter, source_window, dndversion<<24,
-                               dndversion<<24, fl_XdndURIList, XA_STRING, 0);
+                               fl_XdndURIList, fl_XaUtf8String, XA_STRING);
         } else {
           // Send plain text...
-          fl_sendClientMessage(target_window, fl_XdndEnter, source_window,
+          fl_sendClientMessage(target_window, fl_XdndEnter, source_window, dndversion<<24,
-                               dndversion<<24, fl_XaUtf8String, 0, 0);
+                               fl_XaUtf8String, XA_STRING, 0);
         }
       }
     }