Commit aaebb510 authored by Eike Hein's avatar Eike Hein

Defer initial positions apply until listing is complete

Summary:
This fixes the infamous "desktop positions partially scramble on reboot"
bug that occurs when KDirLister completes listing in multiple model
transactions.

This also:
* Disallows moves and drops while listing, for extra safety.
* Cleans up wonky old defer-sometimes code that made little sense.
* Removes a cache for lastRow() that was never actually used.

BUG:354802

Reviewers: #plasma, davidedmundson, chinmoyr

Subscribers: plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D18598
parent c6bab929
......@@ -40,6 +40,7 @@ FocusScope {
property alias isRootView: gridView.isRootView
property alias currentIndex: gridView.currentIndex
property alias url: dir.url
property alias status: dir.status
property alias positions: positioner.positions
property alias errorString: dir.errorString
property alias dragging: dir.dragging
......
......@@ -29,6 +29,7 @@ import org.kde.draganddrop 2.0 as DragDrop
import org.kde.kquickcontrolsaddons 2.0 as KQuickControlsAddons
import org.kde.private.desktopcontainment.desktop 0.1 as Desktop
import org.kde.private.desktopcontainment.folder 0.1 as Folder
import "LayoutManager.js" as LayoutManager
import "FolderTools.js" as FolderTools
......@@ -250,6 +251,11 @@ FolderViewDropArea {
event.ignore();
}
// Don't allow any drops while listing.
if (isFolder && folderViewLayer.view.status === Folder.FolderModel.Listing) {
event.ignore();
}
// Firefox tabs are regular drags. Since all of our drop handling is asynchronous
// we would accept this drop and have Firefox not spawn a new window. (Bug 337711)
if (event.mimeData.formats.indexOf("application/x-moz-tabbrowser-tab") > -1) {
......
......@@ -29,9 +29,8 @@ Positioner::Positioner(QObject *parent): QAbstractItemModel(parent)
, m_enabled(false)
, m_folderModel(nullptr)
, m_perStripe(0)
, m_lastRow(-1)
, m_ignoreNextTransaction(false)
, m_pendingPositions(false)
, m_deferApplyPositions(false)
, m_updatePositionsTimer(new QTimer(this))
{
m_updatePositionsTimer->setSingleShot(true);
......@@ -84,7 +83,6 @@ void Positioner::setFolderModel(QObject *folderModel)
}
m_folderModel = qobject_cast<FolderModel *>(folderModel);
connect(m_folderModel, SIGNAL(urlChanged()), this, SLOT(reset()), Qt::UniqueConnection);
if (m_folderModel) {
connectSignals(m_folderModel);
......@@ -130,10 +128,11 @@ void Positioner::setPositions(const QStringList &positions)
emit positionsChanged();
if (!m_proxyToSource.isEmpty()) {
// Defer applying positions until listing completes.
if (m_folderModel->status() == FolderModel::Listing) {
m_deferApplyPositions = true;
} else {
applyPositions();
} else if (m_positions.size() >= 5) {
m_pendingPositions = true;
}
}
}
......@@ -359,6 +358,11 @@ void Positioner::reset()
}
void Positioner::move(const QVariantList &moves) {
// Don't allow moves while listing.
if (m_folderModel->status() == FolderModel::Listing) {
return;
}
QVector<int> fromIndices;
QVector<int> toIndices;
QVector<int> sourceRows;
......@@ -476,6 +480,13 @@ void Positioner::updatePositions()
}
}
void Positioner::sourceStatusChanged()
{
if (m_deferApplyPositions && m_folderModel->status() != FolderModel::Listing) {
applyPositions();
}
}
void Positioner::sourceDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
const QVector<int>& roles)
{
......@@ -512,13 +523,15 @@ void Positioner::sourceModelReset()
void Positioner::sourceRowsAboutToBeInserted(const QModelIndex &parent, int start, int end)
{
if (m_enabled) {
if (m_proxyToSource.isEmpty()) {
if (!m_pendingPositions) {
beginInsertRows(parent, start, end);
m_beginInsertRowsCalled = true;
// Don't insert yet if we're waiting for listing to complete to apply
// initial positions;
if (m_deferApplyPositions) {
return;
} else if (m_proxyToSource.isEmpty()) {
beginInsertRows(parent, start, end);
m_beginInsertRowsCalled = true;
initMaps(end + 1);
}
initMaps(end + 1);
return;
}
......@@ -608,7 +621,6 @@ void Positioner::sourceRowsAboutToBeRemoved(const QModelIndex &parent, int first
m_proxyToSource = newProxyToSource;
m_sourceToProxy = newSourceToProxy;
m_lastRow = -1;
int newLast = lastRow();
if (oldLast > newLast) {
......@@ -637,13 +649,9 @@ void Positioner::sourceRowsInserted(const QModelIndex &parent, int first, int la
Q_UNUSED(last)
if (!m_ignoreNextTransaction) {
if (!m_pendingPositions) {
if (m_beginInsertRowsCalled) {
endInsertRows();
m_beginInsertRowsCalled = false;
}
} else {
applyPositions();
if (m_beginInsertRowsCalled) {
endInsertRows();
m_beginInsertRowsCalled = false;
}
} else {
m_ignoreNextTransaction = false;
......@@ -651,7 +659,11 @@ void Positioner::sourceRowsInserted(const QModelIndex &parent, int first, int la
flushPendingChanges();
m_updatePositionsTimer->start();
// Don't generate new positions data if we're waiting for listing to
// complete to apply initial positions.
if (!m_deferApplyPositions) {
m_updatePositionsTimer->start();
}
}
void Positioner::sourceRowsMoved(const QModelIndex &sourceParent, int sourceStart,
......@@ -717,7 +729,6 @@ void Positioner::updateMaps(int proxyIndex, int sourceIndex)
{
m_proxyToSource.insert(proxyIndex, sourceIndex);
m_sourceToProxy.insert(sourceIndex, proxyIndex);
m_lastRow = -1;
}
int Positioner::firstRow() const
......@@ -735,13 +746,9 @@ int Positioner::firstRow() const
int Positioner::lastRow() const
{
if (!m_proxyToSource.isEmpty()) {
if (m_lastRow != -1) {
return m_lastRow;
} else {
QList<int> keys(m_proxyToSource.keys());
qSort(keys);
return keys.last();
}
QList<int> keys(m_proxyToSource.keys());
qSort(keys);
return keys.last();
}
return 0;
......@@ -764,7 +771,23 @@ int Positioner::firstFreeRow() const
void Positioner::applyPositions()
{
// We were called while the source model is listing. Defer applying positions
// until listing completes.
if (m_folderModel->status() == FolderModel::Listing) {
m_deferApplyPositions = true;
return;
}
if (m_positions.size() < 5) {
// We were waiting for listing to complete before proxying source rows,
// but we don't have positions to apply. Reset to populate.
if (m_deferApplyPositions) {
m_deferApplyPositions = false;
reset();
}
return;
}
......@@ -864,7 +887,7 @@ void Positioner::applyPositions()
endResetModel();
m_pendingPositions = false;
m_deferApplyPositions = false;
m_updatePositionsTimer->start();
}
......@@ -915,6 +938,12 @@ void Positioner::connectSignals(FolderModel* model)
connect(model, &QAbstractItemModel::layoutChanged,
this, &Positioner::sourceLayoutChanged,
Qt::UniqueConnection);
connect(m_folderModel, &FolderModel::urlChanged,
this, &Positioner::reset,
Qt::UniqueConnection);
connect(m_folderModel, &FolderModel::statusChanged,
this, &Positioner::sourceStatusChanged,
Qt::UniqueConnection);
}
void Positioner::disconnectSignals(FolderModel* model)
......@@ -937,4 +966,8 @@ void Positioner::disconnectSignals(FolderModel* model)
this, &Positioner::sourceRowsRemoved);
disconnect(model, &QAbstractItemModel::layoutChanged,
this, &Positioner::sourceLayoutChanged);
disconnect(m_folderModel, &FolderModel::urlChanged,
this, &Positioner::reset);
disconnect(m_folderModel, &FolderModel::statusChanged,
this, &Positioner::sourceStatusChanged);
}
......@@ -93,6 +93,7 @@ class FOLDERPLUGIN_TESTS_EXPORT Positioner : public QAbstractItemModel
private Q_SLOTS:
void updatePositions();
void sourceStatusChanged();
void sourceDataChanged(const QModelIndex &topLeft, const QModelIndex &bottomRight,
const QVector<int> &roles);
void sourceModelAboutToBeReset();
......@@ -132,7 +133,7 @@ class FOLDERPLUGIN_TESTS_EXPORT Positioner : public QAbstractItemModel
bool m_ignoreNextTransaction;
QStringList m_positions;
bool m_pendingPositions;
bool m_deferApplyPositions;
QTimer *m_updatePositionsTimer;
QHash<int, int> m_proxyToSource;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment