TabModel: Rework drag&drop logic

Use QMimeData subclass to safely pass pointers.
Only allow to drop one tab at a time and disable drop action
when not possible. Allow to move tabs to other windows, with the
exception of tabs with children.

src/lib/tabwidget/tabmodel.cpp

@@ -21,8 +21,39 @@
 #include "tabbedwebview.h"
 #include "browserwindow.h"
 
-#include <QMimeData>
+// TabModelMimeData
+TabModelMimeData::TabModelMimeData()
+    : QMimeData()
+{
+}
 
+WebTab *TabModelMimeData::tab() const
+{
+    return m_tab;
+}
+
+void TabModelMimeData::setTab(WebTab *tab)
+{
+    m_tab = tab;
+}
+
+bool TabModelMimeData::hasFormat(const QString &format) const
+{
+    return mimeType() == format;
+}
+
+QStringList TabModelMimeData::formats() const
+{
+    return {mimeType()};
+}
+
+// static
+QString TabModelMimeData::mimeType()
+{
+    return QSL("application/falkon.tabmodel.tab");
+}
+
+// TabModel
 TabModel::TabModel(BrowserWindow *window, QObject *parent)
     : QAbstractListModel(parent)
     , m_window(window)
@@ -111,61 +142,74 @@ Qt::DropActions TabModel::supportedDropActions() const
     return Qt::MoveAction;
 }
 
-#define MIMETYPE QStringLiteral("application/falkon.tabmodel.tab")
-
 QStringList TabModel::mimeTypes() const
 {
-    return {MIMETYPE};
+    return {TabModelMimeData::mimeType()};
 }
 
 QMimeData *TabModel::mimeData(const QModelIndexList &indexes) const
 {
-    QByteArray data;
+    if (indexes.isEmpty()) {
-    QDataStream stream(&data, QIODevice::WriteOnly);
+        return nullptr;
-
-    for (const QModelIndex &index : indexes) {
-        if (index.isValid() && index.column() == 0) {
-            stream << index.row();
-        }
     }
-
+    WebTab *tab = indexes.at(0).data(WebTabRole).value<WebTab*>();
-    QMimeData *mimeData = new QMimeData();
+    if (!tab) {
-    mimeData->setData(MIMETYPE, data);
+        return nullptr;
+    }
+    TabModelMimeData *mimeData = new TabModelMimeData;
+    mimeData->setTab(tab);
     return mimeData;
 }
 
+bool TabModel::canDropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) const
+{
+    Q_UNUSED(row)
+    if (action != Qt::MoveAction || parent.isValid() || column > 0 || !m_window) {
+        return false;
+    }
+    const TabModelMimeData *mimeData = qobject_cast<const TabModelMimeData*>(data);
+    if (!mimeData) {
+        return false;
+    }
+    return mimeData->tab();
+}
+
 bool TabModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent)
 {
-    if (action == Qt::IgnoreAction) {
+    if (!canDropMimeData(data, action, row, column, parent)) {
-        return true;
-    }
-
-    if (!m_window || !data->hasFormat(MIMETYPE) || parent.isValid() || column != 0) {
         return false;
     }
 
-    QByteArray encodedData = data->data(MIMETYPE);
+    const TabModelMimeData *mimeData = static_cast<const TabModelMimeData*>(data);
-    QDataStream stream(&encodedData, QIODevice::ReadOnly);
+    WebTab *tab = mimeData->tab();
 
-    QVector<WebTab*> tabs;
+    if (tab->browserWindow() == m_window) {
-    while (!stream.atEnd()) {
+        if (tab->isPinned()) {
-        int idx;
+            if (row < 0) {
-        stream >> idx;
+                row = m_window->tabWidget()->pinnedTabsCount();
-        WebTab *t = tab(index(idx));
+            }
-        if (t) {
+            if (row > m_window->tabWidget()->pinnedTabsCount()) {
-            tabs.append(t);
+                tab->togglePinned();
+            }
+        } else {
+            if (row < 0) {
+                row = m_window->tabWidget()->count();
+            }
+            if (row < m_window->tabWidget()->pinnedTabsCount()) {
+                tab->togglePinned();
+                row++;
+            }
         }
-    }
+        tab->moveTab(row > mimeData->tab()->tabIndex() ? row - 1 : row);
-
+    } else {
-    if (tabs.isEmpty()) {
+        if (row < 0) {
-        return false;
+            row = m_window->tabCount();
-    }
+        }
-
+        if (tab->browserWindow()) {
-    for (int i = 0; i < tabs.count(); ++i) {
+            tab->browserWindow()->tabWidget()->detachTab(tab);
-        const int from = tabs.at(i)->tabIndex();
+        }
-        const int to = row >= from ? row - 1 : row++;
+        tab->setPinned(row < m_window->tabWidget()->pinnedTabsCount());
-        // FIXME: This switches order when moving > 2 non-contiguous indices
+        m_window->tabWidget()->insertView(row, tab, Qz::NT_SelectedTab);
-        m_window->tabWidget()->moveTab(from, to);
     }
 
     return true;

src/lib/tabwidget/tabmodel.h

@@ -17,6 +17,8 @@
 * ============================================================ */
 #pragma once
 
+#include <QPointer>
+#include <QMimeData>
 #include <QAbstractListModel>
 
 #include "qzcommon.h"
@@ -24,6 +26,25 @@
 class WebTab;
 class BrowserWindow;
 
+class FALKON_EXPORT TabModelMimeData : public QMimeData
+{
+    Q_OBJECT
+
+public:
+    explicit TabModelMimeData();
+
+    WebTab *tab() const;
+    void setTab(WebTab *tab);
+
+    bool hasFormat(const QString &format) const override;
+    QStringList formats() const override;
+
+    static QString mimeType();
+
+private:
+    QPointer<WebTab> m_tab;
+};
+
 class FALKON_EXPORT TabModel : public QAbstractListModel
 {
     Q_OBJECT
@@ -53,6 +74,7 @@ public:
     Qt::DropActions supportedDropActions() const override;
     QStringList mimeTypes() const override;
     QMimeData *mimeData(const QModelIndexList &indexes) const override;
+    bool canDropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) const override;
     bool dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) override;
 
 private:

src/lib/tabwidget/tabtreemodel.cpp

@@ -18,6 +18,7 @@
 #include "tabtreemodel.h"
 #include "tabmodel.h"
 #include "webtab.h"
+#include "tabwidget.h"
 #include "browserwindow.h"
 
 #include <QTimer>
@@ -29,7 +30,6 @@ public:
     explicit TabTreeModelItem(WebTab *tab = nullptr, const QModelIndex &index = QModelIndex());
     ~TabTreeModelItem();
 
-    bool isRoot() const;
     void setParent(TabTreeModelItem *item);
     void addChild(TabTreeModelItem *item, int index = -1);
 
@@ -52,11 +52,6 @@ TabTreeModelItem::~TabTreeModelItem()
     }
 }
 
-bool TabTreeModelItem::isRoot() const
-{
-    return !tab;
-}
-
 void TabTreeModelItem::setParent(TabTreeModelItem *item)
 {
     if (parent == item) {
@@ -84,8 +79,9 @@ void TabTreeModelItem::addChild(TabTreeModelItem *item, int index)
 
 TabTreeModel::TabTreeModel(BrowserWindow *window, QObject *parent)
     : QAbstractProxyModel(parent)
+    , m_window(window)
 {
-    connect(window, &BrowserWindow::aboutToClose, this, &TabTreeModel::syncTopLevelTabs);
+    connect(m_window, &BrowserWindow::aboutToClose, this, &TabTreeModel::syncTopLevelTabs);
 
     connect(this, &QAbstractProxyModel::sourceModelChanged, this, &TabTreeModel::init);
 }
@@ -186,44 +182,53 @@ QModelIndex TabTreeModel::mapToSource(const QModelIndex &proxyIndex) const
     return it->sourceIndex;
 }
 
+bool TabTreeModel::canDropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) const
+{
+    Q_UNUSED(row)
+    Q_UNUSED(parent)
+    if (action != Qt::MoveAction || column > 0 || !m_window) {
+        return false;
+    }
+    const TabModelMimeData *mimeData = qobject_cast<const TabModelMimeData*>(data);
+    if (!mimeData) {
+        return false;
+    }
+    WebTab *tab = mimeData->tab();
+    if (!tab) {
+        return false;
+    }
+    // This would require moving the entire tab tree
+    if (tab->browserWindow() != m_window && !tab->childTabs().isEmpty()) {
+        return false;
+    }
+    return true;
+}
+
 bool TabTreeModel::dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent)
 {
-    if (action == Qt::IgnoreAction) {
+    if (!canDropMimeData(data, action, row, column, parent)) {
-        return true;
-    }
-
-    const QString mimeType = mimeTypes().at(0);
-
-    if (!data->hasFormat(mimeType) || column > 0) {
         return false;
     }
 
-    QByteArray encodedData = data->data(mimeType);
+    const TabModelMimeData *mimeData = static_cast<const TabModelMimeData*>(data);
-    QDataStream stream(&encodedData, QIODevice::ReadOnly);
+    WebTab *tab = mimeData->tab();
 
-    QVector<WebTab*> tabs;
+    if (tab->isPinned()) {
-    while (!stream.atEnd()) {
+        tab->togglePinned();
-        int index;
+    }
-        stream >> index;
+
-        WebTab *tab = sourceModel()->index(index, 0).data(TabModel::WebTabRole).value<WebTab*>();
+    if (tab->browserWindow() != m_window) {
-        if (tab) {
+        if (tab->browserWindow()) {
-            tabs.append(tab);
+            tab->browserWindow()->tabWidget()->detachTab(tab);
+            m_window->tabWidget()->addView(tab, Qz::NT_SelectedTab);
         }
     }
 
-    if (tabs.isEmpty()) {
-        return false;
-    }
-
-    // Only support moving one tab
-    WebTab *tab = tabs.at(0);
-
     TabTreeModelItem *it = m_items.value(tab);
     TabTreeModelItem *parentItem = item(parent);
     if (!it || !parentItem) {
         return false;
     }
-
     if (!parentItem->tab) {
         tab->setParentTab(nullptr);
         if (row < 0) {

src/lib/tabwidget/tabtreemodel.h

@@ -49,6 +49,7 @@ public:
     QModelIndex mapFromSource(const QModelIndex &sourceIndex) const override;
     QModelIndex mapToSource(const QModelIndex &proxyIndex) const override;
 
+    bool canDropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) const override;
     bool dropMimeData(const QMimeData *data, Qt::DropAction action, int row, int column, const QModelIndex &parent) override;
 
 private:
@@ -67,6 +68,7 @@ private:
     void connectTab(WebTab *tab);
     void syncTopLevelTabs();
 
+    BrowserWindow *m_window;
     TabTreeModelItem *m_root = nullptr;
     QHash<WebTab*, TabTreeModelItem*> m_items;
 };

src/lib/webtab/webtab.cpp

@@ -199,6 +199,11 @@ WebTab::WebTab(QWidget *parent)
     });
 }
 
+BrowserWindow *WebTab::browserWindow() const
+{
+    return m_window;
+}
+
 TabbedWebView* WebTab::webView() const
 {
     return m_webView;

src/lib/webtab/webtab.h

@@ -67,6 +67,7 @@ public:
 
     explicit WebTab(QWidget *parent = nullptr);
 
+    BrowserWindow *browserWindow() const;
     TabbedWebView* webView() const;
     LocationBar* locationBar() const;
     TabIcon* tabIcon() const;