Bug 297488 - Keyboard searches do not reset when entering directory
Summary: Keyboard searches do not reset when entering directory
Status: RESOLVED FIXED
Alias: None
Product: dolphin
Classification: Applications
Component: view-engine: general (show other bugs)
Version: 2.0
Platform: unspecified Linux
: NOR normal
Target Milestone: ---
Assignee: Peter Penz
URL:
Keywords:
: 299196 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-04-04 18:53 UTC by Leszek Lesner
Modified: 2012-05-05 09:38 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In: 4.8.3
Sentry Crash Report:


Attachments
Keyboardsearch fix (2.27 KB, patch)
2012-04-07 11:03 UTC, Leszek Lesner
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Leszek Lesner 2012-04-04 18:53:29 UTC
Together with the change of 4.8.2 keyboard search delay a new bug appeared. 
When entering a directory with the help of keyboard search i.e.
I have one folder called downloads and a subfolder in downloads called work. 

I open dolphin type "down" (Downloads folder gets hightlighted) hit enter and then immediately  start typing "wo" nothing happens. I wait for 5 seconds type in "wo" and my work directory gets highlighted.

The keyboard search should be cleared when entering new directories to prevent this problem.
Comment 1 Frank Reininghaus 2012-04-05 10:25:29 UTC
Thanks for the bug report! Sorry for that stupid regression (well, technically, it's not a regression because the bug has been there before, it was just not easy to see it because the timeout was so short).

I'll try to figure out a clean solution for this before KDE 4.8.3 is released.
Comment 2 FiNeX 2012-04-05 11:37:54 UTC
@Frank: I've just experienced this bug... I want to add that there is something with the case of file names either. When you will work on this bug please try on a directory wih files with both lowercase and uppercase names like "Folder one, folder two, example, Exams..."... I didn't understand a pattern in order to reproduce this bug.
Comment 3 FiNeX 2012-04-05 11:39:11 UTC
P.S: This bug looks not related to bug #297458
Comment 4 Frank Reininghaus 2012-04-05 12:53:33 UTC
@FiNeX: I just tried keyboard searching in a folder with uppercase and lowercase names and noticed nothing unusual. Could you be a bit more specific about what kind of strange behaviour you noticed (preferably not here because this is off-topic for this bug report)? Thanks!
Comment 5 Frank Reininghaus 2012-04-06 09:31:05 UTC
Git commit d8732a59d3b1f2d0bebf43f294df7e9f333abde4 by Frank Reininghaus.
Committed on 05/04/2012 at 18:59.
Pushed by freininghaus into branch 'KDE/4.8'.

Reduce the timeout in KItemListKeyboardSearchManager to 1 second

It turned out that the longer timeout, introduced recently in
02eab49b2de51c31fe46a0d9501327b579b3648e, not only made multi-letter
keyboard searches easier, but also had some unwanted side effects. I
hope that 1 second, which is between the previous value of 5 seconds and
the pre-KDE 4.8.2 value of 0.4 seconds, is a compromise which will fit
most users' needs.
We will try to improve the situation further in future releases by
providing visual feedback about the keyboard search, but such a change
would not be suitable for a bug-fix release.
Related: bug 297458

M  +1    -1    dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp

http://commits.kde.org/kde-baseapps/d8732a59d3b1f2d0bebf43f294df7e9f333abde4
Comment 6 Christian Authmann 2012-04-06 13:45:40 UTC
Regardless of the actual timeout, IMHO the search should be cleared when:
* the location changes / a new folder is opened
* any action is done that's unrelated to keyboard search, e.g. mouseclicks, hotkeys etc
* the window loses focus
* the user starts navigating with the cursor keys
* the user hits escape

The last one is important when you misspell a search and wish to retry without waiting for the timeout to happen.
Clearing on cursor keys, enter and esc would cover the most common cases (for me) without touching too many areas of the code.
Comment 7 Leszek Lesner 2012-04-07 11:03:30 UTC
Created attachment 70206 [details]
Keyboardsearch fix

As we were not satisfied with setting the keyboardsearch timeout to 1 second. I decided to write another patch which is now included in Neptune. 
It will delete the keyboard search item whenever enter key is pressed and if you accidently typed in something wrong you can also delete the search by pressing escape. 
Sorry Christian but I did not see your comment before writing the patch so I only implemented the first and last point of your lrequest list. But I guess with the way its implemented now(with this patch) it should be no problem to adapt the patch to your request.
Comment 8 Frank Reininghaus 2012-04-08 18:48:15 UTC
Thanks for the patch! It's definitely a step into the right direction :-) However, as Christian wrote, one would also expect the search to be cancelled in a couple more situations. Doing that (and providing some visual feedback about the search string) would require quite a few code changes, Nevertheless, I think that it might make sense to use your patch at least in the 4.8 branch to improve the current situation, in particular because it's a very small change that looks unlikely to break anything (although I've learned now that you can never be sure about that). Let's wait for feedback from Peter about this.
Comment 9 Matt Ruffalo 2012-04-09 18:19:45 UTC
Thank you very much for fixing this. I can confirm that a timeout of 1 second is *way* too high -- I know my folder structure pretty well and before 4.8.2 I could navigate to a specific directory 4 or 5 levels deep in under 3 seconds (Doc <enter> res <enter> pap <enter> map <enter> fig <enter>). I can't quite time myself since I'm using 4.8.2 everywhere though :)

I'm looking forward to 4.8.3 mostly to get this patch; thanks again.
Comment 10 Peter Penz 2012-04-09 19:09:31 UTC
@Leszek: Thanks for the patch, but if Frank's patch to decrease the timeout to 1 second is still not sufficient I'd prefer going back to 0.4 seconds for 4.8.x instead of applying the patch. Although the patch resolves the problem for the keyboard navigation, I don't like the approach to hardcode this as part of the controller. I'd say the correct fix would be to reset the keyboard-timeout whenever the items of the model have been cleared and to let the keyboard-manager internally handle this situation. But this is more a 4.9 thing from my point of view.
Comment 11 Frank Reininghaus 2012-04-25 07:09:33 UTC
Git commit 68ce395a192362969783615e50a8004d3029eb7e by Frank Reininghaus.
Committed on 25/04/2012 at 09:01.
Pushed by freininghaus into branch 'KDE/4.8'.

When the current item is removed, make -1 the current index temporarily

This fixes two problems:
1. KItemListKeyboardSearchManger can cancel the current search when a
   new folder is opened (note that this action removes the current item
   from the view).
2. The view can underline the new current item (which is the item that
   used to be below the removed item). Note that this did not work
   before because the view did not receive a currentChanged() signal in
   this case and therefore did not update the "current item" status of
   the new current item.
Related: bug 298782
FIXED-IN: 4.8.3
REVIEW: 104709

M  +2    -0    dolphin/src/kitemviews/kitemlistcontroller.cpp
M  +10   -0    dolphin/src/kitemviews/kitemlistkeyboardsearchmanager.cpp
M  +4    -0    dolphin/src/kitemviews/kitemlistkeyboardsearchmanager_p.h
M  +6    -3    dolphin/src/kitemviews/kitemlistselectionmanager.cpp

http://commits.kde.org/kde-baseapps/68ce395a192362969783615e50a8004d3029eb7e
Comment 12 Frank Reininghaus 2012-04-25 07:22:11 UTC
Git commit 922742762f2b1655ba55756b590e327acb00ce77 by Frank Reininghaus.
Committed on 25/04/2012 at 09:20.
Pushed by freininghaus into branch 'master'.

When the current item is removed, make -1 the current index temporarily

This fixes two problems:
1. KItemListKeyboardSearchManger can cancel the current search when a
   new folder is opened (note that this action removes the current item
   from the view).
2. The view can underline the new current item (which is the item that
   used to be below the removed item). Note that this did not work
   before because the view did not receive a currentChanged() signal in
   this case and therefore did not update the "current item" status of
   the new current item.
Related: bug 298782
REVIEW: 104709
(cherry picked from commit 68ce395a192362969783615e50a8004d3029eb7e)

M  +2    -0    dolphin/src/kitemviews/kitemlistcontroller.cpp
M  +6    -3    dolphin/src/kitemviews/kitemlistselectionmanager.cpp
M  +10   -0    dolphin/src/kitemviews/private/kitemlistkeyboardsearchmanager.cpp
M  +4    -0    dolphin/src/kitemviews/private/kitemlistkeyboardsearchmanager.h

http://commits.kde.org/kde-baseapps/922742762f2b1655ba55756b590e327acb00ce77
Comment 13 Frank Reininghaus 2012-05-05 09:38:51 UTC
*** Bug 299196 has been marked as a duplicate of this bug. ***