<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/122252/">https://git.reviewboard.kde.org/r/122252/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On January 26th, 2015, 9:41 a.m. UTC, <b>Christian Mollekopf</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Looks reasonable to me. I'll apply the patch locally and test it for a while.</p></pre>
</blockquote>
<p>On January 28th, 2015, 3:51 p.m. UTC, <b>Christian Mollekopf</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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 sa
me kmail/kdelibs version + the same account.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The problematic folder tree looks as follows:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;"><div class="codehilite" style="background: #f8f8f8"><pre style="line-height: 125%"> <span style="color: #666666">*</span> Shared Folders (no mimetype)
<span style="color: #666666">**</span> shared (no mimetype)
<span style="color: #666666">***</span> lists (no mimetype)
<span style="color: #666666">****</span> kde.org (no mimetype)
<span style="color: #666666">*****</span> pim (mail mimetype)
<span style="color: #666666">*****</span> ...
<span style="color: #666666">****</span> ...
<span style="color: #666666">***</span> ...
</pre></div>
</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
<br />
<p>- Christian</p>
<br />
<p>On January 25th, 2015, 6:51 p.m. UTC, David Faure wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for kdelibs and Christian Mollekopf.</div>
<div>By David Faure.</div>
<p style="color: grey;"><i>Updated Jan. 25, 2015, 6:51 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdelibs
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">by using an internal cache of the filtering state.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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.</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unit tests.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">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).</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>kdeui/itemviews/krecursivefilterproxymodel.h <span style="color: grey">(c16b62186fb9203ff297bd9fd28d9c13a1c8bdc4)</span></li>
<li>kdeui/itemviews/krecursivefilterproxymodel.cpp <span style="color: grey">(efa286ad87ded962b20c8a581b659d1b154ebf3a)</span></li>
<li>kdeui/tests/krecursivefilterproxymodeltest.cpp <span style="color: grey">(3bcb72980730cb22f887ae8fa5fbd91b5609aeb6)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/122252/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>