Indeed. We have seen this problem while doing patch e8d7d2172627dd3c

But we decided not to fix it as we were afraid to change behavior.

Why do you need this patch?

Sorry, this is a long comment. The executive summary is that current behaviour expects proxy models to clear all internal data on sourceAboutToBeReset, but it should also be reasonable to clear it in sourceReset.

I discovered this while writing unit tests for the kselectionproxymodel on top of a qsortfilterproxymodel and hitting the

Q_ASSERT(srcPersistentIndex.isValid());

in sourceLayoutAboutToBeChanged in my unit tests:
http://websvn.kde.org/?view=revision&revision=1067548

The proxy index is valid in that method, and storing an invalid index for it in sourceLayoutAboutToBeChanged which is recalled and used in sourceLayoutChanged would cause the persistent index of the proxy to be “updated” falsely to the invalid index. Therefore in layoutAboutToBeChanged, the existing persistent indexes are required to always be valid.

QSortFilterProxyModel does not hit this bug because internal structures are cleared in the q_sourceAboutToBeReset slot instead of the q_sourceReset slot. If the lines

invalidatePersistentIndexes();
clear_mapping();

are moved into the _q_sourceReset slot, asserts start to be hit, which are fixed by this patch. Equivalently, by clearing internal structures in the kselectionproxymodel sourceAboutToBeReset slot, I could probably fix the asserts in the unit tests, but I think the “finished” slot is a more sensible place for those things, and it is not documented that putting that stuff in the “before” slot is required.

So, I consider this a bug anyway, and I don’t think the behaviour change is a problem. The current observable (undocumented AFAIK) behaviour is:

modelAboutToBeReset
layoutAboutToBeChanged
layoutChanged
modelReset

And this changes it to

modelAboutToBeReset
modelReset

I can’t think of any reason someone would rely on those signals being emitted in the middle, or anything you could do seeing as the persistent indexes would all be invalid. The reset at the end is still there too, so anything done in those slots between would simply be undone.

So I don’t think the behaviour change poses a problem.

Submitted to master (d149a3faca9b97ce806249bc7ef73fe2f59589d5)