Revert "DragAndDropHelper::updateDropAction: use StatJob for remote URLs"

This reverts commit dc149ec5e52f52c514cf362603d05ba8eea506b8.

This prevents a crash. One issue identified is that the commit that
I am reverting here accesses a QDropEvent at a moment in time in
which it might have already been deleted. We cannot check if it
exists by that time because we do not control its lifetime and it
is not a QObject.

diff --git a/src/dolphintabwidget.cpp b/src/dolphintabwidget.cpp
index 9bfd6076ae132bfac209988235f1831c2146a055..f80b94ea7cc49e43775da1a0d57ae1dfa82920b0 100644 (file)
--- a/src/dolphintabwidget.cpp
+++ b/src/dolphintabwidget.cpp
@@ -9,6 +9,7 @@
 #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>


@@ -23,7 +24,6 @@
 
 DolphinTabWidget::DolphinTabWidget(DolphinNavigatorsWidgetAction *navigatorsWidget, QWidget *parent)
     : QTabWidget(parent)
 
 DolphinTabWidget::DolphinTabWidget(DolphinNavigatorsWidgetAction *navigatorsWidget, QWidget *parent)
     : QTabWidget(parent)

-    , m_dragAndDropHelper{this}

     , m_lastViewedTab(nullptr)
     , m_navigatorsWidget{navigatorsWidget}
 {
     , m_lastViewedTab(nullptr)
     , m_navigatorsWidget{navigatorsWidget}
 {


@@ -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();

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

     }
 }
 
     }
 }
 

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

-#include "views/draganddrophelper.h"

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


@@ -277,8 +276,6 @@ 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 eaf2642eb2cf3fb51904e5cc3f8e826c83d9cb46..ba3451bd5e603f353e38fa4ec01f5e6590cceaf0 100644 (file)
--- a/src/panels/places/placespanel.cpp
+++ b/src/panels/places/placespanel.cpp
@@ -15,6 +15,7 @@
 #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>


@@ -31,7 +32,6 @@
 
 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 {

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

             }
         }
     }
             }
         }
     }

diff --git a/src/panels/places/placespanel.h b/src/panels/places/placespanel.h
index d21e7d64e50282b8744dd7f77cce9909fb94ece9..cbd5fc224ce9763a5ba5fed97ad916dc7da47cbd 100644 (file)
--- a/src/panels/places/placespanel.h
+++ b/src/panels/places/placespanel.h
@@ -10,7 +10,6 @@
 #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>


@@ -79,8 +78,6 @@ 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 7163507a67af07d68d65686681535d3918a5cf00..7b9949df4db17408537aa9743a69504d61c95c14 100644 (file)
--- a/src/views/draganddrophelper.cpp
+++ b/src/views/draganddrophelper.cpp
@@ -73,85 +73,20 @@ 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;

     }
     }

-
-    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;
+    KFileItem item(destUrl);
+    if (!item.isLocalFile() || supportsDropping(item)) {
+        event->setDropAction(event->proposedAction());
+        event->accept();
+    } else {
+        event->setDropAction(Qt::IgnoreAction);
+        event->ignore();

     }
     }

-
-    // 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 df3b49966afb4baa0ac17253d6a646fcf7d6ff0b..73043febc2ee91d70766179197c732e16af4a3ee 100644 (file)
--- a/src/views/draganddrophelper.h
+++ b/src/views/draganddrophelper.h
@@ -11,11 +11,9 @@
 #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;


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

-class DOLPHIN_EXPORT DragAndDropHelper : public QObject
+class DOLPHIN_EXPORT DragAndDropHelper

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


@@ -53,6 +51,16 @@ 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.
      */


@@ -76,55 +84,11 @@ 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