Try2: Fix Journal ListView DnD, fixes #4853

[samdroid-apps]

Replaces #525

Diff vs previous patch:

--- a/src/jarabe/journal/listmodel.py
+++ b/src/jarabe/journal/listmodel.py
@@ -255,7 +255,7 @@ class ListModel(GObject.GObject, Gtk.TreeModel, Gtk.TreeDragSource):
         target_name = target_atom.name()
         if target_name == 'text/uri-list':
             # Get hold of a reference so the temp file doesn't get deleted
-            self._temp_drag_file_path = model.get_file(uid) or ''
+            self._temp_drag_file_path = model.get_file(uid)
             logging.debug('putting %r in selection', self._temp_drag_file_path)
             selection.set(target_atom, 8, self._temp_drag_file_path)
             return True
diff --git a/src/jarabe/journal/listview.py b/src/jarabe/journal/listview.py
index d7e8c14..2949c13 100644
--- a/src/jarabe/journal/listview.py
+++ b/src/jarabe/journal/listview.py
@@ -707,7 +707,8 @@ class ListView(BaseListView):
             self._drag_coords = (event.x, event.y)

     def __drag_data_get_cb(self, widget, context, selection, info, time):
-        # Gtk.TreeDragSource does not work for us on Gtk 3.16+
+        # HACK:  Gtk.TreeDragSource does not work for us on Gtk 3.16+, so
+        #        use our drag source code instead
         if self._drag_coords is None:
             logging.warning('ListView drag-data-get without self._drag_coords')
             return

The GtkTreeDragSource is not working for us, without changes in the
docs. This commit uses the ListView as a proxy for the old methods.

Ticket URL http://bugs.sugarlabs.org/ticket/4853

[samdroid-apps]

I feel bad sending patches that are just hacks, but whatever has happened with GtkTreeDragSource was a very weird edge case. The drag context values were being set 100% correctly (by the tree model), but it just gets lost somewhere :(

[godiard]

@samdroid-apps,
There are a way to not feel bad:

• Create a simple test case to verify the issue is in Gtk and not in Sugar.
• Fill a bug in Gtk bug tracker and provide your test case.
• Add a reference in our ticket to the upstream bug.

Profit!

[samdroid-apps]

• Create a simple test case to verify the issue is in Gtk and not in Sugar.

Adapted from the tutorial on DnD. It should print "Received text: ..." but it does not.

from gi.repository import Gtk, Gdk, GObject

DRAG_ACTION = Gdk.DragAction.COPY

class DragDropWindow(Gtk.Window):

    def __init__(self):
        Gtk.Window.__init__(self, title='DnD TreeView Error')

        vbox = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=6)
        self.add(vbox)

        hbox = Gtk.Box(spacing=12)
        vbox.pack_start(hbox, True, True, 0)

        self.iconview = DragSourceIconView()
        self.drop_area = DropArea()

        hbox.pack_start(self.iconview, True, True, 0)
        hbox.pack_start(self.drop_area, True, True, 0)

        self.drop_area.drag_dest_add_text_targets()
        self.iconview.drag_source_add_text_targets()

class DragSourceIconView(Gtk.TreeView):

    def __init__(self):
        Gtk.TreeView.__init__(self)

        renderer = Gtk.CellRendererText()
        column = Gtk.TreeViewColumn("Text", renderer, text=0)
        self.append_column(column)

        model = ListModel()
        self.set_model(model)
        self.enable_model_drag_source(Gdk.ModifierType.BUTTON1_MASK, [],
            DRAG_ACTION)

class ListModel(Gtk.ListStore, Gtk.TreeDragSource):

    def __init__(self):
        GObject.GObject.__init__(self)
        Gtk.ListStore.__init__(self, str)

        self.add_item("Item 1")
        self.add_item("Item 2")
        self.add_item("Item 3")

    def add_item(self, text):
        self.append([text])

    def do_drag_data_get(self, path, selection):
        text = self[path][0]
        print 'Putting "{}" in drop selection'.format(text)
        selection.set_text(text, -1)
        print 'Set selection to "{}", hope it gets there!'.format(selection.get_text())


class DropArea(Gtk.Label):

    def __init__(self):
        Gtk.Label.__init__(self,
            "Drop something on me!\nI'll print to console.")
        self.drag_dest_set(Gtk.DestDefaults.ALL, [], DRAG_ACTION)
        self.connect("drag-data-received", self.on_drag_data_received)

    def on_drag_data_received(self, widget, drag_context, x,y, data,info, time):
        text = data.get_text()
        print("Received text: %s" % text)

win = DragDropWindow()
win.connect("delete-event", Gtk.main_quit)
win.show_all()
Gtk.main()

[samdroid-apps]

Added upstream ticket on BGO: https://bugzilla.gnome.org/show_bug.cgi?id=750443

[godiard]

I was testing this patch and found two problems:

1. I think is better do not have the motion_notify_cb attached all the time. This means every time the mouse move that code is executed.
   Proposal:
   connect 'motion-notify-event' on __drag_begin_cb and disconnect it the first time that is executed.

diff --git a/src/jarabe/journal/listview.py b/src/jarabe/journal/listview.py
index 2949c13..9ccfb90 100644
--- a/src/jarabe/journal/listview.py
+++ b/src/jarabe/journal/listview.py
@@ -672,16 +672,13 @@ class ListView(BaseListView):
         BaseListView.__init__(self, journalactivity, enable_multi_operations)
         self._is_dragging = False
         self._drag_coords = None
+        self._motion_id = None

         self.tree_view.connect('drag-begin', self.__drag_begin_cb)
         self.tree_view.connect('drag-data-get', self.__drag_data_get_cb)
         self.tree_view.connect('button-release-event',
                                self.__button_release_event_cb)

-        # We need the mouse position before the drag starts, therefore
-        # this must be running all of the time
-        self.connect('motion-notify-event', self.__motion_notify_event)
-
         self.cell_title.connect('edited', self.__cell_title_edited_cb)
         self.cell_title.connect('editing-canceled', self.__editing_canceled_cb)

@@ -701,10 +698,15 @@ class ListView(BaseListView):

     def __drag_begin_cb(self, widget, drag_context):
         self._is_dragging = True
+        # We need the mouse position before the drag starts, therefore
+        # this must be running all of the time
+        self._motion_id = self.connect('motion-notify-event',
+                                        self.__motion_notify_event)

     def __motion_notify_event(self, widget, event):
-        if not self._is_dragging:
-            self._drag_coords = (event.x, event.y)
+        self._drag_coords = (event.x, event.y)
+        self.disconnect(self._motion_id)
+        self._motion_id = None

     def __drag_data_get_cb(self, widget, context, selection, info, time):
         # HACK:  Gtk.TreeDragSource does not work for us on Gtk 3.16+, so

2. When drag a file to the clipboard, the log show a error saying the file does not exist. A icon is created, but do not have palette. This is probably a different problem, but is related to dnd.

[samdroid-apps]

1. But then we get the mouse position too late
2. IDK

[godiard]

@samdroid-apps ,
1, Really? So late that is in another treeview cell?

[samdroid-apps]

I tried it initially but that is what happened for me

[godiard]

@samdroid-apps, ok, i need test in slower hardware. In my desktop worked ok.

[godiard]

@samdroid-apps, I found a even simpler solution to this problem, and do not require a motion-notifi callback method.
Please test #540

[samdroid-apps]

Other patch was merged. Closing