Review Request 109471: Fix Bug 312014 - testing: group by file does not work correctly

[Frank Reininghaus]

> On March 16, 2013, 10:53 a.m., Frank Reininghaus wrote:
> > Thanks Emmanuel, but even with your new patch, I can still reproduce the bug, and the unit test still fails.
> > 
> > I think the problem is the following: when the folder is loaded, and the "Type" is neither shown nor used as the sort role, retrieveData() does not set the "TypeRole". If we then switch to "Sort by Type", that role is still empty, i.e., all items look like they have the same type, which is why the fallback sorting by "Name" is used.
> > 
> > Idea how to fix this: in KFileItemModel::onSortRoleChanged(), if the new sort role is "Type", try to determine the mime types synchronously for 200 ms *before* the resorting, and makes sure that KFileItemModelRolesUpdater does the rest asynchronously.
> 
> Emmanuel Pescosta wrote:
>     Thanks for testing! :)
>     
>     > I can still reproduce the bug
>     Hmm, ok. Very strange
>     It works fine for me (34 different type groups -> 36 folders + 100 files) - Can you please tell me your setup?
>     
>     > and the unit test still fails
>     All unit tests work in master ;) Maybe we should make this a master-only fix?
>     
>     > Idea how to fix this
>     Ok thanks! I will try it during the night duty or maybe tomorrow (depends on the number of deployments of emergency services)

First of all, I think that bug 312014 and https://bugs.kde.org/show_bug.cgi?id=310705 are actually the same and that this is not related to grouping at all. Here is how I reproduce the latter (no matter if your patch is applied or I just use the KDE/4.10 branch):

I have Dolphin set up to use Details View with Name/Date/Size columns enabled, common view properties in all folders. Previews are enabled, but I think it doesn't matter. "Sort by Name" is enabled.

I create a test folder and run Dolphin in it:

mkdir test && cd test && touch a.txt b.jpg c.txt && dolphin .

The view shows the files sorted by name:

a.txt
b.jpg
c.txt

After choosing "Type" from the "Sort by" menu, the order of the files does not change, but it does when I press F5.


- Frank


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109471/#review29308
-----------------------------------------------------------


On March 15, 2013, 2:53 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109471/
> -----------------------------------------------------------
> 
> (Updated March 15, 2013, 2:53 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> Fix wrong grouping of files (Esp. when sorting by file type)
> 
> Changes needed for master:
> Add "const ItemData* parent = itemData->parent;" to retrieveData and remove the parent parameter from retrieveData.
> 
> Ideas for master:
> Maybe use "void QtConcurrent::blockingMap ( Iterator begin, Iterator end, MapFunction function )" instead of a simple loop in setRoles, slotRefreshItems and createItemDataList.
> 
> 
> This addresses bug 312014.
>     http://bugs.kde.org/show_bug.cgi?id=312014
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.h ef9dc98 
>   dolphin/src/kitemviews/kfileitemmodel.cpp 7927245 
> 
> Diff: http://git.reviewboard.kde.org/r/109471/diff/
> 
> 
> Testing
> -------
> 
> Works for me.
> 
> Open a folder -> Sort by Type -> Sort by Rating -> Sort by .... -> Finally: Sort by Type (Files should be in the right file-type group)
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20130317/64de7be6/attachment.htm>