in nsBrowserGlue.js, when automatically importing from bookmarks html format, use bookmarks.postplaces.html, if it exists (otherwise, defer to bookmarks.html) the reason is if places.sqlite gets corrupted, or we do a schema migration (before we have json), we should use bookmarks.postplaces.html if it exists. That file, if it exists, is more up to date than bookmarks.html

note: when we fix this, we should also fix the fix for bug #406094. when recreating from bookmarks.postplaces.html, we should not reset the browser.places.createdDefaultQueries pref, as if it is set, bookmarks.postplaces.html will already have the special folder, and we'd end up with duplicates.

as with bug #406094, this is important for the places.sqlite corruption, migration and the case where the user (right now, power user / nightly tester) decides to remove places.sqlite manually fix in hand.

here are some test cases: note, by default, browser.bookmarks.overwrite is false (as we don't overwrite fx 2's bookmarks.html. instead we write to bookmarks.postplaces.html) with browser.bookmarks.overwrite to false (which is the default) 1) create a profile using fx2, add some bookmarks, exit. migrate to fx 3, add some bookmarks. notice you have the "Smart Bookmarks" folder. exit. for the purpose of testing this bug, when adding bookmarks, use the personal toolbar (as the "unfiled" root is not exported yet.) remove places.sqlite, we should restore (as best we can, given that not everything is in the html format, bugs are logged on this), and you should see the bookmarks you created in fx 3 and you will have the "Smart Bookmarks" folder, as it was in bookmarks.postplaces.html. You should not get a duplicate "Smart Bookmarks" folder. 2) create a profile using fx2, add some bookmarks, exit. migrate to fx 3, add some bookmarks. notice you have the "Smart Bookmarks" folder. exit. for the purpose of testing this bug, when adding bookmarks, use the personal toolbar (as the "unfiled" root is not exported yet.) remove places.sqlite and bookmarks.postplaces.html, we should restore (as best we can, given that not everything is in the html format, bugs are logged on this), and you should see the bookmarks you created in fx 2, but not fx 3. note, we will not recreate the "Smart Bookmarks" folder.

note to dietrich, the change to nsNavHistory.cpp is what I mentioned in comment #1

Comment on attachment 291578 [details] [diff] [review] patch this looks ok. we should hammer on these scenarios on the testday on friday. r=me.

this regressed ("use the right bookmarks.html file when places.sqlite is corrupt or removed") when I fixed bug #381216

updating summary. fixed. Checking in browser/components/nsBrowserGlue.js; /cvsroot/mozilla/browser/components/nsBrowserGlue.js,v <-- nsBrowserGlue.js new revision: 1.45; previous revision: 1.44 done Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.206; previous revision: 1.205 done

if (overwriteBookmarks) { rv = prefs->SetBoolPref(PREF_BROWSER_CREATEDSMARTBOOKMARKS, PR_FALSE); NS_ENSURE_SUCCESS(rv, rv); } this is true if we don't have places.sqlite and we have bookmarks.postplaces, but if we don't have any of them, we should however create the smart bookmarks, while this now depends on overwriteBookmarks value. there should be a check if bookmarks.postplaces.html exists like (pseudocode) if (overwriteBookmarks || !file_exists("bookmarks.postplaces.html"))

backing out to test Ts impact Checking in browser/components/nsBrowserGlue.js; /cvsroot/mozilla/browser/components/nsBrowserGlue.js,v <-- nsBrowserGlue.js new revision: 1.46; previous revision: 1.45 done Checking in toolkit/components/places/src/nsNavHistory.cpp; /cvsroot/mozilla/toolkit/components/places/src/nsNavHistory.cpp,v <-- nsNavHistory.cpp new revision: 1.210; previous revision: 1.209 done

This had a 14 ms impact on Ts. here's what I'm not understanding yet: for an existing profile, browser.places.importBookmarksHTML would be false, so we shouldn't be hitting the code I added to nsBrowserGlue.js unless places.sqlite was corrupt or non-existent. See http://lxr.mozilla.org/seamonkey/source/browser/components/nsBrowserGlue.js#350 for where we return early. Sorry for asking this about the Ts test, but is it a new profile every time? I think Ts kills firefox.exe, but I don't think that should corrupt places.sqlite. browser.places.importBookmarksHTML starts off as true. So the first Ts should be slow. I wonder if for Ts we a start with a new profile, import bookmarks.html, kill firefox, before the pref change is flushed to disk. This would mean that each time we ran nsBrowserGlue.js for Ts, we would be executing a bunch of code we don't need to measure (as it is first time only import code.) I'l see if I can verify this theory locally.

This also seems to have affected Txul on Linux. It jumped from 212 to 220, when checked in, and dropped again when backed out. http://build-graphs.mozilla.org/graph/query.cgi?tbox=bl-bldlnx03_fx-linux-tbox-head&testname=xulwinopen&autoscale=1&size=&units=ms&ltype=&points=&showpoint=2007%3A12%3A05%3A14%3A57%3A32%2C212&avg=1&days=1

mconnor points out that once we have .json export / import, restoring after removing / corrupting places.sqlite should be using the .json file, not bookmarks.postplaces.html. So this bug is really a wontfix, if we get the .json stuff.

So it might be a wontfix once json is done but right now not having this is causing dataloss.

And this isn't really a wontfix unless the json stuff actually fixes it, which the initial patch there doesn't look like it does.

Should this still be assigned to Seth ?

mh should be fixed in JSON, unassigning. change back if that is not correct.

bug 384370 causes initial import to be done from the latest json backup first, and if that doesn't exist, then bookmarks.html. bookmarks.postplaces.html is no longer exported. -> fixed.

The bug is out of the scope of the library subgroup in FFT, so setting in-litmus flag- .

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