<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/110915/">https://git.reviewboard.kde.org/r/110915/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On June 11th, 2013, 3:56 p.m. UTC, <b>Aurélien Gâteau</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;">Hi Schlomi! Thanks a lot for taking the time to provide a way to reproduce the bug and for writing the patch.
I tried the patch, and while it fixes the bug, I don't think it fixes the root cause of the problem.
After some experimentation, I realized the problem is caused by the fact Gwenview reconfigures the dir model when switching between browse and view modes. If you look at MainWindow::setActiveViewModeAction(), you can see we call setDirModelShowDirs(true) in browse mode and setDirModelShowDirs(false) in view mode. This is done so that folders are shown in the thumbnail view in browse mode, but are hidden from the thumbnail bar in view mode. Unfortunately, this cause the model content to refresh and thus the selection is lost. If I comment out calls to setDirModelShowDirs(), the bug goes away, is it the same for you?
The solution I am looking at right now is to always have mDirModel list dirs, have mThumbnailView uses this model, create a proxy model on top of mDirModel which would filter out dirs and use this proxy model for thumbnail bars. This way there is no need to force models to refresh themselves and the bug should be fixed. It is not ready yet, hopefully will be done this week.</pre>
</blockquote>
<p>On June 11th, 2013, 5:02 p.m. UTC, <b>Shlomi Fish</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;">Hi Aurélien! Yes, you are right that removing the calls to setDirModelShowDirs($BOOL) fixes the problem and makes the bug go away. I don't understand what your proposed solution involves, but I'll let you carry on with it. Please let me know if I can help.
By the way - it's "Shlomi" - not "Schlomi": http://www.shlomifish.org/meta/FAQ/#your_name .
Best regards,
-- Shlomi Fish</pre>
</blockquote>
<p>On June 11th, 2013, 9:45 p.m. UTC, <b>Aurélien Gâteau</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;">First of all, sorry for mistyping your first name.
I hope you don't mind me going with my more invasive solution, your report and patch highlighted a flaw in Gwenview design I intend to fix. I will file my version in a new review request and will add you to the reviewers so you can give it a try. Can you mark this request as "discarded"? I do not have the power to do so.</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 do have that power, and will mark it as such.</p></pre>
<br />
<p>- Jan</p>
<br />
<p>On June 9th, 2013, 6:29 p.m. UTC, Shlomi Fish 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 Gwenview.</div>
<div>By Shlomi Fish.</div>
<p style="color: grey;"><i>Updated June 9, 2013, 6:29 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=319302">319302</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
gwenview
</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;">Fix for bug #319302. Tested.</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;">Going over the browser view, and switching from image view mode to browse mode using "Browse" and the up button.</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>app/mainwindow.h <span style="color: grey">(9670c9a)</span></li>
<li>app/mainwindow.cpp <span style="color: grey">(70b8d3b)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/110915/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>