Review Request 110915: After viewing an image and pressing "Up" or "Browse", gwenview should retain the active selection

[Jan Kundrát]

> On June 11, 2013, 3:56 p.m., Aurélien Gâteau wrote:
> > 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.
> 
> Shlomi Fish wrote:
>     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
> 
> Aurélien Gâteau wrote:
>     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.

I do have that power, and will mark it as such.


- Jan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/110915/#review34152
-----------------------------------------------------------


On June 9, 2013, 6:29 p.m., Shlomi Fish wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/110915/
> -----------------------------------------------------------
> 
> (Updated June 9, 2013, 6:29 p.m.)
> 
> 
> Review request for Gwenview.
> 
> 
> Bugs: 319302
>     http://bugs.kde.org/show_bug.cgi?id=319302
> 
> 
> Repository: gwenview
> 
> 
> Description
> -------
> 
> Fix for bug #319302. Tested.
> 
> 
> Diffs
> -----
> 
>   app/mainwindow.h 9670c9a 
>   app/mainwindow.cpp 70b8d3b 
> 
> Diff: https://git.reviewboard.kde.org/r/110915/diff/
> 
> 
> Testing
> -------
> 
> Going over the browser view, and switching from image view mode to browse mode using "Browse" and the up button.
> 
> 
> Thanks,
> 
> Shlomi Fish
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/gwenview-devel/attachments/20150529/d747d97e/attachment.html>