DragAndDropHelper::updateDropAction: use StatJob for remote URLs

When dragging onto tabs/Places from a remote URL, we don't process
the QDropEvent immediately, but start a StatJob and process the
event when it finishes.

Also, the result of the StatJob is cached for 30 seconds, to avoid
starting duplicate jobs.

diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp
index f80b94ea7cc49e43775da1a0d57ae1dfa82920b0..32e251f02bb22ab923edf8e29c2b876ba3427ee5 100644 (file)
--- a/src/dolphintabwidget.cpp
+++ b/src/dolphintabwidget.cpp
@@ -9,7 +9,6 @@
 #include "dolphin_generalsettings.h"
 #include "dolphintabbar.h"
 #include "dolphinviewcontainer.h"
 #include "dolphin_generalsettings.h"
 #include "dolphintabbar.h"
 #include "dolphinviewcontainer.h"

-#include "views/draganddrophelper.h"

 
 #include <KAcceleratorManager>
 #include <KConfigGroup>
 
 #include <KAcceleratorManager>
 #include <KConfigGroup>


@@ -26,6 +25,7 @@ DolphinTabWidget::DolphinTabWidget(DolphinNavigatorsWidgetAction *navigatorsWidg
     : QTabWidget(parent)
     , m_lastViewedTab(nullptr)
     , m_navigatorsWidget{navigatorsWidget}
     : QTabWidget(parent)
     , m_lastViewedTab(nullptr)
     , m_navigatorsWidget{navigatorsWidget}

+    , m_dragAndDropHelper{this}

 {
     KAcceleratorManager::setNoAccel(this);
 
 {
     KAcceleratorManager::setNoAccel(this);
 


@@ -394,7 +394,7 @@ void DolphinTabWidget::tabDragMoveEvent(int index, QDragMoveEvent *event)
 {
     if (index >= 0) {
         DolphinView *view = tabPageAt(index)->activeViewContainer()->view();
 {
     if (index >= 0) {
         DolphinView *view = tabPageAt(index)->activeViewContainer()->view();

-        DragAndDropHelper::updateDropAction(event, view->url());
+        m_dragAndDropHelper.updateDropAction(event, view->url());

     }
 }
 
     }
 }
 

diff --git a/src/dolphintabwidget.h b/src/dolphintabwidget.h
index a28a6bea1ff0941d78e265a28bec044c6febdede..a02c8fb841b6ce25cba94fafd42fd0e931ca9f11 100644 (file)
--- a/src/dolphintabwidget.h
+++ b/src/dolphintabwidget.h
@@ -9,6 +9,7 @@
 
 #include "dolphinnavigatorswidgetaction.h"
 #include "dolphintabpage.h"
 
 #include "dolphinnavigatorswidgetaction.h"
 #include "dolphintabpage.h"

+#include "views/draganddrophelper.h"

 
 #include <QTabWidget>
 #include <QUrl>
 
 #include <QTabWidget>
 #include <QUrl>


@@ -276,6 +277,8 @@ private:
      */
     const std::optional<const ViewIndex> viewShowingItem(const QUrl &item) const;
 
      */
     const std::optional<const ViewIndex> viewShowingItem(const QUrl &item) const;
 

+    DragAndDropHelper m_dragAndDropHelper;
+

 private:
     QPointer<DolphinTabPage> m_lastViewedTab;
     QPointer<DolphinNavigatorsWidgetAction> m_navigatorsWidget;
 private:
     QPointer<DolphinTabPage> m_lastViewedTab;
     QPointer<DolphinNavigatorsWidgetAction> m_navigatorsWidget;

diff --git a/src/panels/places/placespanel.cpp b/src/panels/places/placespanel.cpp
index ba3451bd5e603f353e38fa4ec01f5e6590cceaf0..eaf2642eb2cf3fb51904e5cc3f8e826c83d9cb46 100644 (file)
--- a/src/panels/places/placespanel.cpp
+++ b/src/panels/places/placespanel.cpp
@@ -15,7 +15,6 @@
 #include "dolphin_placespanelsettings.h"
 #include "dolphinplacesmodelsingleton.h"
 #include "settings/dolphinsettingsdialog.h"
 #include "dolphin_placespanelsettings.h"
 #include "dolphinplacesmodelsingleton.h"
 #include "settings/dolphinsettingsdialog.h"

-#include "views/draganddrophelper.h"

 
 #include <KFilePlacesModel>
 #include <KIO/DropJob>
 
 #include <KFilePlacesModel>
 #include <KIO/DropJob>


@@ -32,6 +31,7 @@
 
 PlacesPanel::PlacesPanel(QWidget *parent)
     : KFilePlacesView(parent)
 
 PlacesPanel::PlacesPanel(QWidget *parent)
     : KFilePlacesView(parent)

+    , m_dragAndDropHelper(this)

 {
     setDropOnPlaceEnabled(true);
     connect(this, &PlacesPanel::urlsDropped, this, &PlacesPanel::slotUrlsDropped);
 {
     setDropOnPlaceEnabled(true);
     connect(this, &PlacesPanel::urlsDropped, this, &PlacesPanel::slotUrlsDropped);


@@ -161,7 +161,7 @@ void PlacesPanel::dragMoveEvent(QDragMoveEvent *event)
             if (!url.isValid() || !KProtocolManager::supportsWriting(url)) {
                 event->setDropAction(Qt::IgnoreAction);
             } else {
             if (!url.isValid() || !KProtocolManager::supportsWriting(url)) {
                 event->setDropAction(Qt::IgnoreAction);
             } else {

-                DragAndDropHelper::updateDropAction(event, url);
+                m_dragAndDropHelper.updateDropAction(event, url);

             }
         }
     }
             }
         }
     }

diff --git a/src/panels/places/placespanel.h b/src/panels/places/placespanel.h
index cbd5fc224ce9763a5ba5fed97ad916dc7da47cbd..d21e7d64e50282b8744dd7f77cce9909fb94ece9 100644 (file)
--- a/src/panels/places/placespanel.h
+++ b/src/panels/places/placespanel.h
@@ -10,6 +10,7 @@
 #define PLACESPANEL_H
 
 #include "panels/panel.h"
 #define PLACESPANEL_H
 
 #include "panels/panel.h"

+#include "views/draganddrophelper.h"

 
 #include <KFilePlacesView>
 #include <QUrl>
 
 #include <KFilePlacesView>
 #include <QUrl>


@@ -78,6 +79,8 @@ private:
     QAction *m_configureTrashAction;
     QAction *m_openInSplitView;
     QAction *m_lockPanelsAction;
     QAction *m_configureTrashAction;
     QAction *m_openInSplitView;
     QAction *m_lockPanelsAction;

+
+    DragAndDropHelper m_dragAndDropHelper;

 };
 
 #endif // PLACESPANEL_H
 };
 
 #endif // PLACESPANEL_H

diff --git a/src/views/draganddrophelper.cpp b/src/views/draganddrophelper.cpp
index 7b9949df4db17408537aa9743a69504d61c95c14..7163507a67af07d68d65686681535d3918a5cf00 100644 (file)
--- a/src/views/draganddrophelper.cpp
+++ b/src/views/draganddrophelper.cpp
@@ -73,20 +73,85 @@ bool DragAndDropHelper::supportsDropping(const KFileItem &destItem)
     return (destItem.isDir() && destItem.isWritable()) || destItem.isDesktopFile();
 }
 
     return (destItem.isDir() && destItem.isWritable()) || destItem.isDesktopFile();
 }
 

+DragAndDropHelper::DragAndDropHelper(QObject *parent)
+    : QObject(parent)
+{
+    m_destItemCacheInvalidationTimer.setSingleShot(true);
+    m_destItemCacheInvalidationTimer.setInterval(30000);
+    connect(&m_destItemCacheInvalidationTimer, &QTimer::timeout, this, [this]() {
+        m_destItemCache = KFileItem();
+    });
+}
+

 void DragAndDropHelper::updateDropAction(QDropEvent *event, const QUrl &destUrl)
 {
 void DragAndDropHelper::updateDropAction(QDropEvent *event, const QUrl &destUrl)
 {

+    auto processEvent = [this](QDropEvent *event) {
+        if (supportsDropping(m_destItemCache)) {
+            event->setDropAction(event->proposedAction());
+            event->accept();
+        } else {
+            event->setDropAction(Qt::IgnoreAction);
+            event->ignore();
+        }
+    };
+
+    m_lastUndecidedEvent = nullptr;
+

     if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) {
         event->setDropAction(Qt::IgnoreAction);
         event->ignore();
     if (urlListMatchesUrl(event->mimeData()->urls(), destUrl)) {
         event->setDropAction(Qt::IgnoreAction);
         event->ignore();

+        return;

     }
     }

-    KFileItem item(destUrl);
-    if (!item.isLocalFile() || supportsDropping(item)) {
-        event->setDropAction(event->proposedAction());
-        event->accept();
-    } else {
-        event->setDropAction(Qt::IgnoreAction);
-        event->ignore();
+
+    if (destUrl == m_destItemCache.url()) {
+        // We already received events for this URL, and already have the
+        // stat result cached because:
+        // 1. it's a local file, and we already called KFileItem(destUrl)
+        // 2. it's a remote file, and StatJob finished
+        processEvent(event);
+        return;

     }
     }

+
+    if (m_statJob) {
+        if (destUrl == m_statJobUrl) {
+            // We already received events for this URL. Still waiting for
+            // the stat result. StatJob will process the event when it finishes.
+            m_lastUndecidedEvent = event;
+            return;
+        }
+
+        // We are waiting for the stat result of a different URL. Cancel.
+        m_statJob->kill();
+        m_statJob = nullptr;
+        m_statJobUrl.clear();
+    }
+
+    if (destUrl.isLocalFile()) {
+        // New local URL. KFileItem will stat on demand.
+        m_destItemCache = KFileItem(destUrl);
+        m_destItemCacheInvalidationTimer.start();
+        processEvent(event);
+        return;
+    }
+
+    // New remote URL. Start a StatJob and process the event when it finishes.
+    m_lastUndecidedEvent = event;
+    m_statJob = KIO::stat(destUrl, KIO::StatJob::SourceSide, KIO::StatDetail::StatBasic, KIO::JobFlag::HideProgressInfo);
+    m_statJobUrl = destUrl;
+    connect(m_statJob, &KIO::StatJob::result, this, [this, processEvent](KJob *job) {
+        KIO::StatJob *statJob = static_cast<KIO::StatJob *>(job);
+
+        m_destItemCache = KFileItem(statJob->statResult(), m_statJobUrl);
+        m_destItemCacheInvalidationTimer.start();
+
+        if (m_lastUndecidedEvent) {
+            processEvent(m_lastUndecidedEvent);
+            m_lastUndecidedEvent = nullptr;
+        }
+
+        m_statJob = nullptr;
+        m_statJobUrl.clear();
+    });

 }
 
 void DragAndDropHelper::clearUrlListMatchesUrlCache()
 }
 
 void DragAndDropHelper::clearUrlListMatchesUrlCache()

diff --git a/src/views/draganddrophelper.h b/src/views/draganddrophelper.h
index 73043febc2ee91d70766179197c732e16af4a3ee..df3b49966afb4baa0ac17253d6a646fcf7d6ff0b 100644 (file)
--- a/src/views/draganddrophelper.h
+++ b/src/views/draganddrophelper.h
@@ -11,9 +11,11 @@
 #include "dolphin_export.h"
 
 #include <KFileItem>
 #include "dolphin_export.h"
 
 #include <KFileItem>

+#include <KIO/StatJob>

 
 #include <QList>
 #include <QString>
 
 #include <QList>
 #include <QString>

+#include <QTimer>

 #include <QUrl>
 
 class QDropEvent;
 #include <QUrl>
 
 class QDropEvent;


@@ -24,7 +26,7 @@ namespace KIO
 class DropJob;
 }
 
 class DropJob;
 }
 

-class DOLPHIN_EXPORT DragAndDropHelper
+class DOLPHIN_EXPORT DragAndDropHelper : public QObject

 {
 public:
     /**
 {
 public:
     /**


@@ -51,16 +53,6 @@ public:
      */
     static bool supportsDropping(const KFileItem &destItem);
 
      */
     static bool supportsDropping(const KFileItem &destItem);
 

-    /**
-     * Updates the drop action according to whether the destination supports dropping.
-     * If supportsDropping(destUrl), set dropAction = proposedAction. Otherwise, set
-     * dropAction = Qt::IgnoreAction.
-     *
-     * @param event     Drop event.
-     * @param destUrl   Destination URL.
-     */
-    static void updateDropAction(QDropEvent *event, const QUrl &destUrl);
-

     /**
      * @return True if destUrl is contained in the urls parameter.
      */
     /**
      * @return True if destUrl is contained in the urls parameter.
      */


@@ -84,11 +76,55 @@ public:
      */
     static void clearUrlListMatchesUrlCache();
 
      */
     static void clearUrlListMatchesUrlCache();
 

+    DragAndDropHelper(QObject *parent);
+
+    /**
+     * Updates the drop action according to whether the destination supports dropping.
+     * If supportsDropping(destUrl), set dropAction = proposedAction. Otherwise, set
+     * dropAction = Qt::IgnoreAction.
+     *
+     * @param event     Drop event.
+     * @param destUrl   Destination URL.
+     */
+    void updateDropAction(QDropEvent *event, const QUrl &destUrl);
+

 private:
     /**
      * Stores the results of the expensive checks made in urlListMatchesUrl.
      */
     static QHash<QUrl, bool> m_urlListMatchesUrlCache;
 private:
     /**
      * Stores the results of the expensive checks made in urlListMatchesUrl.
      */
     static QHash<QUrl, bool> m_urlListMatchesUrlCache;

+
+    /**
+     * When updateDropAction() is called with a remote URL, we create a StatJob to
+     * check if the destination is a directory or a desktop file. We cache the result
+     * here to avoid doing the stat again on subsequent calls to updateDropAction().
+     */
+    KFileItem m_destItemCache;
+
+    /**
+     * Only keep the cache for 30 seconds, because the stat of the destUrl might change.
+     */
+    QTimer m_destItemCacheInvalidationTimer;
+
+    /**
+     * A StatJob on-fly to fill the cache for a remote URL. We shouldn't create more
+     * than one StatJob at a time, so we keep a pointer to the current one.
+     */
+    KIO::StatJob *m_statJob = nullptr;
+
+    /**
+     * The URL for which the StatJob is running.
+     * Note: We can't use m_statJob->url() because StatJob might resolve the URL to be
+     *       different from what we passed into stat(). E.g. "mtp:<bus-name>" is resolved
+     *       to "mtp:<phone name>"
+     */
+    QUrl m_statJobUrl;
+
+    /**
+     * The last event we received in updateDropAction(), but can't react to yet,
+     * because a StatJob is on-fly.
+     */
+    QDropEvent *m_lastUndecidedEvent = nullptr;

 };
 
 #endif
 };
 
 #endif