Bug 335616 - Navigate to parent folder should select child folder
Summary: Navigate to parent folder should select child folder
Status: RESOLVED FIXED
Alias: None
Product: frameworks-kio
Classification: Frameworks and Libraries
Component: general (show other bugs)
Version: unspecified
Platform: unspecified Linux
: NOR wishlist
Target Milestone: ---
Assignee: David Faure
URL:
Keywords:
: 374101 (view as bug list)
Depends on:
Blocks:
 
Reported: 2014-05-30 23:24 UTC by Gregor Mi
Modified: 2017-10-16 17:39 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gregor Mi 2014-05-30 23:24:46 UTC
This is a feature suggestion of a little detail that helps navigating back and forth in folder structures with many items.

1. Assume the location bar shows this address (in navigate mode; if full path is shown or not is not relevant in this case): 
/home/user1/documents/workset1/

2. Now click on "documents" or Ctrl+Up to go one level up.

3a. CURRENT BEHAVIOUR: In the appearing files and folder list the first item is selected
3b. SUGGESTED BEHAVIOUR: Instead of selecting the first item the folder "workset1" should be selected by default because it is the one from where the user came from.

Rationale:
- When navigating back from a folder hierarchy one might want to go to "workset2" or "workset3" as next action which will be easier to reach with the suggestion assuming the list of folders is large.
- If the first item should be selected anyway one can always press the home key (as opposed to start typing "workset" in the above example or start scrolling to find original folder)

Reproducible: Always
Comment 1 Frank Reininghaus 2014-06-03 07:58:21 UTC
Thanks for the suggestion.

A couple of remarks:

(a) The user already has the possibility to restore the view state (i.e., the current item and the scroll position) when going "back": simply click the "Back" button, or press Alt+Left. If we make the "Up" action (which is not part of the default UI at all, BTW) behave exactly the same, what is the point in having two separate actions at all? Could there be users who use the "Up" action for a reason, namely, to navigate to the parent folder WITHOUT restoring the view state?

Whenever software tries to be smart and guess what the user wants, there is a risk that it sometimes does not quite what the user wants, and then the user has no way to get what they really want. Right now, the behavior is quite predictable and transparent: "Back" restores the view state, everything else doesn't.

(b) If the folder that is clicked in the breadcrumbs bar is not the direct parent of the current folder, but, say, the grandparent: should the view state also be restored? Probably yes. But where should this stop? Should we always scan the entire history if a folder in the breadcrumbs bar is clicked and see if it has been visited already, to restore the view state if that is the case? This may lead to surprising results in some situations.

(c) Concerning the actual implementation: you say that you want the view state restored, but this is only a part of the story. How should the feature interact with the history?

To make it clear what I mean, here is a simple example. The folders in each row show the current state of the history (oldest on top, newest at the bottom), and the folder in [ ] is the current folder.

Step 1: home directory

[/home/user]

Step 2: go to 'folder'

/home/user
[/home/user/folder]

Step 3: go to 'subfolder'

/home/user
/home/user/folder
[/home/user/folder/subfolder]

Step 4a: go 'up, current behavior:

/home/user
/home/user/folder
/home/user/folder/subfolder
[/home/user/folder]

This means that a new history entry will be created, and going "Back" will go back to "subfolder" again.

Step 4b: Interpret "Up" or breadcrumb click as "Back"

/home/user
[/home/user/folder]
/home/user/folder/subfolder

This means that going "Back" in this situation will go to the home directory, even though the last visited folder was actually "subfolder"! This might not be what all users expect.

Alternatively, one could create a new history entry and just restore the view state from another entry, but it starts to get very complex then, and it might still not quite be what the user expects.


I hope that I made it clear that implementing such a feature is not quite as trivial as your report suggests. Some decisisons have to be taken which are far from obvious from my point of view, there is a risk that we replace the current behavior, which is very transparent and predictable, by something which will surprise and annoy some users in certain situations, and it will definitely require more complex code which will have new bugs, of course. From my point of view, it is not clear if the possible benefits of such a change are so big that these drawbacks are acceptable.
Comment 2 Gregor Mi 2014-06-07 12:11:26 UTC
Hello Frank,

thanks for the detailed feedback. In reply to your remarks I give a more specific description of the suggested behaviour.

At first let me say that
- I did not come up with the idea myself; I first discovered the change in behaviour in MS Windows Explorer that came with Windows 7.
- my initial description now feels quite incomplete

With the help of your remarks I did some more experimenting and found that the behaviour can be implemented pretty straightforward.

Specification:
-----------------
The user can change the URL to be displayed in two ways
  1) by using the history buttons (Back, forward) or the corresponding keyboard shortcuts
  2) by using "direct URL input" which includes
   - click on any of the bread crumb button
   - press Alt+Up to go one level up
   - type or paste a URL in the editable bread crumb and press Enter

The feature implementation must know two things when the displayed URL changes:
    a) Was the URL change triggered by the history buttons by "direct URL input"?
    b) Which was the URL that was displayed in the previous view?

Now, only when the URL changes because of direct URL input then compare the current URL with the previous one, then do:
    Is the previous URL is a child of the current one?
      yes --> the direct child should be selected in the current folder view
      no  --> do nothing

Consequences:
--------------------
Most notable: The behaviour when using the Back and Forward buttons is not affected in any aspect. And also the history stack implementation is not affected.
(a counterexample is the implementation in the GNOME file manager "Files". There there is no distinction made between history buttons and direct URL input.
As a result when using the history buttons the view state is not restored in the expected way. In my opinion this is not the way how it should be done)

Reply to remarks:
----------------------
> (a) [...] Could there be users who use the "Up" action for a reason, namely, to navigate to the parent folder WITHOUT restoring the view state?

I can only speak for myself (I use graphical file managers as primary way to interact with files in my daily work):
I use the "Up" action if I would like to go to the parent folder without having to care about the current history stack. And I found that having the child item selected helps to keep the overview in a subtle way:
- I see where I was coming from
- It is much easier to go to the first item than to search for the previous child.

So I suspect that most of the users would not be harmed.

But yes, I actually can imagine that there are users who would care about such a detail AND would not like the feature. So just in case the feature could be implemented as an option.

> Right now, the behaviour is quite predictable and transparent: "Back" restores the view state, everything else doesn't.

After having read my more detailed specification, what do you think about predictability?
I would say: "Back" restores the view state, everything else tries to compare the current URL with the previous one if there is one.

> (b) [...] not the direct parent [...]

Should not be an issue anymore, right?

> (c) [...] How should the feature interact with the history? [...]
> [...] implementing such a feature is not quite as trivial as your report suggests [...]

As described above, the current history implementation should not be affected in any way.

So the implementation is probably not as complex as it seemed by reading my initial description. What do you think?
Comment 3 Frank Reininghaus 2014-06-10 10:17:08 UTC
Thanks for your detailed reply!

(In reply to comment #2)
> So the implementation is probably not as complex as it seemed by reading my
> initial description. What do you think?

I really don't know what's best. I see that some users might prefer your suggestion, but I'm still not quite convinced that it would do more good than harm. This should probably be discussed with the usability team - feel free to start a discussion with them.

> But yes, I actually can imagine that there are users who would care about
> such a detail AND would not like the feature. So just in case the feature
> could be implemented as an option.

I am strongly against adding an option for this: "Let's add an option" always looks like the easiest way to make everyone happy. However, any new option makes the user interface more complex. Moreover, the code gets more complex too, and we have additional code paths which make future maintenance and testing harder, and the change would most likely introduce new bugs.

Therefore, one always has to ask the question "Is there a *very* good reason why we want to offer the possibility to configure this behavior? IMHO, the answer is "No" in this case. This is such a minor issue that we just have to make a choice - either do it this way or the other way.

In any case, this is not exactly a Dolphin issue - saving and historing the view state (including the current item and the scroll position) is mostly controlled by the class KUrlNavigator from kdelibs.

If Dolphin, the file dialog, or any other application that uses KUrlNavigator should restore the view state, then KUrlNavigator emits its historyChanged() signal.

So if the result of the discussion with the usability team is that the suggested change should be made, then KUrlNavigator should be modified such that it checks whether the new URL is equal to the previous one after pressing "Up" or a breadcrumb button, copies the history state of that URL in that case, and emits the historyChanged() signal.

No changes in Dolphin are required to make this work, so I'll reassign to KUrlNavigator.
Comment 4 Frank Reininghaus 2014-06-10 12:25:29 UTC
For some earlier discussions about this issue, see, e.g., bug 315471 and bug 193549.
Comment 5 Gregor Mi 2014-11-02 19:31:02 UTC
The usability team approves the new behaviour. To the question if there is a use case to keep the current behaviour there was no answer (mail from Heiko Tietze on kde-usability@kde.org, 14/06/14 11:29, "[KDE Usability] Dolphin/KUrlNavigator: setUrl should select child folder item of last URL if available").
Comment 6 Gregor Mi 2015-03-22 17:38:07 UTC
I looked at frameworks/kio/src/filewidgets/kurlnavigator.h and frameworks/kio/src/filewidgets/kurlnavigator.h and it seems like selecting the child folder has to be done the application that uses KUrlNavigator and not by KUrlNavigator itself.
Comment 7 Emmanuel Pescosta 2015-03-22 18:07:13 UTC
(In reply to Gregor Mi from comment #6)

Thanks for asking the usability team! :)

Given that Frank has raised some valid concerns about this topic, some final words from him would be really nice ;)

> I looked at frameworks/kio/src/filewidgets/kurlnavigator.h and
> frameworks/kio/src/filewidgets/kurlnavigator.h and it seems like selecting
> the child folder has to be done the application that uses KUrlNavigator and
> not by KUrlNavigator itself.

Dolphin already contains code for restoring the view states when the KUrlNavigator emits the historyChanged signal, see DolphinViewContainer::slotHistoryChanged. So no adjustments are needed in Dolphin itself. The KUrlNavigator must check if the url history contains a location state for the "up"-url (in KUrlNavigator::goUp), if yes it must set the right history index and emit all necessary signals (same as in KUrlNavigator::goBack) and if not the current goUp code must be executed. 

I hope this helps ;)
Comment 8 Frank Reininghaus 2015-03-22 18:24:35 UTC
(In reply to Gregor Mi from comment #5)
> The usability team approves the new behaviour. To the question if there is a
> use case to keep the current behaviour there was no answer (mail from Heiko
> Tietze on kde-usability@kde.org, 14/06/14 11:29, "[KDE Usability]
> Dolphin/KUrlNavigator: setUrl should select child folder item of last URL if
> available").

To make it easier for others to find this mail, here is the link: http://marc.info/?l=kde-usability&m=140273827601672&w=2

(In reply to Emmanuel Pescosta from comment #7)
> Given that Frank has raised some valid concerns about this topic, some final
> words from him would be really nice ;)

I'm not against such a change, provided someone volunteers to make the necessary changes in KUrlNavigator, tests them extensively to make sure that the history does not get messed up in any case (ideally with unit tests that model some sequences of back/forward/up/manual URL input actions), and takes care of any regressions that are reported in the near future. I am definitely against adding an option for this though, as I said earlier.

I'll reassign to the KIO framework because such a change should not be done in kfile from kdelibs 4.x.
Comment 9 Gregor Mi 2015-03-22 19:45:05 UTC
> Dolphin already contains code for restoring the view states when the KUrlNavigator emits the historyChanged 
> signal, see DolphinViewContainer::slotHistoryChanged. So no adjustments are needed in Dolphin itself. The 
> KUrlNavigator must check if the url history contains a location state for the "up"-url (in KUrlNavigator::goUp), if yes 
> it must set the right history index and emit all necessary signals (same as in KUrlNavigator::goBack) and if not the 
> current goUp code must be executed. 

Thanks for the pointers. I currently don't see how restoring a history index can work in this example:

1. Enter "/usr/local/share" in the location bar and hit ENTER.
2. Enter "/usr" in the location bar and hit ENTER.
3. Expected: the item "local" in the view that displays "/usr/" becomes selected. But this selection has never been in the history list (or I do not understand yet how the history works).
Comment 10 Emmanuel Pescosta 2015-03-22 22:53:12 UTC
(In reply to Frank Reininghaus from comment #8)
> I am definitely against adding an option for this though, as I said earlier.

+1

Thanks for your feedback! :)
Comment 11 Emmanuel Pescosta 2015-03-22 23:11:14 UTC
(In reply to Gregor Mi from comment #9)
> > Dolphin already contains code for restoring the view states when the KUrlNavigator emits the historyChanged 
> > signal, see DolphinViewContainer::slotHistoryChanged. So no adjustments are needed in Dolphin itself. The 
> > KUrlNavigator must check if the url history contains a location state for the "up"-url (in KUrlNavigator::goUp), if yes 
> > it must set the right history index and emit all necessary signals (same as in KUrlNavigator::goBack) and if not the 
> > current goUp code must be executed. 
> 
> Thanks for the pointers. I currently don't see how restoring a history index
> can work in this example:

Ok sorry, I thought the question is only about the go-up.
 
> 1. Enter "/usr/local/share" in the location bar and hit ENTER.
> 2. Enter "/usr" in the location bar and hit ENTER.
> 3. Expected: the item "local" in the view that displays "/usr/" becomes
> selected. But this selection has never been in the history list (or I do not
> understand yet how the history works).

In this case some adjustments in Dolphin are needed. 

1. option:
Save the current navigator url on urlAboutToBeChanged and check if the new navigator url is a parent url of the previous one, then use DolphinView::markUrlAsCurrent which automatically scrolls to the right position.

2. option (better IMO):
Use KUrlNavigator::locationUrl to get the previous url and check if the new navigator url is a parent url of the previous one, then use DolphinView::markUrlAsCurrent which automatically scrolls to the right position.
Comment 12 Gregor Mi 2015-04-04 13:06:18 UTC
Implementation suggestion: https://git.reviewboard.kde.org/r/123253/
Comment 13 fsando 2015-08-30 15:24:48 UTC
I posted the same request years ago - can't find the reference anymore. This is my number one issue with dolphin and the only reason I'm still ocasionally looking for a replacement. 

+1
Comment 14 Gregor Mi 2015-09-01 09:26:37 UTC
As you can see also from the review request's comments (https://git.reviewboard.kde.org/r/123253/) there are two things that currently block this feature

1. doubts of the usefulness of the feature
2. discussions about the correct way of implementing it

My suggestion in the RR was to first remove the doubts by bringing the change in. My patch is currently minimal invasive and functional as far as I tested it. Probably someone else should try it, too.

(In a later step there should be changes to kio so that the new behaviour is also available in file dialogs and not dolphin only.)
Comment 15 Christoph Feck 2016-12-31 05:16:46 UTC
*** Bug 374101 has been marked as a duplicate of this bug. ***
Comment 16 Hrtmt Brng 2016-12-31 10:18:14 UTC
Wow, big discussion!

I think the following minimal solution is agreed by the most of you:

Typing Alt+Up goes the path up and brings the cursor on the folder where we are comming from.

Thats all.

For navigation bar we don't need this features because the navigation bar is used by mouse. This is also what the user expects because when using the navigation bar, the user explicitly selects a folder where he wants to navigate to. This is a different thing than just going up. In a web browser you either do not expect an Up-Action.

The history behaviour shall not be affected because this would confuse the users. Imagine the situation that dolphin is just started in /home/user1 folder. Alt+Left is not available and shall not be available. But Alt+Up brings us to /home. So Alt+Up has nothing to do with Alt+Left. And then of course Alt+Left brings us back from /home to /home/user1 because this is the history.

So, I think it is the most intuitive solution to implement Alt+Up without interfering other features.
Comment 17 Roman Gilg 2017-08-01 11:45:52 UTC
The patch 

https://cgit.kde.org/kio.git/commit/?id=fa6bad3be579ec89c8126b00438234d0bc16c508

for this wishlist item committed two days ago broke my file selection dialog:

ASSERT: "childPath.length() >= (parentPath.length() + 2)" in file /home/roman/Entwicklung/kde/source/frameworks/kio/src/filewidgets/urlutil_p.h, line 73

Reproducible in QtCreator > Open Project.

Reverting the patch makes it work again.
Comment 18 Gregor Mi 2017-08-03 08:29:15 UTC
Thankfully, Daniel Vrátil wrote a patch (https://phabricator.kde.org/D7043) to fix this bug.
Comment 19 Emmanuel Pescosta 2017-09-21 13:45:35 UTC
Git commit 3749b5552729b434655ddc950b120bb14fe888c7 by Emmanuel Pescosta, on behalf of Gregor Mi.
Committed on 21/09/2017 at 13:41.
Pushed by emmanuelp into branch 'master'.

Select child folder when navigating to a parent folder

Whenever the dolphin view is initialized to show the contents of a new URL
(e.g. "/home/x/test") it will be checked if the new URL is a parent of the
previous displayed URL (e.g. "/home/x/test/documents/aaa"). If the check is
successful, then the common child (in this example: "/home/x/test/documents/")
folder item will be selected and scrolled into view.

REVIEW: 123253
Related: bug 377392

M  +1    -1    CMakeLists.txt
M  +11   -0    src/dolphinviewcontainer.cpp
M  +5    -0    src/dolphinviewcontainer.h

https://commits.kde.org/dolphin/3749b5552729b434655ddc950b120bb14fe888c7
Comment 20 Nate Graham 2017-10-16 17:39:03 UTC
Looks like for some reason this didn't get marked as resolved when the code made it in.