> On Jan. 26, 2015, 9:41 a.m., Christian Mollekopf wrote: > > Looks reasonable to me. I'll apply the patch locally and test it for a > > while. > > Christian Mollekopf wrote: > This patch brings the original problem back, that shared folders do not > appear until something causes a dataChanged signal (usually a sync). Since > the model now seems to be behaving correctly, I assume the kmail model stack > is buggy in yet another place (would have been to trivial otherwise wouldn't > it?), and the superfluous dataChanged signal just happened to hide that > problem. > > Christian Mollekopf wrote: > I tracked the problem down to still be in FolderTreeWidgetProxyModel > (which is a KRecursiveFilterProxyModel). I know the relevant index makes it > through the sourcemodel because of the debug output added in > FolderTreeWidgetProxyModel::acceptRow (which also returns the correct > values). I'm fairly sure that the model is the cause for the folder not > showing up because I set the foldertreewidgetproxymodel directly as source of > the folderTreeView (a QTreeView). Given that the filtering seems to work > correctly, and assuming QTreeView isn't buggy, the reason for the folder not > showing up has to be in KRecursiveFilterProxyModel (right?). The problem is > most likely timing/order dependant because I cannot reproduce the problem > with another user but the same kmail/kdelibs version + the same account. > > The problematic folder tree looks as follows: > ``` > * Shared Folders (no mimetype) > ** shared (no mimetype) > *** lists (no mimetype) > **** kde.org (no mimetype) > ***** pim (mail mimetype) > ***** ... > **** ... > *** ... > ``` > > The problem is that the complete folder hierarchy, including "Shared > Folders" doesn't make it into the visible tree. The top 4 folders of the > hierarch would of course only be pulled in by the actual mail folder (pim). > > So far I couldn't find the actual problem to write another testcase, but > I have to assume that KRecursiveFilterProxyModel ist still buggy, and the > additional dataChanged signal just happen to rectify the problem.
This definitely sounds like missing rowsInserted signals from the proxy. I'll try adding a testcase where the source model does rowsInserted(1), rowsInserted(1.1), rowsInserted(1.1.1 + with flag set). - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122252/#review74753 ----------------------------------------------------------- On Jan. 25, 2015, 6:51 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122252/ > ----------------------------------------------------------- > > (Updated Jan. 25, 2015, 6:51 p.m.) > > > Review request for kdelibs and Christian Mollekopf. > > > Repository: kdelibs > > > Description > ------- > > by using an internal cache of the filtering state. > > The tricky part is that filterAcceptsRow() must not use the cache > (too bad, it would be faster), because of the setFilterFixedString() > feature which can change the filtering status of indexes without > KRFPM even being informed (QSFPM has no virtual method, no event...) > So it only ever writes to the cache, but when dataChanged() > or row insertion/removal comes in, we can look into the cache > to find the old state and compare. > > > Diffs > ----- > > kdeui/itemviews/krecursivefilterproxymodel.h > c16b62186fb9203ff297bd9fd28d9c13a1c8bdc4 > kdeui/itemviews/krecursivefilterproxymodel.cpp > efa286ad87ded962b20c8a581b659d1b154ebf3a > kdeui/tests/krecursivefilterproxymodeltest.cpp > 3bcb72980730cb22f887ae8fa5fbd91b5609aeb6 > > Diff: https://git.reviewboard.kde.org/r/122252/diff/ > > > Testing > ------- > > Unit tests. > > Using kmail (and especially testing the substring filtering in the "move > dialog", which an earlier version of this patch broke). > Looking at the number of calls to > Akonadi::FavoriteCollectionsModel::Private::reload() for a single > dataChanged() signal from the ETM, which went from 10 to 4 (still too many, > but the remaining problem is elsewhere). > > > Thanks, > > David Faure > >
