Prevent folders from drag and dropping onto themselves in dolphin main view

Summary:
This patch aims to improve user experience by not allowing the user to drag and drop a folder into itself.

The current behavior shows a red message at the top which can then be closed by the user, instead of relying on that, this patch disables the option of dropping onto self and uses the "Invalid drop target cursor" to highlight the behavior.

BUG: 307747

Since spectacle is unable to screenshot the cursor overlay, find attached a photo of the screen.
{F3787651}

Test Plan:
1. Drag a folder.
2. Drop it onto itself.

Reviewers: #dolphin, elvisangelaccio, ngraham, rkflx, dfaure

Reviewed By: #dolphin, elvisangelaccio, rkflx, dfaure

Subscribers: rkflx, ngraham, elvisangelaccio, dfaure, anthonyfieroni, #konqueror

Tags: #dolphin

Differential Revision: https://phabricator.kde.org/D6281

src/kitemviews/kfileitemmodel.h

@@ -73,7 +73,7 @@ public:
      *         the root-parent of all items.
      * @see rootItem()
      */
-    QUrl directory() const;
+    QUrl directory() const Q_DECL_OVERRIDE;
 
     /**
      * Cancels the loading of a directory which has been started by either

src/kitemviews/kitemlistcontroller.cpp

@@ -39,6 +39,7 @@
 #include <QMimeData>
 #include <QTimer>
 #include <QAccessible>
+#include <views/draganddrophelper.h>
 
 KItemListController::KItemListController(KItemModelBase* model, KItemListView* view, QObject* parent) :
     QObject(parent),
@@ -842,6 +843,7 @@ bool KItemListController::dragLeaveEvent(QGraphicsSceneDragDropEvent* event, con
     Q_UNUSED(event);
     Q_UNUSED(transform);
 
+    m_autoActivationTimer->stop();
     m_view->setAutoScroll(false);
     m_view->hideDropIndicator();
 
@@ -859,8 +861,8 @@ bool KItemListController::dragMoveEvent(QGraphicsSceneDragDropEvent* event, cons
         return false;
     }
 
-    event->acceptProposedAction();
 
+    QUrl hoveredDir = m_model->directory();
     KItemListWidget* oldHoveredWidget = hoveredWidget();
 
     const QPointF pos = transform.map(event->pos());
@@ -883,6 +885,11 @@ bool KItemListController::dragMoveEvent(QGraphicsSceneDragDropEvent* event, cons
         }
 
         const int index = newHoveredWidget->index();
+
+        if (m_model->isDir(index)) {
+            hoveredDir = m_model->url(index);
+        }
+
         if (!droppingBetweenItems) {
             if (m_model->supportsDropping(index)) {
                 // Something has been dragged on an item.
@@ -908,6 +915,8 @@ bool KItemListController::dragMoveEvent(QGraphicsSceneDragDropEvent* event, cons
         m_view->hideDropIndicator();
     }
 
+    event->setAccepted(!DragAndDropHelper::urlListMatchesUrl(event->mimeData()->urls(), hoveredDir));
+
     return false;
 }
 

src/kitemviews/kitemmodelbase.cpp

@@ -164,3 +164,17 @@ void KItemModelBase::onSortOrderChanged(Qt::SortOrder current, Qt::SortOrder pre
     Q_UNUSED(previous);
 }
 
+QUrl KItemModelBase::url(int index) const
+{
+    return data(index).value("url").toUrl();
+}
+
+bool KItemModelBase::isDir(int index) const
+{
+    return data(index).value("isDir").toBool();
+}
+
+QUrl KItemModelBase::directory() const
+{
+    return QUrl();
+}

src/kitemviews/kitemmodelbase.h

@@ -182,6 +182,20 @@ public:
      */
     QString blacklistItemDropEventMimeType() const;
 
+    /**
+     * @return URL of the item at the specified index
+     */
+    virtual QUrl url(int index) const;
+
+    /**
+     * @return True, if item at specified index is a directory
+     */
+    virtual bool isDir(int index) const;
+
+    /**
+     * @return Parent directory of the items that are shown
+     */
+    virtual QUrl directory() const;
 signals:
     /**
      * Is emitted if one or more items have been inserted. Each item-range consists

src/panels/places/placesitemmodel.cpp

@@ -1159,6 +1159,12 @@ QString PlacesItemModel::timelineDateString(int year, int month, int day)
     return date;
 }
 
+bool PlacesItemModel::isDir(int index) const
+{
+    Q_UNUSED(index);
+    return true;
+}
+
 QUrl PlacesItemModel::createSearchUrl(const QUrl& url)
 {
     QUrl searchUrl;

src/panels/places/placesitemmodel.h

@@ -132,6 +132,7 @@ public:
      */
     void saveBookmarks();
 
+    bool isDir(int index) const Q_DECL_OVERRIDE;
 signals:
     void errorMessage(const QString& message);
     void storageSetupDone(int index, bool success);

src/tests/CMakeLists.txt

@@ -57,3 +57,5 @@ LINK_LIBRARIES dolphinprivate dolphinstatic Qt5::Test)
 ecm_add_test(dolphinmainwindowtest.cpp
 TEST_NAME dolphinmainwindowtest
 LINK_LIBRARIES dolphinprivate dolphinstatic Qt5::Test)
+
+ecm_add_test(draganddrophelpertest.cpp LINK_LIBRARIES dolphinprivate Qt5::Test)

src/tests/draganddrophelpertest.cpp

@@ -0,0 +1,81 @@
+/***************************************************************************
+ *   Copyright (C) 2017 by Emirald Mateli <aldo.mateli@gmail.com>          *
+ *                                                                         *
+ *   This program is free software; you can redistribute it and/or modify  *
+ *   it under the terms of the GNU General Public License as published by  *
+ *   the Free Software Foundation; either version 2 of the License, or     *
+ *   (at your option) any later version.                                   *
+ *                                                                         *
+ *   This program is distributed in the hope that it will be useful,       *
+ *   but WITHOUT ANY WARRANTY; without even the implied warranty of        *
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the         *
+ *   GNU General Public License for more details.                          *
+ *                                                                         *
+ *   You should have received a copy of the GNU General Public License     *
+ *   along with this program; if not, write to the                         *
+ *   Free Software Foundation, Inc.,                                       *
+ *   51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA            *
+ ***************************************************************************/
+
+#include <QTest>
+#include <views/draganddrophelper.h>
+
+class DragAndDropHelperTest : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testUrlListMatchesUrl_data();
+    void testUrlListMatchesUrl();
+};
+
+void DragAndDropHelperTest::testUrlListMatchesUrl_data()
+{
+    QTest::addColumn<QList<QUrl>>("urlList");
+    QTest::addColumn<QUrl>("url");
+    QTest::addColumn<bool>("expected");
+
+    QTest::newRow("test_equal")
+            << QList<QUrl> {QUrl::fromLocalFile("/root")}
+            << QUrl::fromLocalFile("/root")
+            << true;
+
+    QTest::newRow("test_trailing_slash")
+            << QList<QUrl> {QUrl::fromLocalFile("/root/")}
+            << QUrl::fromLocalFile("/root")
+            << true;
+
+    QTest::newRow("test_ftp_scheme")
+            << QList<QUrl> {QUrl("ftp://server:2211/dir")}
+            << QUrl("ftp://server:2211/dir")
+            << true;
+
+    QTest::newRow("test_not_matched")
+            << QList<QUrl> {QUrl::fromLocalFile("/usr/share"), QUrl::fromLocalFile("/usr/local/bin")}
+            << QUrl::fromLocalFile("/usr/bin")
+            << false;
+
+    QTest::newRow("test_empty_target")
+            << QList<QUrl> {QUrl::fromLocalFile("/usr/share"), QUrl::fromLocalFile("/usr/local/bin")}
+            << QUrl()
+            << false;
+
+    QTest::newRow("test_empty_list")
+            << QList<QUrl>()
+            << QUrl::fromLocalFile("/usr/bin")
+            << false;
+}
+
+void DragAndDropHelperTest::testUrlListMatchesUrl()
+{
+    QFETCH(QList<QUrl>, urlList);
+    QFETCH(QUrl, url);
+    QFETCH(bool, expected);
+
+    QCOMPARE(DragAndDropHelper::urlListMatchesUrl(urlList, url), expected);
+}
+
+
+QTEST_MAIN(DragAndDropHelperTest)
+
+#include "draganddrophelpertest.moc"

src/views/draganddrophelper.cpp

@@ -29,6 +29,14 @@
 #include <KIO/DropJob>
 #include <KJobWidgets>
 
+
+bool DragAndDropHelper::urlListMatchesUrl(const QList<QUrl>& urls, const QUrl& destUrl)
+{
+    return std::find_if(urls.constBegin(), urls.constEnd(), [destUrl](const QUrl& url) {
+        return url.matches(destUrl, QUrl::StripTrailingSlash);
+    }) != urls.constEnd();
+}
+
 KIO::DropJob* DragAndDropHelper::dropUrls(const QUrl& destUrl, QDropEvent* event, QWidget* window)
 {
     const QMimeData* mimeData = event->mimeData();
@@ -42,6 +50,10 @@ KIO::DropJob* DragAndDropHelper::dropUrls(const QUrl& destUrl, QDropEvent* event
         message.setArguments({destUrl.toDisplayString(QUrl::PreferLocalFile)});
         QDBusConnection::sessionBus().call(message);
     } else {
+        if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) {
+            return nullptr;
+        }
+
         // Drop into a directory or a desktop-file
         KIO::DropJob *job = KIO::drop(event, destUrl);
         KJobWidgets::setWindow(job, window);

src/views/draganddrophelper.h

@@ -22,9 +22,10 @@
 #define DRAGANDDROPHELPER_H
 
 #include "dolphin_export.h"
+#include <QList>
+#include <QUrl>
 
 
-class QUrl;
 class QDropEvent;
 class QWidget;
 namespace KIO { class DropJob; }
@@ -42,11 +43,17 @@ public:
      *                  is true.
      * @param event     Drop event.
      * @param window    Widget where the drop happened, will be used as parent of the drop menu.
-     * @return          KIO::DropJob pointer
+     * @return          KIO::DropJob pointer or null in case the destUrl is contained
+     *                  in the mimeData url list.
      */
     static KIO::DropJob* dropUrls(const QUrl& destUrl,
                                   QDropEvent* event,
                                   QWidget *window);
+
+    /**
+     * @return True if destUrl is contained in the urls parameter.
+     */
+    static bool urlListMatchesUrl(const QList<QUrl>& urls, const QUrl& destUrl);
 };
 
 #endif