Adds drag and drop functionality

[deeplow]

Fixes #409

Behaviors

1 valid file	2 valid files

1 invalid file	1 valid file + 1 invalid file

[deeplow]

I was thinking about about how to make the "drag and drop" functionality more discoverable and it hit me that we could just show both the drop area and the add button at the same time. On the draft mockups that we have, both the "drop area" and the button are visible at the same time.

mockup	option 1 (dynamic)	option 2 (static but more discoverable)

So @apyrgio if you agree with this approach, I's squash it. It ended up significantly simplifying the code as well, since it doesn't need any dynamic parts anymore.

[harrislapiroff]

That looks like a good UI solution to me 👍 It's looking good!

[deeplow]

Security note: mime handling

The following line raised security concerns for me: https://github.com/freedomofpress/dangerzone/blob/7bdb8dcdca7e3c68ad0df3fda3bcf0815d1b4c3c/dangerzone/gui/main_window.py#L685

I was afraid that it could be doing some sort of Mime type guessing by going into the suspicious files. So I went down the rabbit hole of checking if this affected us.

TL;DR there is no security concern as far as I can tell. The rest of this post is just the journey.

---

I started by looking up QMimeData to see what it was all about. It seems to be pretty powerful and able to handle a variety of situations for dropping and moving elements in general (text, files, UI elements, etc.). Complexity is the opposite of what we want.

Taking a look at some related source code I see cause for concern in this case switch:

    switch (mode) {
    case QMimeDatabase::MatchDefault:
        break;
    case QMimeDatabase::MatchExtension:
        return mimeTypeForFileExtension(fileName);
    case QMimeDatabase::MatchContent: {
        QFile file(fileName);
        return mimeTypeForData(&file);
    }

In particular the QMimeDatabase::MatchContent case, which implies that it's digging into the file itself. That that's indeed what the function does.

However, what disarmed the situation was me later realizing that the MIME data is for a list of files (text/uri-list) and not any particular file. So it's just a list of URLs for local files...

I got to this conclusion by inspecting the events. Dropping the code here in case it's ever useful or in case anyone wants to confirm this:

diff --git a/dangerzone/gui/main_window.py b/dangerzone/gui/main_window.py
index d9583543..acfa708b 100644
--- a/dangerzone/gui/main_window.py
+++ b/dangerzone/gui/main_window.py
@@ -631,6 +631,25 @@ class DocSelectionWidget(QtWidgets.QWidget):
             pass
 
 
+class DropEventFilter(QtCore.QObject):
+    def __init__(self, target_widget):
+        super().__init__(target_widget)
+        self.target_widget = target_widget
+        self.target_widget.installEventFilter(self)
+
+    def eventFilter(self, target: QtWidgets.QWidget, event: QtCore.QEvent) -> bool:
+        log.debug(f"EVENT FILTERED {event}")
+        if target is self.target_widget and event.type() == QtGui.QDropEvent:
+            dropEvent = QtGui.QDropEvent(event)
+            if dropEvent.key() == Qt.Key_Tab:
+                # Special tab handling
+                return True
+            else:
+                return False
+
+        return False
+
+
 class DocSelectionDropFrame(QtWidgets.QFrame):
     """
     HACK Docs selecting widget "drag-n-drop" border widget
@@ -650,6 +669,7 @@ class DocSelectionDropFrame(QtWidgets.QFrame):
 
         # Drag and drop functionality
         self.setAcceptDrops(True)
+        self.installEventFilter(DropEventFilter(self))
 
         self.document_image_text = QtWidgets.QLabel(
             "Drag and drop\n documents here\n\n or"

The output will look something like this:

[DEBUG] EVENT FILTERED <PySide6.QtGui.QDragMoveEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,99, answerRect=QRect(430,99 1x1), formats=QList("text/uri-list", "text/plain;charset=utf-8"), LeftButton>
[DEBUG] EVENT FILTERED <PySide6.QtGui.QDragMoveEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,99, answerRect=QRect(430,99 1x1), formats=QList("text/uri-list", "text/plain;charset=utf-8"), LeftButton>
[DEBUG] EVENT FILTERED <PySide6.QtGui.QDragMoveEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,98, answerRect=QRect(430,98 1x1), formats=QList("text/uri-list", "text/plain;charset=utf-8"), LeftButton>
[DEBUG] EVENT FILTERED <PySide6.QtGui.QDragMoveEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,97, answerRect=QRect(430,97 1x1), formats=QList("text/uri-list", "text/plain;charset=utf-8"), LeftButton>
[DEBUG] EVENT FILTERED <PySide6.QtGui.QDragMoveEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,96, answerRect=QRect(430,96 1x1), formats=QList("text/uri-list", "text/plain;charset=utf-8"), LeftButton>
[DEBUG] EVENT FILTERED <PySide6.QtGui.QDropEvent(dropAction=CopyAction, proposedAction=CopyAction, possibleActions=CopyAction|MoveAction|LinkAction, posF=430,96, formats=QList("text/uri-list", "text/plain;charset=utf-8"), NoButton>
[INFO] Assigning ID '4pxYgt' to doc '/home/user/dangerzone/tests/test_docs/sample-bmp.bmp'
[DEBUG] Removing all documents
[DEBUG] EVENT FILTERED <PySide6.QtCore.QEvent(QEvent::Hide)>
[DEBUG] EVENT FILTERED <PySide6.QtCore.QEvent(QEvent::HideToParent)>

[Erioldoesdesign]

I was thinking about about how to make the "drag and drop" functionality more discoverable and it hit me that we could just show both the drop area and the add button at the same time. On the draft mockups that we have, both the "drop area" and the button are visible at the same time.

mockup option 1 (dynamic) option 2 (static but more discoverable) So @apyrgio if you agree with this approach, I's squash it. It ended up significantly simplifying the code as well, since it doesn't need any dynamic parts anymore.

OOoooooooooo COOL!

[deeplow]

I just realized that this last iteration looks very much like the original dangerzone UX improvement suggestions. I had the feeling I had seen this somewhere. Now I know where:

[almet]

(Just rebased this on latest main)

[almet]

Added some mypy type checking, removed some checks that weren't accurate anymore. All tests are passing except on debian bookworkm

[almet]

I'm not sure if it's related to the CI or with my machine, but I can't reproduce locally the errors on this distribution right now. Retriggering the CI to see if the second take is better.

[apyrgio]

Thanks a lot Alexis, I'll let you know once I've taken a look.

[apyrgio]

You're good to go Alexis, thanks a lot. Let's add a QA scenario at some point for this feature.