379986 - make bookmark folders use ids from the moz_bookmarks table

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Dietrich Ayala (:dietrich)]

breaking up bug 372508 into smaller chunks.

Comment 1

[Dietrich Ayala (:dietrich)]

Attachment: fix v5

in addition to the previous patch, this one removes moz_bookmarks_folders, and adds a folder_type column to moz_bookmarks.

Comment 2

[Asaf Romano (gone)]

What's b.fk for a folder item, and for a separator item? It would be really nice  not to have separate pre-compiled statements here.

Comment 3

[Dietrich Ayala (:dietrich)]

Attachment: fix v6

(In reply to comment #2)
> What's b.fk for a folder item, and for a separator item?

this patch explicitly sets the column's default value to null.

> It would be really
> nice  not to have separate pre-compiled statements here.
> 

it would be nice, but we don't want to have to join against moz_places just to get folder info.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 264130 [details] [diff] [review]
fix v6

s/mDBGetFolderProperties/mDBGetItemProperties/

there's a lot of cleanup to do, but let's land this as is for now so we can get the schema changes done.

Comment 5

[Dietrich Ayala (:dietrich)]

Attachment: fix v7

combines the Get*Properties statements per IRC, and changes both the statment and keys to "GetItemProperties", per Mano's previous comment.

Comment 6

[Asaf Romano (gone)]

Comment on attachment 264191 [details] [diff] [review]
fix v7

much better

Comment 7

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/public/nsINavBookmarksService.idl;
/cvsroot/mozilla/toolkit/components/places/public/nsINavBookmarksService.idl,v  <--  nsINavBookmarksService.idl
new revision: 1.36; previous revision: 1.35
done
Checking in toolkit/components/places/src/nsNavBookmarks.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavBookmarks.cpp,v  <--  nsNavBookmarks.cpp
new revision: 1.84; previous revision: 1.83
done
Checking in toolkit/components/places/src/nsNavBookmarks.h;
/cvsroot/mozilla/toolkit/components/places/src/nsNavBookmarks.h,v  <--  nsNavBookmarks.h
new revision: 1.39; previous revision: 1.38
done
Checking in toolkit/components/places/src/nsNavHistory.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v  <--  nsNavHistory.cpp
new revision: 1.121; previous revision: 1.120
done
Checking in toolkit/components/places/tests/bookmarks/test_bookmarks.js;
/cvsroot/mozilla/toolkit/components/places/tests/bookmarks/test_bookmarks.js,v  <--  test_bookmarks.js
new revision: 1.8; previous revision: 1.7
done

Comment 8

[pal-moz]

all bookmarks disappear.
cannot import from files.

regression?

Comment 9

[Dietrich Ayala (:dietrich)]

(In reply to comment #8)
> all bookmarks disappear.
> cannot import from files.
> 
> regression?
> 

hi pal-moz,

can you please provide more information? i've tested with a new profile, and upgrading from non-places, and both worked without error. can you use bookmarks after that point? is there anything in the js console?

Comment 10

[pal-moz]

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a5pre) Gecko/2007050821 Minefield/3.0a5pre [places-bookmarks]

test again, with creating new profile.
cannot reproduce now.
I don't know why.

last time, use hourly (maybe 050820) with existing/old profile.
then bookmarks disappear, and cannot import.

Comment 11

[Jim Jeffery not reading bug-mail 1/2/11]

From reading https://bugzilla.mozilla.org/show_bug.cgi?id=379986#c10 appears this may have broken Import, and folders. 

Comment 12

[Jim Jeffery not reading bug-mail 1/2/11]

posted to wrong bug - Bah!

Comment 13

[(not reading, please use [e-mail] instead)]

note, after this change we are no longer creating the moz_bookmarks_folders table, but in migration we are still attempting to drop it.  see bug #380801 for a related issue.

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