Reset the BufferViewFilter after setting a new config

I'm not sure why, but somehow BufferView crashes in rare occasions because of invalid modelindexes
if we don't reset the BufferModel after setting its config. This is even true if we haven't set
a source model before, so an attached View couldn't even get any invalid indexes in the first
place. Anyway, it stops Quassel from crashing here, so I shouldn't complain, I guess.

This hopefully fixes #663, the Heisenbug that has haunted us for a long time now. At least for me.

diff --git a/src/uisupport/bufferview.cpp b/src/uisupport/bufferview.cpp
index ecaf7e9..72b1ce3 100644 (file)
--- a/src/uisupport/bufferview.cpp
+++ b/src/uisupport/bufferview.cpp
@@ -71,9 +71,6 @@ void BufferView::init() {
 
   setAnimated(true);
 
 
   setAnimated(true);
 

-  // FIXME This is to workaround bug #663
-  setUniformRowHeights(true);
-

 #ifndef QT_NO_DRAGANDDROP
   setDragEnabled(true);
   setAcceptDrops(true);
 #ifndef QT_NO_DRAGANDDROP
   setDragEnabled(true);
   setAcceptDrops(true);


@@ -246,8 +243,8 @@ void BufferView::dropEvent(QDropEvent *event) {
     return QTreeView::dropEvent(event);
 
   int res = QMessageBox::question(0, tr("Merge buffers permanently?"),
     return QTreeView::dropEvent(event);
 
   int res = QMessageBox::question(0, tr("Merge buffers permanently?"),

-                                 tr("Do you want to merge the buffer \"%1\" permanently into buffer \"%2\"?\n This cannot be reversed!").arg(Client::networkModel()->bufferName(bufferId2)).arg(Client::networkModel()->bufferName(bufferId1)),
-                                 QMessageBox::Yes|QMessageBox::No, QMessageBox::No);
+                                  tr("Do you want to merge the buffer \"%1\" permanently into buffer \"%2\"?\n This cannot be reversed!").arg(Client::networkModel()->bufferName(bufferId2)).arg(Client::networkModel()->bufferName(bufferId1)),
+                                  QMessageBox::Yes|QMessageBox::No, QMessageBox::No);

   if(res == QMessageBox::Yes) {
     Client::mergeBuffersPermanently(bufferId1, bufferId2);
   }
   if(res == QMessageBox::Yes) {
     Client::mergeBuffersPermanently(bufferId1, bufferId2);
   }


@@ -408,7 +405,7 @@ void BufferView::addFilterActions(QMenu *contextMenu, const QModelIndex &index)
     if(!filterActions.isEmpty()) {
       contextMenu->addSeparator();
       foreach(QAction *action, filterActions) {
     if(!filterActions.isEmpty()) {
       contextMenu->addSeparator();
       foreach(QAction *action, filterActions) {

-       contextMenu->addAction(action);
+        contextMenu->addAction(action);

       }
     }
   }
       }
     }
   }


@@ -444,11 +441,11 @@ void BufferView::wheelEvent(QWheelEvent* event) {
         QModelIndex parent = currentIndex.parent();
         QModelIndex aunt = parent.sibling( parent.row() + rowDelta, parent.column() );
         if( rowDelta == -1 )
         QModelIndex parent = currentIndex.parent();
         QModelIndex aunt = parent.sibling( parent.row() + rowDelta, parent.column() );
         if( rowDelta == -1 )

-         resultingIndex = aunt.child( model()->rowCount( aunt ) - 1, 0 );
+          resultingIndex = aunt.child( model()->rowCount( aunt ) - 1, 0 );

         else
         else

-         resultingIndex = aunt.child( 0, 0 );
+          resultingIndex = aunt.child( 0, 0 );

         if( !resultingIndex.isValid() )
         if( !resultingIndex.isValid() )

-         return;
+          return;

       }
   selectionModel()->setCurrentIndex( resultingIndex, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows );
   selectionModel()->select( resultingIndex, QItemSelectionModel::ClearAndSelect );
       }
   selectionModel()->setCurrentIndex( resultingIndex, QItemSelectionModel::ClearAndSelect | QItemSelectionModel::Rows );
   selectionModel()->select( resultingIndex, QItemSelectionModel::ClearAndSelect );

diff --git a/src/uisupport/bufferviewfilter.cpp b/src/uisupport/bufferviewfilter.cpp
index 6bd6565..12109ba 100644 (file)
--- a/src/uisupport/bufferviewfilter.cpp
+++ b/src/uisupport/bufferviewfilter.cpp
@@ -46,6 +46,7 @@ public:
 BufferViewFilter::BufferViewFilter(QAbstractItemModel *model, BufferViewConfig *config)
   : QSortFilterProxyModel(model),
     _config(0),
 BufferViewFilter::BufferViewFilter(QAbstractItemModel *model, BufferViewConfig *config)
   : QSortFilterProxyModel(model),
     _config(0),

+    _tmpConfig(0),

     _sortOrder(Qt::AscendingOrder),
     _showServerQueries(false),
     _editMode(false),
     _sortOrder(Qt::AscendingOrder),
     _showServerQueries(false),
     _editMode(false),


@@ -56,8 +57,8 @@ BufferViewFilter::BufferViewFilter(QAbstractItemModel *model, BufferViewConfig *
 
   setDynamicSortFilter(true);
 
 
   setDynamicSortFilter(true);
 

-  connect(this, SIGNAL(_dataChanged(const QModelIndex &, const QModelIndex &)),
-         this, SLOT(_q_sourceDataChanged(QModelIndex,QModelIndex)));
+  connect(this, SIGNAL(_dataChanged(QModelIndex,QModelIndex)),
+          this, SLOT(_q_sourceDataChanged(QModelIndex,QModelIndex)));

 
   _enableEditMode.setCheckable(true);
   _enableEditMode.setChecked(_editMode);
 
   _enableEditMode.setCheckable(true);
   _enableEditMode.setChecked(_editMode);


@@ -72,51 +73,45 @@ void BufferViewFilter::setConfig(BufferViewConfig *config) {
   if(_config == config)
     return;
 
   if(_config == config)
     return;
 

-  if(_config) {
-    disconnect(_config, 0, this, 0);
-  }
+#if QT_VERSION >= 0x040600
+  beginResetModel();
+#endif

 
 

-  _config = config;
-
-  if(!config) {
-    invalidate();
-    setObjectName("");
-    return;
-  }
+  _tmpConfig = config;  // don't invalidate the old config before we're initialized

 
 

-  if(config->isInitialized()) {
+  if(!config || config->isInitialized()) {

     configInitialized();
   } else {
     // we use a queued connection here since manipulating the connection list of a sending object
     // doesn't seem to be such a good idea while executing a connected slots.
     connect(config, SIGNAL(initDone()), this, SLOT(configInitialized()), Qt::QueuedConnection);
     configInitialized();
   } else {
     // we use a queued connection here since manipulating the connection list of a sending object
     // doesn't seem to be such a good idea while executing a connected slots.
     connect(config, SIGNAL(initDone()), this, SLOT(configInitialized()), Qt::QueuedConnection);

-    invalidate();
+    //invalidate(); // not needed as we still have the old config and will reset once init is done

   }
 }
 
 void BufferViewFilter::configInitialized() {
   }
 }
 
 void BufferViewFilter::configInitialized() {

-  if(!config())
-    return;
-
-//   connect(config(), SIGNAL(bufferViewNameSet(const QString &)), this, SLOT(invalidate()));
-  connect(config(), SIGNAL(configChanged()), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(networkIdSet(const NetworkId &)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(addNewBuffersAutomaticallySet(bool)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(sortAlphabeticallySet(bool)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(hideInactiveBuffersSet(bool)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(allowedBufferTypesSet(int)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(minimumActivitySet(int)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(bufferListSet()), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(bufferAdded(const BufferId &, int)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(bufferMoved(const BufferId &, int)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(bufferRemoved(const BufferId &)), this, SLOT(invalidate()));
-//   connect(config(), SIGNAL(bufferPermanentlyRemoved(const BufferId &)), this, SLOT(invalidate()));
+  if(_config) {
+    disconnect(_config, 0, this, 0);
+  }

 
 

-  disconnect(config(), SIGNAL(initDone()), this, SLOT(configInitialized()));
+  _config = _tmpConfig;
+  _tmpConfig = 0;

 
 

-  setObjectName(config()->bufferViewName());
+  if(config()) {
+    connect(config(), SIGNAL(configChanged()), this, SLOT(invalidate()));
+    disconnect(config(), SIGNAL(initDone()), this, SLOT(configInitialized()));
+    setObjectName(config()->bufferViewName());
+  } else {
+    setObjectName("");
+  }

 
 

-  invalidate();
+  // not resetting the model can trigger bug #663 for some reason I haven't understood yet
+  // we get invalid model indexes in attached views even if no source model has been set yet... wtf?
+#if QT_VERSION >= 0x040600
+  endResetModel();
+#else
+  reset();
+#endif

   emit configChanged();
 }
 
   emit configChanged();
 }
 


@@ -151,12 +146,12 @@ void BufferViewFilter::enableEditMode(bool enable) {
     QSet<BufferId>::const_iterator iter;
     for(iter = _toTempRemove.constBegin(); iter != _toTempRemove.constEnd(); iter++) {
       if(config()->temporarilyRemovedBuffers().contains(*iter))
     QSet<BufferId>::const_iterator iter;
     for(iter = _toTempRemove.constBegin(); iter != _toTempRemove.constEnd(); iter++) {
       if(config()->temporarilyRemovedBuffers().contains(*iter))

-        continue;
+         continue;

       config()->requestRemoveBuffer(*iter);
     }
     for(iter = _toRemove.constBegin(); iter != _toRemove.constEnd(); iter++) {
       if(config()->removedBuffers().contains(*iter))
       config()->requestRemoveBuffer(*iter);
     }
     for(iter = _toRemove.constBegin(); iter != _toRemove.constEnd(); iter++) {
       if(config()->removedBuffers().contains(*iter))

-        continue;
+         continue;

       config()->requestRemoveBufferPermanently(*iter);
     }
   }
       config()->requestRemoveBufferPermanently(*iter);
     }
   }


@@ -185,7 +180,7 @@ Qt::ItemFlags BufferViewFilter::flags(const QModelIndex &index) const {
     if(bufferType != BufferInfo::QueryBuffer) {
       ClientBufferViewConfig *clientConf = qobject_cast<ClientBufferViewConfig *>(config());
       if(clientConf && clientConf->isLocked()) {
     if(bufferType != BufferInfo::QueryBuffer) {
       ClientBufferViewConfig *clientConf = qobject_cast<ClientBufferViewConfig *>(config());
       if(clientConf && clientConf->isLocked()) {

-       flags &= ~(Qt::ItemIsDropEnabled | Qt::ItemIsDragEnabled);
+        flags &= ~(Qt::ItemIsDropEnabled | Qt::ItemIsDragEnabled);

       }
     }
   }
       }
     }
   }


@@ -210,29 +205,29 @@ bool BufferViewFilter::dropMimeData(const QMimeData *data, Qt::DropAction action
     bufferId = bufferList[i].second;
     if(droppedNetworkId == networkId) {
       if(row < 0)
     bufferId = bufferList[i].second;
     if(droppedNetworkId == networkId) {
       if(row < 0)

-       row = 0;
+        row = 0;

 
       if(row < rowCount(parent)) {
 
       if(row < rowCount(parent)) {

-       QModelIndex source_child = mapToSource(index(row, 0, parent));
-       BufferId beforeBufferId = sourceModel()->data(source_child, NetworkModel::BufferIdRole).value<BufferId>();
-       pos = config()->bufferList().indexOf(beforeBufferId);
-       if(_sortOrder == Qt::DescendingOrder)
-         pos++;
+        QModelIndex source_child = mapToSource(index(row, 0, parent));
+        BufferId beforeBufferId = sourceModel()->data(source_child, NetworkModel::BufferIdRole).value<BufferId>();
+        pos = config()->bufferList().indexOf(beforeBufferId);
+        if(_sortOrder == Qt::DescendingOrder)
+          pos++;

       } else {
       } else {

-       if(_sortOrder == Qt::AscendingOrder)
-         pos = config()->bufferList().count();
-       else
-         pos = 0;
+        if(_sortOrder == Qt::AscendingOrder)
+          pos = config()->bufferList().count();
+        else
+          pos = 0;

       }
 
       if(config()->bufferList().contains(bufferId) && !config()->sortAlphabetically()) {
       }
 
       if(config()->bufferList().contains(bufferId) && !config()->sortAlphabetically()) {

-       if(config()->bufferList().indexOf(bufferId) < pos)
-         pos--;
-       ClientBufferViewConfig *clientConf = qobject_cast<ClientBufferViewConfig *>(config());
-       if(!clientConf || !clientConf->isLocked())
-         config()->requestMoveBuffer(bufferId, pos);
+        if(config()->bufferList().indexOf(bufferId) < pos)
+          pos--;
+        ClientBufferViewConfig *clientConf = qobject_cast<ClientBufferViewConfig *>(config());
+        if(!clientConf || !clientConf->isLocked())
+          config()->requestMoveBuffer(bufferId, pos);

       } else {
       } else {

-       config()->requestAddBuffer(bufferId, pos);
+        config()->requestAddBuffer(bufferId, pos);

       }
 
     } else {
       }
 
     } else {


@@ -280,14 +275,14 @@ void BufferViewFilter::addBuffers(const QList<BufferId> &bufferIds) const {
     bool lt;
     for(int i = 0; i < bufferList.count(); i++) {
       if(config() && config()->sortAlphabetically())
     bool lt;
     for(int i = 0; i < bufferList.count(); i++) {
       if(config() && config()->sortAlphabetically())

-       lt = bufferIdLessThan(bufferId, bufferList[i]);
+        lt = bufferIdLessThan(bufferId, bufferList[i]);

       else
       else

-       lt = bufferId < config()->bufferList()[i];
+        lt = bufferId < config()->bufferList()[i];

 
       if(lt) {
 
       if(lt) {

-       pos = i;
-       bufferList.insert(pos, bufferId);
-       break;
+        pos = i;
+        bufferList.insert(pos, bufferId);
+        break;

       }
     }
     config()->requestAddBuffer(bufferId, pos);
       }
     }
     config()->requestAddBuffer(bufferId, pos);


@@ -309,7 +304,7 @@ bool BufferViewFilter::filterAcceptBuffer(const QModelIndex &source_bufferIndex)
     if(config()->isInitialized()
        && !config()->removedBuffers().contains(bufferId) // it hasn't been manually removed and either
        && ((config()->addNewBuffersAutomatically() && !config()->temporarilyRemovedBuffers().contains(bufferId)) // is totally unknown to us (a new buffer)...
     if(config()->isInitialized()
        && !config()->removedBuffers().contains(bufferId) // it hasn't been manually removed and either
        && ((config()->addNewBuffersAutomatically() && !config()->temporarilyRemovedBuffers().contains(bufferId)) // is totally unknown to us (a new buffer)...

-          || (config()->temporarilyRemovedBuffers().contains(bufferId) && activityLevel > BufferInfo::OtherActivity))) { // or was just temporarily hidden and has a new message waiting for us.
+           || (config()->temporarilyRemovedBuffers().contains(bufferId) && activityLevel > BufferInfo::OtherActivity))) { // or was just temporarily hidden and has a new message waiting for us.

       addBuffer(bufferId);
     }
     // note: adding the buffer to the valid list does not temper with the following filters ("show only channels" and stuff)
       addBuffer(bufferId);
     }
     // note: adding the buffer to the valid list does not temper with the following filters ("show only channels" and stuff)

diff --git a/src/uisupport/bufferviewfilter.h b/src/uisupport/bufferviewfilter.h
index 01812b9..d1483d3 100644 (file)
--- a/src/uisupport/bufferviewfilter.h
+++ b/src/uisupport/bufferviewfilter.h
@@ -90,7 +90,7 @@ private slots:
   void showServerQueriesChanged();
 
 private:
   void showServerQueriesChanged();
 
 private:

-  QPointer<BufferViewConfig> _config;
+  QPointer<BufferViewConfig> _config, _tmpConfig;

   Qt::SortOrder _sortOrder;
 
   bool _showServerQueries;
   Qt::SortOrder _sortOrder;
 
   bool _showServerQueries;