Note: There are a few cases of duplicates in user autocompletion which are being worked on.

Sort by Name should sort between separators

RESOLVED FIXED in Firefox 3 alpha7

Status

()

Firefox
Bookmarks & History
P3
normal
RESOLVED FIXED
13 years ago
8 years ago

People

(Reporter: Ger Teunis, Assigned: mano)

Tracking

unspecified
Firefox 3 alpha7
Points:
---
Bug Flags:
blocking-aviary1.5 -
blocking-firefox3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [asaP2])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

13 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040930 Firefox/0.10
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7.3) Gecko/20040930 Firefox/0.10

Sort by Name now deletes all seperators and then sorts the remaining items.
It should sort items between two seperators.
I.e.

Bookmarks:

Folder2
Folder1
---------
Item3
Item1
---------
Item4
Item2


Should sort into:

Folder1
Folder2
---------
Item1
Item3
---------
Item2
Item4

Reproducible: Always
Steps to Reproduce:
1. Open Bookmarks Menu
2. Rightclick an bookmark item
3. Select Sort By Name

Actual Results:  
All seperators are deleted and remaining items sorted.

Expected Results:  
Bookmarks with the seperators and sorted items between two seperators. Look at
details desription for example.

Updated

13 years ago
Summary: Sort by Name should not delete seperators but sort items between two seperators → Sort by Name should not delete separators, but sort items between two separators
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.7.3) Gecko/20040930
Firefox/0.10

Confirming

->NEW


related bugs:
bug 258692
and a dupe of bug 211023#c53 (a comment that was simply ignored)
Status: UNCONFIRMED → NEW
Ever confirmed: true
*** Bug 270586 has been marked as a duplicate of this bug. ***
*** Bug 270986 has been marked as a duplicate of this bug. ***
Assignee: vladimir → vladimir+bm
(Reporter)

Updated

13 years ago
Flags: blocking-aviary1.1?
*** Bug 281215 has been marked as a duplicate of this bug. ***
The previous dupe mentions possible dataloss when undoing sort.

Comment 6

13 years ago
If you ctrl-z to undo after the separator is deleted, the separator returns 
and overwrites the bookmark/folder in its previous space.

Updated

13 years ago
Whiteboard: [asaP2]
nice to have, probably better to wait for the bookmarks rewrite so we can do
this right.
Flags: blocking-aviary1.1? → blocking-aviary1.1-
*** Bug 310809 has been marked as a duplicate of this bug. ***
Summary: Sort by Name should not delete separators, but sort items between two separators → Sort by Name should not remove/delete separators, but sort items between two seperators
Assignee: vladimir+bm → nobody
sorry for bugspam, long-overdue mass reassign of ancient QA contact bugs, filter on "beltznerLovesGoats" to get rid of this mass change
QA Contact: mconnor → bookmarks

Updated

11 years ago
Flags: blocking-firefox3?
Blocking
--> Places, which might do this already, at which point they can close it. :)
Component: Bookmarks → Places
Flags: blocking-firefox3? → blocking-firefox3+
QA Contact: bookmarks → places
Duplicate of this bug: 380717

Updated

10 years ago
Target Milestone: --- → Firefox 3 alpha6

Updated

10 years ago
Assignee: nobody → cyen

Comment 12

10 years ago
In the current Firefox 3 trunk:
Sorting bookmarks by any criteria in the Bookmarks Manager sets all separators to invisible, then sorts all contents by bookmark/folder name.

The problem preventing this from being a quick fix to achieve the behavior listed in Comment #0 is that in the _buildVisibleList ( http://mxr.mozilla.org/seamonkey/source/browser/components/places/content/treeView.js#166 ) function, you can see that it makes a query (line 183, with the asContainer(rootNode) method) to a nsNavHistoryContainerResultNode. The node then figures out what the sort comparator should be from the UI, and eventually calls nsNavHistoryContainerResultNode::RecursiveSort ( http://mxr.mozilla.org/seamonkey/source/toolkit/components/places/src/nsNavHistoryResult.cpp#615 ).

Separators are not currently included in the sorted list for the following reasons:

1) With little data associated with each separator, the SortComparator used to sort a list of the node's children would be ineffective. Firefox 2 doesn't hide separators while sorting; as a result, separators are often bunched together or scattered randomly through the sorted bookmark list.

2) The way RecursiveSort is written, groups of bookmarks separated by separators would have to be recognized as separate containers to be sorted independently. I can think of two ways to do that off the top of my head -- one, a separator would have to somehow act as a container for all of the bookmarks below it to fool RecursiveSort. A second way would be... splitting the root node's children into individual nsNavHistoryContainerResultNodes by separators, then somehow merging each of the new ns..Nodes into the root node again.

Another problem Seth saw when we were discussing this is if, while viewing a list of bookmarks correctly separated by separators as in Comment #0, what happens when a user tries to insert a separator in the middle of the bookmarks.

Firefox 2 shows bookmarks while sorted, but the separators (IIRC from testing 20 minutes ago) just get bunched together and that behavior doesn't make much more sense than separators disappearing.

One other alternative I'd like to suggest -- keeping the trunk behavior of hiding separators while sorted, but also instead preventing users from adding a new separator unless they're in the unsorted mode. Plus, then there's an additional visual cue (the "New Separator" button greying out) upon sorting.

Comment 13

10 years ago
> One other alternative I'd like to suggest -- keeping the trunk behavior of
> hiding separators while sorted, but also instead preventing users from adding a
> new separator unless they're in the unsorted mode. Plus, then there's an
> additional visual cue (the "New Separator" button greying out) upon sorting.
> 

See Bug #333758
another angle:

I am wondering if sorting in the bookmark organizer dialog should not show containers (like we exclude separators) and only show bookmark items (and livemark items) when sorted.

I'm trying to think of another UI where sorting sorts within containers, yet keeps the containers themselves in the hierarchical order.

one analogy is our history sidebar.  you can have containers (view by date), but when you sort (last visited) the containers go away.

another UI that hides container on sorting is thread panes in mail readers.  there are containers when in threaded mode, but when you sort, the containers go away.

you could argue:  we should show containers when sorting, and just sort them as well.  one problem with that is if you delete a container when sorted, you may not realize that you are deleting all of its children as well.

Perhaps this is just food for thought for the places organizer UI redesign that is going on in d.a.firefox.
another way is I'm proposing sorting to be a search that returns all bookmarks for the selected container, and then we sort it based on the sorting mode.

and the commands would be enabled / disabled like the should be when searching over a container.
I think the current behavior is fine: choosing unsorted shows the separators, and sorting hides them.

Updated

10 years ago
Target Milestone: Firefox 3 alpha6 → ---
based on alex's comment #16, I think the next step will be to mark this WONTFIX.

I still think there might be value in not showing containers at all when sorted (see comment #14 - #15), but I'll spin off a new bug about that.
Assignee: cyen → nobody
from christine's comment #12:

"Firefox 2 shows separators while sorted, but the separators (IIRC from testing
20 minutes ago) just get bunched together and that behavior doesn't make much
more sense than separators disappearing."
OK, I'm a little confused.

Comment 0 is talking about the "Sort by Name" function which isn't a view mode for bookmarks, but rather a permanent data manipulation action that changes the order of the bookmarks as they exist in the bookmarks menu.

Yet, comment 12 and onward seems to imply that "Sort by Name" is a mode which can be turned on and off when the user wishes to see their bookmarks in alphabetical order. (This, fwiw, is not the behaviour I see when using my Minefield build, but maybe certain changes have yet to land on trunk?)

Separators are only explicitly added by users as an organizational mechanism, and I think it's fairly safe to assume that they are added to indicate some form of semantic split. Similarly, "Sort by Name" wasn't originally meant to be a view option (like "Keep arranged by" or "Sort by") , but rather a way for someone to have the computer do what computers are good at reorder the hodge-podge that was inadvertantly created by adding bookmarks over time.

Now, this bug is asking that the "Sort by Name" function observe the semantic dividers implied by separators, which I think is a laudable goal and not at all a WONTFIX. For types like my lovely wife, who spend hours dreaming up new and exciting organization schemes, it's a fantastic tool.

Creating a view-mode where we view would be a seperate bug, IMO.

Updated

10 years ago
Target Milestone: --- → Firefox 3 beta1
To resummarize:

Given a folder as follows:

--------------------
Why Programs Crash
Things That Go Boom
--------------------
Nutty Things on Google Video
Crazy Things On YouTube
--------------------
My Buglist
Seth's Buglist
--------------------

If you right click on the folder, or on a non-folder child, we currently get: 

--------------------
Crazy Things On YouTube
My Buglist
Nutty Things on Google Video
Seth's Buglist
Things That Go Boom
Why Programs Crash
--------------------

Separators are removed, all contents sorted in a single list.

But what we really want is:

--------------------
Things That Go Boom
Why Programs Crash
--------------------
Crazy Things On YouTube
Nutty Things on Google Video
--------------------
My Buglist
Seth's Buglist
--------------------

Separators are maintained, and the items between the separators are sorted as distinct groups.
mconnor / beltzer:  thanks for clarifying.  

I went off into the weeds there on Comment #12 - #18, sorry about that.

This bug is about what "Sort by Name" does to the contents.

follow up comments / quetsions:

1)  it seems like that command, like insert separator, should only be enabled when you are not sorted.  (menus are not sorted, so the command is enabled and in the organized bookmark dialog, the "right pane" can be sorted.   when sorted, I think the "sort by name command" should be disabled)

2)  Should we have "Edit | Sort By Name" menu item?

3)  Is it confusing to have "Sort by Name" but also "View | Sort by Title"?
(In reply to comment #21)
> 1)  it seems like that command, like insert separator, should only be enabled
> when you are not sorted.  (menus are not sorted, so the command is enabled and
> in the organized bookmark dialog, the "right pane" can be sorted.   when
> sorted, I think the "sort by name command" should be disabled)

Since the sorting doesn't require user intervention and would be well defined, it would not be necessary to disable it (contrary to the separator insertion, which is impossible since the user can't make a decision about it's placement when using a sorted view). However, the idea might be good as to avoid the risk of accidentally sorting  by name and not getting visual feedback of the mistake in order to promptly act upon the mistake.


> 2)  Should we have "Edit | Sort By Name" menu item?

Unless there are ambiguities, I would find it harmless (though being used to taking advantage of context menus, I cannot speak for its usefulness, except for being direct left-click accessible). How would it work? Would it act as if right clicking on the active node of the left pane, or would the action be restricted to when there is a folder selected on the right pane (I don't think it should do both unless a pop-up is used). (I would go for the former.)


> 3)  Is it confusing to have "Sort by Name" but also "View | Sort by Title"?

If you mean "View | Sorted by Title", in Firefox 2.0.0.x it is called "View | Sorted by Name", which is more coherent: either "title" or "name" should be used, but not both.
Created attachment 270238 [details] [diff] [review]
patch
Assignee: nobody → mano
Status: NEW → ASSIGNED
Attachment #270238 - Flags: ui-review?(mconnor)
Attachment #270238 - Flags: review?(sspitzer)
OS: Windows XP → All
Priority: -- → P3
Hardware: PC → All
Comment on attachment 270238 [details] [diff] [review]
patch

this doesn't work, and at first glance, I don't see how it could.

I think you need to take items into sub arrays (split by seps) and then sort using the existing sort function.

note, extra ";" on this line, too:

+             this._view.getResult().sortingMode ==
+                 Ci.nsINavHistoryQueryOptions.SORT_BY_NONE;;
Attachment #270238 - Flags: review?(sspitzer) → review-
Created attachment 270261 [details] [diff] [review]
patch
Attachment #270238 - Attachment is obsolete: true
Attachment #270261 - Flags: ui-review?(mconnor)
Attachment #270261 - Flags: review?(sspitzer)
Attachment #270238 - Flags: ui-review?(mconnor)
Comment on attachment 270261 [details] [diff] [review]
patch

I think you re-attached the first patch again.
Attachment #270261 - Flags: ui-review?(mconnor)
Attachment #270261 - Flags: review?(sspitzer)
>> 3)  Is it confusing to have "Sort by Name" but also "View | Sort by Title"?
>
> If you mean "View | Sorted by Title", in Firefox 2.0.0.x it is called "View |
> Sorted by Name", which is more coherent: either "title" or "name" should be
> used, but not both.

spun out to bug #386287
Created attachment 270378 [details] [diff] [review]
patch
Attachment #270261 - Attachment is obsolete: true
Attachment #270378 - Flags: review?(sspitzer)
Comment on attachment 270378 [details] [diff] [review]
patch

r=sspitzer
Attachment #270378 - Flags: review?(sspitzer) → review+
Summary: Sort by Name should not remove/delete separators, but sort items between two seperators → Sort by Name should sort between seperators
mozilla/browser/components/places/content/controller.js 1.164
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Duplicate of this bug: 333592

Updated

10 years ago
Summary: Sort by Name should sort between seperators → Sort by Name should sort between separators
Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.