420003 - Sort by name on tag container produces unexpected behavior

Status: VERIFIED FIXED

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

Description

[Damian Shaw [Quan]]

Attachment: Before and after of the error

Latest trunk build as I post this:

1) Click "Smart Bookmarks" in bookmark toolbar
2) Go to recent tags
3) Right click on a tag container and sort by name
4) View horrific result

Restart restores titles...

Comment 1

[Damian Shaw [Quan]]

Just test on Linux, exactly the same problem, so I assume it's fairly safe to say it's cross platform.

Comment 2

[Ondrej Brablc]

Order of query based queries should not be modifiable. The solution for this bugs is to disable this command.

Comment 3

[Ondrej Brablc]

(In reply to comment #2)
> Order of query based queries should not be modifiable. The solution for this
> bugs is to disable this command.
> 

We have 3 places where we can see tag containers.

A) Bookmark Toolbar -> Smart Bookmarks -> Recent Tags
B) Places Library -> All Bookmarks (and the same as above)
C) Places Library -> Tags

The set of commands seems to be the same for A and C, with the same result when sorting (fortunately titles are not really deleted, just nulled in memory). In case B they are not modifiable.

But there are more problems, it seems strange to me, that it is allowed to create a new folder or new bookmark inside of tag container. When I right click on the right side I can even create a separator. Creating a new bookmark this way automatically shows as if it was tagged this way (what seems correct).

My suggestion for this bug:

- disallow adding new content to the tag containers everywhere,
- fix sort by name to work properly (probably only annotation versus bookmark title problem).

Comment 4

[Ondrej Brablc]

The problem of empty titles is caused by wrong query in mDBBookmarkToUrlResult (nsNavHistory.cpp). It does not perform the same COALESCE on places title as mDBGetChildren does (this may get fixed by bug 419731, but I decouple those two for a moment). 

Another problem is sorting. Sort by name actually works - it changes the position of included links, however, the bookmark query that displays them in "Smart Bookmarks" sorts by SORT_BY_DATE_DESCENDING. Sorting will be removed to use the original one. Unfortunately this will not solve the sorting problem due to bug 359332.

Comment 5

[Ondrej Brablc]

Attachment: Fix disappearing titles

This does not solve the bug completely, but it solves the scarifying problem of disappearing titles. The fact that sort order is not visible should be solved with bug 359332 (which I will nominate to block Fx3).

Comment 6

[Dietrich Ayala (:dietrich)]

Comment on attachment 307260 [details]
Fix disappearing titles

r=me. please note on bug 359332 that the sorting options removed here need to be re-added when it gets fixed.

Comment 7

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.267; previous revision: 1.266
done

Comment 8

[Tracy Walker [:tracy]]

verified with: 
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b5pre)
Gecko/2008032106 Minefield/3.0b5pre

Comment 9

[Damian Shaw [Quan]]

The current behavior for this is even worse, currently all bookmarks are removed!

But given that tag behavior is changing, I'm not sure what can be done about this at the moment, see bug 419731.

Comment 10

[Damian Shaw [Quan]]

Sorry, I'd like to update the behavior here.

All the bookmarks aren't removed when sorting by name, just 1 of them. However when I tested this there was only 1 bookmark in that selected tag, so it got removed, tested on:

Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9b5pre) Gecko/2008032106 Minefield/3.0b5pre ID:2008032106

Comment 11

[Tracy Walker [:tracy]]

I didn't see that on Win XP when I verified this bug.  

Please file a new bug.

Comment 12

[Damian Shaw [Quan]]

(In reply to comment #11)
> I didn't see that on Win XP when I verified this bug.  
> 
> Please file a new bug.
> 

Well I got someone else to confirm it on XP, but will file a new bug anyway.

Comment 13

[Damian Shaw [Quan]]

Made bug 424648

Please try confirming there, I can confirm 100% of the time on different computers and have got other people to confirm on different OS'.

Comment 14

[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