477739 - [PPC] Bookmarks root folder gets deleted when removing a folder [@ nsINavBookmarksService.getFolderIdForItem]

Status: VERIFIED FIXED

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

Description

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

Attachment: Broken places.sqlite

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b3pre) Gecko/20090209 Shiretoko/3.1b3pre Ubiquity/0.1.5 ID:20090209020514

Marcia showed me a serious problem today while sitting together in the QA lab. There is no way to edit a bookmark while clicking the star or pressing Cmd+D. The following exception is thrown:

Error: uncaught exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsINavBookmarksService.getFolderIdForItem]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: file:///Applications/Minefield.app/Contents/MacOS/modules/utils.js :: anonymous :: line 877"  data: no]

After digging into this problem I've noticed that we have a broken places.sqlite file. The folder hierarchy is infinite. Means opening the unsorted bookmarks always displays the bookmarks root folder with all its sub-folders again. Further this also results in broken JSON backups. All backups which were made the last days have a filesize of zero. All bookmarks will be lost if you have the need to restore them.

I'll attach the affected places.sqlite file here, so it is easier to reproduce. The problem is that we don't have an idea how it gets broken. Even the steps are not known how to get into this problem. Sadly. :/ The profile was created with Shiretoko a month ago. So we don't suffer from an older bug, which has been already fixed.

Marco and Dietrich, could you have a look at this places.sqlite file?

Comment 1

[Scott]

Experiencing the problem with Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.2a1pre) Gecko/20090209 Minefield/3.2a1pre - Build ID: 20090209020504 as well as in Firefox 3.06. 
I thought the problem may have been related to syncing my bookmarks with Foxmarks so I disabled Foxmarks, deleted the places.sqlite file and tried to restore the bookmarks which is when I discovered that the backups had a filesize of zero. I exported an html file from Foxmarks and imported it. The next day the infinite heirarchy had returned. I installed the extension SyncPlaces http://www.andyhalford.com/syncplaces/index.html as a replacement for Foxmarks. The extension creates a backup before syncing. When I tried to use it, I got an error "could not save bookmarks". 

I created a new profile, went to some random sites, bookmarked them, then attempted to export the json file. The file had a 0 kb size. After 2 days the bookmark library is not showing an infinite hierarchy and I can still use the star to bookmark sites.

With both the old and new profile, when I tried to export the JSON file, the error console showed the message  "Error: uncaught exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsINavHistoryService.executeQuery]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: file:///Applications/Firefox3.app/Contents/MacOS/modules/utils.js :: PU_backupBookmarksToFile :: line 1554"  data: no]"

Comment 2

[Marco Bonardo [:mak]]

nice, the database is missing the root node (id = 1).
the fact that all roots are now children of unsorted is due to preventive maintenance, i'll add a check there to be sure we don't move roots.

Hwv the fact root is missing is quite scary and i have not yet a reason for that.

Comment 3

[Marco Bonardo [:mak]]

filed bug 477793 to avoid making more damages with preventive maintenance.

Still investigating on possible causes of losing the root node.

Comment 4

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

Marco, is there a simple way to make the database work again without losing anything? I think that Scott would be really interested in.

Comment 5

[Marco Bonardo [:mak]]

Attachment: wallpaper patch

this is only a wallpaper we could take in the meantime, if we want to.

Comment 6

[Marco Bonardo [:mak]]

(In reply to comment #4)
> Marco, is there a simple way to make the database work again without losing
> anything? I think that Scott would be really interested in.

these queries should restore the root:
INSERT INTO moz_bookmarks (id, type, fk, parent, position, title) VALUES (1, 2, NULL, 0, 0, NULL);
UPDATE moz_bookmarks SET parent = 1 WHERE id IN (SELECT folder_id FROM moz_bookmarks_roots);

Comment 7

[Shawn Wilsher :sdwilsh]

Comment on attachment 361520 [details] [diff] [review]
wallpaper patch

This would only abort in debug builds.

Comment 8

[Marcia Knous [:marcia]]

The funny thing about this bug is I never saw it on any of my other machines - only manifested itself on the PPC machine in the lab.  And it seems the only other person that saw it was also on PPC.  Sadly I also have no idea how I got into that state on that Mac.

Comment 9

[Marco Bonardo [:mak]]

that makes me think something is using wrong endianness and trying to remove itemId 1 instead of a valid itemId...

Comment 10

[Marco Bonardo [:mak]]

with dependancies fixed is no more possible to usa an API to remove the root node, nor preventive maintenance will move roots even if they are wrong.

Based on previous comments we need to investigate where an endianness change could cause such a thing, unless someone could reproduce this on an x86 machine.
Sqlite by itself is not dependant on 32bit/64bit/little-endian/big-endian changes.

Comment 11

[Marco Bonardo [:mak]]

Attachment: patch v1.0

OMG, this is there from some time, we could have a bunch of ppc profiles missing the root.
I'll probably also add a preventive task to fix them.

Comment 12

[Marco Bonardo [:mak]]

notice what happens is that we delete WHERE parent = 0 (exactly the root), preventive maintenance has made this issue visible trying to fix the orphan roots.

Comment 13

[Marco Bonardo [:mak]]

Also notice the test change above won't do anything more than a sanity check atm, since we don't have a ppc unit test box, Ted told me there's a bug around asking to setup one, when one will exist the test will catch this.

Comment 14

[Marco Bonardo [:mak]]

Attachment: patch v2.0

- added a preventive task to fix existing broken profiles, with a test. Detects if the root is missing and creates a new one.
- removed dead code i found in initRoots while looking how it was performing, for 3.2 i would change this function to remove old migration code and make createRoot more nitpicky on root's folders existance.
- changed code to use BindInt64Parameter, looks less error prone than the nsPrintfCString.

I think this is a P1 though, so i'd like to push this, or a subset of this, today, if possible.

Comment 15

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

Adding bug 418643 to the dependency list which has been introduced this problem 3 month ago.

Comment 16

[Dietrich Ayala (:dietrich)]

Comment on attachment 361767 [details] [diff] [review]
patch v2.0

talked over irc, r=me. thanks for quick action here.

Comment 17

[Marco Bonardo [:mak]]

filed Bug 478034 and Bug 478035 as 3.2 follow-ups

Comment 18

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/19520d065520

Comment 19

[Marco Bonardo [:mak]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/47331aa58d63

Comment 20

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

Marco, as you mentioned on IRC I should watch the id of the bookmarks root folder inside moz_bookmarks. As what I can see is that there is none entry with id 1 at all. Even after creating a new profile. Do we also fail in creating the database here? Could you please cross-check? If yes, I would file a new bug on that.

Comment 21

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

Filed bug 478258 for that.

Comment 22

[Scott]

1)  Will patch repair broken places.sqlite or only prevent place from breaking?
2)  Is patch in 2/13 build or will it be in 2/14?

Comment 23

[Marco Bonardo [:mak]]

it is in 2/13. the repair happens on idle, so you should leave your browser open for some time on first opening (so let's say tomorrow you open your browser and leave it open without touching it for one hour).
We have a patch on trunk to make that happen before, but it's not in 1.9.1, In future we could provide some way of forcing a check from the user.

Comment 24

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

Verified fixed on trunk and 1.9.1 branch with the following builds:

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.1b3pre) Gecko/20090215 Shiretoko/3.1b3pre ID:20090215020429

Mozilla/5.0 (Macintosh; U; PPC Mac OS X 10.4; en-US; rv:1.9.2a1pre) Gecko/20090215 Minefield/3.2a1pre

I've triggered the maintenance mode manually by using the following command in the Error Console:

Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm"); PlacesDBUtils.maintenanceOnIdle();

Scott, you can do this too. Just open the Error Console, put the above line in the code textbox and click on Evaluate. After that step your places.sqlite file will contain the root folder again.

Comment 25

[Scott]

Let Firefox idle for about 2 hours. Bookmark star and export now working but the duplicate bookmarks remain. Perhps a large bookmark file reques more time?

Comment 26

[Marco Bonardo [:mak]]

which duplicate bookmarks?

Comment 27

[Scott]

The infinite folder hierarchy where inside unsorted bookmarks the whole bookmark hierarchy is repeated including another unsorted booksmarks folder with another hierarchy, etc. etc.

Comment 28

[Marco Bonardo [:mak]]

did you execute comment 6 (don't do that) before the idle?
Can you try do what henrik suggests in comment 24?

Open the error console and eval:
Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");
PlacesDBUtils.maintenanceOnIdle();

Comment 29

[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