400474 - Bookmarks menu no longer groups folders before bookmarks after "Sort by Name" is performed.

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P4)

Description

[RenegadeX]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.7) Gecko/20070914 Firefox/2.0.0.7
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101905 Minefield/3.0a9pre

Performing a Bookmarks Menu 'Sort by Name' in Firefox 3.0a9pre currently sorts everything alphabetically, intermixing bookmark items and folders. This is a change from the way it was previously.

Reproducible: Always

Actual Results:  
ex of Firefox 3.0a9pre bookmarks menu sorted by 'Sort By Name':
- [Accounting]->
- Firefox start page
- [Golf]->
- Mozillazine forums
- [News sites]->
- YouTube

Expected Results:  
Folders are sorted alphabetically and grouped above sorted individual bookmarks.

ex of Firefox 2.x (and before) bookmarks menu sorted by 'Sort By Name':
- [Accounting]->
- [Golf]->
- [News sites]->
- Firefox start page
- Mozillazine forums
- YouTube

Was there a reason for this change?
If it wasn't intended (and I hope not), was it possibly broken here: https://bugzilla.mozilla.org/show_bug.cgi?id=325342 ??

I came across a similar bug, Bug 379560 https://bugzilla.mozilla.org/show_bug.cgi?id=379560 filed in May, but that filed on 2.0.0.3, which I don't believe was really affected. In fact, it was  marked a duplicate of Bug 288260, https://bugzilla.mozilla.org/show_bug.cgi?id=288260 which refers to sorting in the Bookmarks Manager view.

Comment 1

[Ria Klaassen (not reading all bugmail)]

I see this already in a build of 23 May 2007 so I think this was indeed caused by bug 325342. Confirming, because I can't imagine that this was intended, and because I don't see this bug in the dependencies field of bug 325342 so I assume this is not a duplicate. 

Comment 2

[Gabriel Chadwick]

This is a regression and is major bug in sorting... Should not be a P4 IMHO !

Comment 4

[Marco Bonardo [:mak]]

Attachment: put folders before

make sorting put folders before bookmarks

Comment 5

[Asaf Romano (gone)]

Comment on attachment 296119 [details] [diff] [review]
put folders before

yeah, that's what Fx2 seems to do:
http://lxr.mozilla.org/mozilla1.8/source/browser/components/bookmarks/content/bookmarks.js#1080

r=mano.

Comment 6

[Asaf Romano (gone)]

Comment on attachment 296119 [details] [diff] [review]
put folders before

actually, let's make this check nodeIsContainer so saved searches are treaded in the same manner.

Comment 7

[Marco Bonardo [:mak]]

Attachment: put containers before

changed folders to containers

Comment 8

[Asaf Romano (gone)]

Comment on attachment 296162 [details] [diff] [review]
put containers before

r=mano, thanks.

Comment 9

[Asaf Romano (gone)]

mozilla/browser/components/places/src/nsPlacesTransactionsService.js 1.12

Comment 11

[Henrik Skupin [:whimboo][⌚️UTC+2]]

After having a talk with dietrich and tchung on IRC, I have to reopen this bug. It is not fixed. Bookmarks and folders are still mixed-up after sorting by name via "Views -> Sort -> Sort by Name".

Did a test with

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3 ID:2008020511

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008020804 Minefield/3.0b4pre

Comment 12

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: places.sqlite with broken "Bookmarks Menu" entries

As far as I can see sometimes it works and sometimes not. Here is my places.sqlite from a test profile which shows this issue for "Bookmarks Menu".

Comment 13

[Marco Bonardo [:mak]]

mh i think this should go in a different report since is not served by the transaction code, and it does not appear related to context menu "sort by name".

however i'm taking a look there

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marco, from an outer point of view this is the same issue which was described in the initial comment. I also haven't written anything sorting via the context menu. The sorting is done via the Views menu. 

Comment 15

[Marco Bonardo [:mak]]

no problem, i've a patch for this, my comment was because i did not want to mix up  the problem in context menu (That is due to transaction manager) and the problem in view -> sort menu (that is due to nsNavHistoryResult and is only a view option)

Original report is not too clear on that.

i'll attach soon after some more testing. Thank you

Comment 16

[Marco Bonardo [:mak]]

Attachment: patch

i've added a new sorting method to nsINavHistoryQueryOptions that is SORT_BY_CONTAINERANDTITLE_*
it's mostly using actual SORT_BY_TITLE_* comparator, but putting folders before, then i've changed the treeView to use that comparator (this way i've not touched actual SORT_BY_TITLE since third party users could want the normal alphabetic behaviour)

Comment 17

[Asaf Romano (gone)]

Please, one issue per bug report - sort by name has nothing to do with tree sorting.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mano, I didn't talk about tree sorting myself. And meanwhile I'm really confused about what issue this bug report is really about. For my case the 'Bookmarks Menu' items within the main menu are *not* grouped by folders before bookmarks. In my understanding this bug is about this issue.

Comment 19

[Marco Bonardo [:mak]]

sorry i'm misunderstanding your request...

you said "Bookmarks and folders are still mixed-up after sorting by name
via "Views -> Sort -> Sort by Name"
Views -> Sort options are relative to Views in the Library while now you speak about "'Bookmarks Menu' items within the main menu"... Views -> sort has no effect on the "menu" dropdown...

Comment 20

[Marco Bonardo [:mak]]

i'm moving my patch to Bug 416555

Comment 21

[Asaf Romano (gone)]

This bug was about the context menu item "Sort By Name" afaict.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, so lets merge the information... As long as the context menu entry doesn't work for folders directly located under "All Bookmarks" (bug 416468) I did a check with sub folder. Sort by name works fine with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3) Gecko/2008020511 Firefox/3.0b3 ID:2008020511 => Verified. Resetting TM to beta3.

I'll file a new bug for the "Bookmarks" menu directly. There the folders and bookmarks aren't sorted by name, which is a regression.

Comment 23

[Aakash Desai [:aakashd]]

The issue is still being discussed in bug416555 referenced by Mark Bonardo. So, changing the in-litmus flag to "-" and adding a "?" to the new bug.

Comment 24

[Gervase Markham [:gerv]]

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