1093623 - Bookmarks are lost when upgrade 2.31 from 2.30

Status: RESOLVED DUPLICATE of bug 942937

(SeaMonkey :: Bookmarks & History, defect)

Description

[Alice0775 White]

Steps To Reproduce:
1. Download 2.31b1 from http://ftp.mozilla.org/pub/mozilla.org/seamonkey/releases/2.31b1/win32/en-US/ and install
2. Start  Seamonkey 2.31b1 with existing profile

Actual Results:
Bookmarks are initialized.
User added bookmarks are lost..

Comment 1

[rsx11m]

Not blocking development, but dataloss anyway.

No problems here going from 2.30 beta 2 to 2.31 beta 1, all bookmarks remain unharmed. This sounds familiar though, I'm recalling others reporting loss of bookmarks when updating to 2.30 or even 2.29 already, so this may be a duplicate.

Comment 2

[therube]

Confirmed.

My first attempt was WFM.
I simply opened an existing Profile using 2.31b1, & that worked.

Quit.

Emptied that Profile directory, entirely.
Copied over existing Profile places.sqlite - only.

Open SeaMonkey 2.31b1.
What were existing bookmarks were no longer there, only the (I suppose it is) default set displayed.

> User added bookmarks are lost.

Adding new bookmarks did look to me to be WFM.

Not sure if these are related:

> Failed to load native module at path 'C:\TMP\SEA\SM2.31b1\X\components\xpcomsample.dll':
> (80520012) <unknown; can't get error from NSPR>


> Error: DEPRECATION WARNING: PlacesBackups.getMostRecent is deprecated and will be removed in a future version
> You may find more details about this deprecation at: https://bugzilla.mozilla.org/show_bug.cgi?id=859695
> resource://gre/modules/PlacesBackups.jsm 263 PB_getMostRecent
> resource://gre/components/nsSuiteGlue.js 723 SuiteGlue.prototype._initPlaces
> resource://gre/components/nsSuiteGlue.js 199 SuiteGlue.prototype.observe
> null 0 null
>
> Source File: resource:///modules/Deprecated.jsm
> Line: 79

> Error: DEPRECATION WARNING: Passing a callback to Livermarks methods is deprecated. Please use the returned promise instead.
> You may find more details about this deprecation at: https://developer.mozilla.org/docs/Mozilla/JavaScript_code_modules/Promise.jsm
> resource://gre/components/nsLivemarkService.js 354 LS_getLivemark
> chrome://communicator/content/places/browserPlacesViews.js 659 PVB_containerStateChanged
> chrome://communicator/content/places/browserPlacesViews.js 75 PlacesViewBase.prototype.result
> chrome://communicator/content/places/browserPlacesViews.js 49 PlacesViewBase.prototype.place
> chrome://communicator/content/places/browserPlacesViews.js 13 PlacesViewBase
> chrome://communicator/content/places/browserPlacesViews.js 857 PlacesToolbar
> chrome://communicator/content/bookmarks/browser-places.js 847 PTH_init
> chrome://navigator/content/navigator.js 664 Startup
> chrome://navigator/content/navigator.xul 1 onload
> null 0 null
>
> Source File: resource:///modules/Deprecated.jsm
> Line: 79

> Error: this._view.result is null
> Source File: chrome://communicator/content/places/controller.js
> Line: 160

Comment 3

[[e-mail]]

(In reply to therube from comment #2)
> Confirmed.
> 
> My first attempt was WFM.
> I simply opened an existing Profile using 2.31b1, & that worked.
> 
> Quit.
> 
> Emptied that Profile directory, entirely.
> Copied over existing Profile places.sqlite - only.
> 
> Open SeaMonkey 2.31b1.
> What were existing bookmarks were no longer there, only the (I suppose it
> is) default set displayed.

Maybe your existing profile had some bookmarks backups and this is how your bookmarks got preserved.

Comment 4

[therube]

> Maybe your existing profile had some bookmarks backups and this is how your
> bookmarks got preserved

You're right!
How do you like that.  Never realized that on some sort of "fail", that it would automatically attempt to do a restore from bookmarkbackups/*.json, but that does seem to be the case.  (Or at least something along those lines? Or at least the existence of the [a] .json file does something that seems to avert the bug?)

New (totally empty) Profile
Copy over existing places.sqlite (only)
Open 2.31b1

Result: bookmarks lost

New (totally empty) Profile
Copy over existing places.sqlite
And copy over existing bookmarkbackups/*.json
Open 2.31b1

Result: bookmarks exist (presumably recovered from the backup ?)
 
 
BUT thinking there may be more to it then that?

Because while bookmarks are retained in the second scenario, Bug 1061672 does still apply (to me). (Or at least I am unable to manually import any of my backups, as described in the bug. Or maybe this /automated/ recovery, if that is in fact what it is doing, does work, where a /manual/ Restore of a .json fails.)

> Bug 1061672 - Import json-file from Firefox (or even from SeaMonkey) do not work

Comment 5

[Grinch]

this bug should be solved soon. Cause not all people save the places.sqlite before... I don't know :-) but the backup should work as before (I guess it was Version 2.26.1) (using the latest bookmark file at startup)

Comment 6

[therube]

This issue also occurs at times other then with an upgrade to 2.31 from 2.30.

There are times, though not always repeatable, when this can happen with SeaMonkey 2.30 alone.

New Profile
Open Profile (once is enough)
Quit

Copy an existing (SeaMonkey 2.30) places.sqlite into that new Profile

Again run SeaMonkey 2.30 with that Profile
Check Bookmarks Manager

Sometimes, perhaps most times, the existing bookmarks (from the copied in places.sqlite) are there, but /sometimes/ they will go missing.

You may have to repeat the STR a number of times, < 10 I would think, but it will occur. (Don't know how of if *-shm or *-wal may play into this, or not.)

(Typical new places.sqlite size seems to be 10 MB.  If you use a larger, say 40 MB, existing places.sqlite as the file to copy into the new Profile, when visually examining your Profile, the size difference makes makes that much easier to recognize when something has gone wrong.)

Comment 7

[therube]

I see, when the above happens, a places.sqlite.corrupt is created.

Comment 8

[therube]

On the first Quit with the new Profile, sometimes places.sqlite-shm & places.sqlite-wal remain.

If you then copy into that Profile an existing places.sqlite - from a different Profile, it /seems/ you are more likely to get places.sqlite.corrupt, therefore a truncated places.sqlite - but again, not always.

And likewise there are times when the new Profile -shm & -wal did not exist, yet you still get truncated places.sqlite.

Comment 9

[Alice0775 White]

Please read STR of comment#1.

Do not copy places.sqlite.
Just Run Seamonkey2.31 with existing profile which was used Seamonkey2.30.
Again, I never copy places.sqlite.

again, STR. and 100% reproducible.
1. Run Seamonkey2.30 and create new profile, start with the profile.
2. Add some bookmarks then quit Seamonkey
3. Start Seamonkey2.31 with the profile (repeat step3 if Seamonkey crashed)
4. Then the added bookmarks are lost.

This is a critical bug.

Comment 10

[Alice0775 White]

err
s/Please read STR of comment#1/Please read STR of comment#0/

Comment 11

[Alice0775 White]

FYI.
> 3. Start Seamonkey2.31 with the profile (repeat step3 if Seamonkey crashed)
The crash is Bug 1092810 / Bug 1057581

Comment 12

[therube]

Yes I understand your STR & yes I can duplicate.

I'm just pointing out that there was/are other steps/times that this can occur outside of an upgrade to 2.31 from 2.30.

> FYI.
> > 3. Start Seamonkey2.31 with the profile (repeat step3 if Seamonkey crashed)
> The crash is Bug 1092810 / Bug 1057581

Right.
Which is exactly why I've come across the other STR I've mentioned here, for the very reason that I want to avert running into both this bug & the startup crash bug before I actually update to 2.31.  And as I'm almost assuredly not going to avert the startup crash, I do want to know just what may or may not happen so that I'm prepared.

Comment 13

[Edmund Wong (:ewong)]

Here's what I understand from the code.

From http://mxr.mozilla.org/comm-central/source/suite/common/src/nsSuiteGlue.js#729

729    var importBookmarks = !aInitialMigrationPerformed &&
730                           (dbStatus == PlacesUtils.history.DATABASE_STATUS_CREATE ||
731                            dbStatus == PlacesUtils.history.DATABASE_STATUS_CORRUPT ||
732                            !bookmarksBackupFile);

2.30 -> 2.31 change (from what I see):

   aInitialMigrationPerformed = false
   dbStatus == PlacesUtils.history.DATABASE_STATUS_UPGRADED
   bookmarksBackupFile == null (if bookmarksbackup/ has nothing in it)
    or                 == latest json backup file  

Neil, it's my understanding that if we also checked if the database wasn't
also upgraded (which in 2.30 -> 2.31 case, it is) then we do the import; otherwise
we leave it alone.

Comment 14

[Edmund Wong (:ewong)]

Attachment: proposed patch (v1)

Comment 15

[Edmund Wong (:ewong)]

Attachment: proposed patch (v1)

Comment 16

[Grinch]

well any good news about this patch?? Hope so

Comment 17

[Philip Chee]

Comment on attachment 8534101 [details] [diff] [review]
proposed patch (v1)

There are several unit tests at
http://mxr.mozilla.org/comm-central/source/suite/common/places/tests/unit/
Do they all pass?

Comment 18

[Grinch]

so will Neill review this proposed patch (v1)??? or is this patch ok to pack this SM???

Comment 19

[Edmund Wong (:ewong)]

(In reply to Mario from comment #18)
> so will Neill review this proposed patch (v1)??? or is this patch ok to pack
> this SM???

This patch is not ok to pack into SeaMonkey because it hasn't been reviewed.
As for your first question, you'll need to wait for Neil's response.

Comment 20

[Grinch]

another half year passed without any action here :-( well I know the Seamonkey Team had soem other critial issues left to work on but... Is there a chance to get Neill back on this patch review asap?

Comment 21

[Grinch]

no other comments till yet really disapoointing

Comment 22

[Edmund Wong (:ewong)]

Been working so much on the RelEng issues for so long, this bug (and
other non-RelEng bugs) have fallen off the radar.  My apologies.

With Neil very busy (and pretty much not online), I'm asking IanN for
review instead.

Comment 23

[Ian Neal]

Comment on attachment 8534101 [details] [diff] [review]
proposed patch (v1)

[Triage Comment]
Shouldn't the comment also be adjusted to reflect the change too?
r=me with that fixed and a=me for c-c and c-a, probably needs to bake a bit before going onto c-b

Comment 24

[Frank-Rainer Grahl (:frg)]

ewong this still looks valid. Seems not to be in the tree. The code for c-c is now broken and I didn't have time yet to fix it but maybe should go into 2.49.3?

Comment 25

[Edmund Wong (:ewong)]

(In reply to Frank-Rainer Grahl (:frg) from comment #24)
> ewong this still looks valid. Seems not to be in the tree. The code for c-c
> is now broken and I didn't have time yet to fix it but maybe should go into
> 2.49.3?

Good idea.

Comment 26

[Frank-Rainer Grahl (:frg)]

Bug 942937 should take care of it for 2.57+. The proper fix would be to backport this but not sure if we should do at the late in the cycle.

Comment 27

[Frank-Rainer Grahl (:frg)]

Bug 942937 was backported for 2.53.1. Should no longer happen.