From ff0543524b7b2df4931aaa4e81bdfa9fe82b76f0 Mon Sep 17 00:00:00 2001 From: Albrecht Schlosser Date: Fri, 29 Jan 2021 00:06:34 +0100 Subject: [PATCH] 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 7ce6d2cf5dfc0488ec30d9f9f1709be73353479c). Closes #182. --- CHANGES.txt | 2 + src/Fl_x.cxx | 250 +++++++++++++++++++++++++++-------------------- src/fl_dnd_x.cxx | 13 +-- 3 files changed, 153 insertions(+), 112 deletions(-) diff --git a/CHANGES.txt b/CHANGES.txt index 3ac67d96c..97f1da18d 100644 --- a/CHANGES.txt +++ b/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 diff --git a/src/Fl_x.cxx b/src/Fl_x.cxx index 3229a8154..c4c635125 100644 --- a/src/Fl_x.cxx +++ b/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; - int retval = 0; - if (strcmp(type, Fl::clipboard_plain_text) == 0) { - for (i = 0; ixselection.property) for (;;) { @@ -1447,64 +1518,43 @@ int fl_handle(const XEvent& thisevent) } if (actual == TARGETS || actual == XA_ATOM) { - /*for (unsigned i = 0; ihandle(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 { - // char* x = XGetAtomName(fl_display,e.target); - // fprintf(stderr,"selection request of %s\n",x); - // XFree(x); + } else { // no data available 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... - unsigned i; - Atom type = ((Atom*)fl_dnd_source_types)[0]; - for (i = 0; fl_dnd_source_types[i]; i ++) { - Atom t = ((Atom*)fl_dnd_source_types)[i]; - //printf("fl_dnd_source_types[%d]=%ld(%s)\n",i,t,XGetAtomName(fl_display,t)); - 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" + // Pick the "best" source (text) type... + // *FIXME* what if we don't find a suitable type? (see below: first type?) + // *FIXME* count (zero terminated) dnd sources (must be at least 1) + int dnd_sources; + for (dnd_sources = 0; fl_dnd_source_types[dnd_sources]; dnd_sources++) { + // empty } - 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) { diff --git a/src/fl_dnd_x.cxx b/src/fl_dnd_x.cxx index d8911c000..6ec34d286 100644 --- a/src/fl_dnd_x.cxx +++ b/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, - dndversion<<24, fl_XdndURIList, XA_STRING, 0); + fl_sendClientMessage(target_window, fl_XdndEnter, source_window, dndversion<<24, + fl_XdndURIList, fl_XaUtf8String, XA_STRING); } else { // Send plain text... - fl_sendClientMessage(target_window, fl_XdndEnter, source_window, - dndversion<<24, fl_XaUtf8String, 0, 0); + fl_sendClientMessage(target_window, fl_XdndEnter, source_window, dndversion<<24, + fl_XaUtf8String, XA_STRING, 0); } } }