Summary: | Keyboard searches do not reset when entering directory | ||
---|---|---|---|
Product: | [Applications] dolphin | Reporter: | Leszek Lesner <leszek.lesner> |
Component: | view-engine: general | Assignee: | Peter Penz <peter.penz19> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | christian, finex, frank78ac, matt.ruffalo, nitro, Ondrej.Machulda, stuffcorpse |
Priority: | NOR | ||
Version: | 2.0 | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | http://commits.kde.org/kde-baseapps/68ce395a192362969783615e50a8004d3029eb7e | Version Fixed In: | 4.8.3 |
Sentry Crash Report: | |||
Attachments: | Keyboardsearch fix |
Description
Leszek Lesner
2012-04-04 18:53: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. @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. P.S: This bug looks not related to bug #297458 @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! 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 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. 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.
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. 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. @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. 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 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 *** Bug 299196 has been marked as a duplicate of this bug. *** |