Bug 324371 - Press Left & Right keys in Search Results => Folders dont wrap back
Summary: Press Left & Right keys in Search Results => Folders dont wrap back
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: view-engine: details mode (show other bugs)
Version: 4.11.0
Platform: Chakra Linux
: NOR minor
Target Milestone: ---
Assignee: Dolphin Bug Assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-09-01 18:58 UTC by goat13
Modified: 2013-10-07 07:31 UTC (History)
0 users

See Also:
Latest Commit:
Version Fixed In: 4.12.0


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description goat13 2013-09-01 18:58:07 UTC
[IMG]http://i.imgur.com/8F20p8t.png[/IMG]

Reproducible: Sometimes

Steps to Reproduce:
1. Do a search so that you have some results incl. folder
2. Press Left & Right keys on a folder
Comment 1 goat13 2013-09-01 19:04:30 UTC
and if i press Right only  the steady file is listed twice!
Comment 2 Frank Reininghaus 2013-09-02 21:45:04 UTC
Thanks for the bug report! I can confirm that this strange effect occurs sometimes when pressing Right+Left in a list of search results in Details View, but I haven't found a pattern yet.
Comment 3 goat13 2013-09-02 22:46:16 UTC
maybe when both folder name and filename in it respond the search request...
Comment 4 Frank Reininghaus 2013-09-03 12:18:29 UTC
(In reply to comment #3)
> maybe when both folder name and filename in it respond the search request...

Yes, good idea! The problem is that the file is shown twice in that case: once as a child of its expanded parent folder (which matches the search), and once on its own (because it matches the search criteria as well).

When the parent folder is collapsed, the wrong item is removed: not the one that is a child of the parent, but the one that is listed on its own in the search results.

Hm, it might not be straightforward to fix. In fact, the root cause of the problem is that the internal data structures of the model are not consistent any more in the use case that you describe. We have a hash that maps a URL to an index, but if an item is shown twice in the view, then the URL is mapped to the *last* index that this item corresponds to. This is the item that gets removed then.
Comment 5 goat13 2013-09-03 15:16:56 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > maybe when both folder name and filename in it respond the search request...
> 
> Yes, good idea! The problem is that the file is shown twice in that case:
> once as a child of its expanded parent folder (which matches the search),
> and once on its own (because it matches the search criteria as well).
> 
> When the parent folder is collapsed, the wrong item is removed: not the one
> that is a child of the parent, but the one that is listed on its own in the
> search results.
> 
> Hm, it might not be straightforward to fix. In fact, the root cause of the
> problem is that the internal data structures of the model are not consistent
> any more in the use case that you describe. We have a hash that maps a URL
> to an index, but if an item is shown twice in the view, then the URL is
> mapped to the *last* index that this item corresponds to. This is the item
> that gets removed then.

Tried to search 'mus' when having music/music1/music2/music3/mus
1. It DOESNT find the 'mus' file!
2. Wrapping/unwrapping your listing is definitely corrupt now =))

I think you should loose these wrapping triangles in Search listing (cant imagine any satisfying behavior for it). And make statusbar show the relative way to the element (not just its name) on mouse-over.
Comment 6 Frank Reininghaus 2013-10-07 07:31:24 UTC
Git commit 84b40da88d9821c6fc0c9ccbc3a72ec752033763 by Frank Reininghaus.
Committed on 07/10/2013 at 07:26.
Pushed by freininghaus into branch 'master'.

Make the code that removes items from KFileItemModel more robust

When we remove items from the model, we called the function
KFileItemModel::removeItems(const KFileItemList&, RemoveItemsBehavior).
This function then looked up the indexes of the items using the hash
m_items. This is wasteful in the situations when the indexes of the
removed items are known in advance (like when an expanded folder is
collapsed in Details View), and it can cause trouble if one item is
contained in the model multiple times (can happen when searching, and a
file both matches the search and is a child of a folder that matches
the search). Even if expanding folders in the search results list might
not be particularly useful most of the time, it makes sense to make the
model more robust to prevent crashes and other unexpected behavior in
such situations.

This patch makes the following changes to achieve that goal:

* Change the argument of removeItems() from KFileItemList to
  KItemRangeList. To make this work, the "look the indexes up in
  m_items" code is moved from that function to slotItemsDeleted(). In
  the other places where removeItems() is called, the indexes are
  calculated directly (which is not more difficult than determining the
  removed items as a KFileItemList, if one considers that we needed the
  function childItems(KFileItem) for that, which is not needed any more
  with this patch).

* Also removeFilteredChildren() takes a KItemRangeList now. Rather than
  putting the parent KFileItems into a QSet for O(1) lookup (which
  prevents O(N^2) worst case behavior for the entire function), it uses
  a QSet<ItemData*> now, which should even be more efficient (hashing a
  pointer is cheaper than hashing a KFileItem/KUrl).
Related: bug 325359
FIXED-IN: 4.12.0
REVIEW: 113070

M  +77   -70   dolphin/src/kitemviews/kfileitemmodel.cpp
M  +2    -7    dolphin/src/kitemviews/kfileitemmodel.h
M  +1    -1    dolphin/src/tests/kfileitemmodelbenchmark.cpp
M  +58   -0    dolphin/src/tests/kfileitemmodeltest.cpp

http://commits.kde.org/kde-baseapps/84b40da88d9821c6fc0c9ccbc3a72ec752033763