Error restoring from bookmark backup (JSON import, livemark failures)

RESOLVED WORKSFORME

Status

()

Toolkit
Places
P1
normal
RESOLVED WORKSFORME
11 months ago
10 months ago

People

(Reporter: Porcelain Mouse, Assigned: standard8)

Tracking

55 Branch
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [fxsearch])

(Reporter)

Description

11 months ago
User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170824070828

Steps to reproduce:

I exported my books marks, created a new profile, and imported from the bookmarks backup file.


Actual results:

I got a pop-up error message that said "Unable to process backup file".


Expected results:

No error.

Updated

11 months ago
Component: Untriaged → Places
Product: Firefox → Toolkit
Please can you open the Browser Console (Tools -> Web Developer -> Browser Console), and then try the import again?

There may be useful errors displayed during the import that would help us fix this.
Flags: needinfo?(porcelain_mouse)
(Reporter)

Comment 2

10 months ago
Okay, I created another new profile and tried to import again.  I had to 'continue' the script three times, but here is what got added to the console:

Exception... "Invalid livemark"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: jar:file:///usr/lib64/firefox/omni.ja!/components/nsLivemarkService.js :: getLivemark/< :: line 302"  data: no] (unknown)
	getLivemark/< jar:file:///usr/lib64/firefox/omni.ja!/components/nsLivemarkService.js:302:15
	InterpretGeneratorResume self-hosted:1272:8
	next self-hosted:1179:9
	openModalWindow jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:363:5
	openPrompt jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:578:9
	confirmEx jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:719:9
	PTV_nodeHistoryDetailsChanged chrome://browser/content/places/treeView.js:828:1
	BI_importJSONNode resource://gre/modules/BookmarkJSONUtils.jsm:480:14
	runBatched resource://gre/modules/BookmarkJSONUtils.jsm:333:19
	importFromJSON resource://gre/modules/BookmarkJSONUtils.jsm:364:7
	next self-hosted:1179:9
	onStreamComplete resource://gre/modules/BookmarkJSONUtils.jsm:204:21
TypeError: parentDocShell.getDocShellEnumerator is not a function[Learn More]  tab.js:62:23
Error: Error(s) encountered during statement execution: UNIQUE constraint failed: moz_keywords.keyword  Sqlite.jsm:819:25
[Exception... "Invalid livemark"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: jar:file:///usr/lib64/firefox/omni.ja!/components/nsLivemarkService.js :: getLivemark/< :: line 302"  data: no] (unknown)
	getLivemark/< jar:file:///usr/lib64/firefox/omni.ja!/components/nsLivemarkService.js:302:15
	InterpretGeneratorResume self-hosted:1272:8
	next self-hosted:1179:9
	openModalWindow jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:363:5
	openPrompt jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:578:9
	confirmEx jar:file:///usr/lib64/firefox/omni.ja!/components/nsPrompter.js:719:9
	PTV_nodeHistoryDetailsChanged chrome://browser/content/places/treeView.js:828:1
	BI_importJSONNode resource://gre/modules/BookmarkJSONUtils.jsm:480:14
	runBatched resource://gre/modules/BookmarkJSONUtils.jsm:333:19
	importFromJSON resource://gre/modules/BookmarkJSONUtils.jsm:364:7
	next self-hosted:1179:9
	onStreamComplete resource://gre/modules/BookmarkJSONUtils.jsm:204:21
Failed to restore bookmarks from /home/pdestefa/tmp/bookmarks-2017-09-07.json: Error: Error(s) encountered during statement execution: UNIQUE constraint failed: moz_keywords.keyword  BookmarkJSONUtils.jsm:110
	BJU_importFromFile/< resource://gre/modules/BookmarkJSONUtils.jsm:110:9
	throw self-hosted:1196:9
TypeError: parentDocShell.getDocShellEnumerator is not a function[Learn More]  tab.js:62:23
Flags: needinfo?(porcelain_mouse)

Comment 3

10 months ago
The error refers to old code, but regardless, looks like insertTree should be more tolerant towards errors.

For sure it should tolerate errors coming from handleBookmarkItemSpecialData or livemarks (if it fails converting to livemark it should remove the placeholder).

Comment 4

10 months ago
A patch here should probably be uplifted considered the recent reports we got about people unable to restore bookmark backups.
Tentatively P1, in the worst case we should uplift to 57.
Priority: -- → P1
Whiteboard: [fxsearch]

Updated

10 months ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
Assignee: nobody → standard8
Status: NEW → ASSIGNED
Summary: Error restoring from bookmark backup → Error restoring from bookmark backup (JSON import, livemark failures)
Would you be able to try this on Firefox Beta 56 (https://www.mozilla.org/firefox/channel/desktop/), or would you be willing to send me the file so that I can attempt to see what the real issue is?

A lot of the json import code has changed between 55 and 56 - I doubt this will be resolved by it, but it would certainly be a different error location if it isn't resolved.

Comment 6

10 months ago
What I find odd is that we are creating an invalid export in the first place. I suspect Porcelain Mouse created the export in 55 in an attempt to work around issues he was having in bug 1397163.
Flags: needinfo?(porcelain_mouse)
(Reporter)

Comment 7

10 months ago
(In reply to Mark Banner (:standard8) from comment #5)
> Would you be able to try this on Firefox Beta 56
> (https://www.mozilla.org/firefox/channel/desktop/), or would you be willing
> to send me the file so that I can attempt to see what the real issue is?
> 
> A lot of the json import code has changed between 55 and 56 - I doubt this
> will be resolved by it, but it would certainly be a different error location
> if it isn't resolved.

Yes, of course.  It will take me a short while to test beta, but I can do both.  Is there a way I can send you bookmarks directly?  I feel a little weird about posting them, but I will if that is the only way.  It includes my bank and a few links with site ids or usernames.

(In reply to Mark Hammond [:markh] from comment #6)
> What I find odd is that we are creating an invalid export in the first
> place. I suspect Porcelain Mouse created the export in 55 in an attempt to
> work around issues he was having in bug 1397163.

That is correct.
I've contacted Porcelain Mouse directly about the options.

I also think that the reasons we are having an issue in the first place is also important to resolve, so I'll be looking at both areas.
Flags: needinfo?(porcelain_mouse)
(Reporter)

Comment 9

10 months ago
Hi all,

Well, i tried beta-56 and I received no errors exporting and no errors importing from the new export.  I didn't even see any errors in the web console.
I've received Porcelain's files. Trying it in nightly I see 6 messages saying:

[Exception... "Invalid livemark"  nsresult: "0x80070057 (NS_ERROR_ILLEGAL_VALUE)"  location: "JS frame :: file:///Users/mark/dev/gecko/objdir-ff/dist/Nightly.app/Contents/Resources/components/nsLivemarkService.js :: getLivemark/< :: line 302"  data: no]

which comes from:

http://searchfox.org/mozilla-central/rev/51eae084534f522a502e4b808484cde8591cb502/toolkit/components/places/nsLivemarkService.js#302

I'll dig into it and the places file to see if I can figure out what's up with livemarks.
I'm now pretty sure the livemark errors aren't the problem here. If I open the bookmark library window, then select the History display and do the import, there's no livemark errors.

I have added some debug, and from what I've seen, a request to get a livemark goes in, it fails, and then it gets requested again and passes. The get is coming from one of the routines that do an update of the command status - which is kicking in for each bookmark being imported, when the "Other Bookmarks" view is selected (in this case).

I suspect what is happening is that the update of the command status is happening whilst the livemark service is still doing its stuff to save the livemark, and hence errors.

So, I think that we could do with some batching of the view here during import which will also help performance (I've added a note to bug 1392533 to try and cover it, or I'll separate it out to another bug), but that's about it with the livemark error messages.

Next I'll try and validate if the pre-export and post-import databases look the same.
I've been digging in a bit more, based on the "UNIQUE constraint failed: moz_keywords.keyword" error in comment 2.

I have discovered that in FF 55, the import will fail if there's two bookmarks with the same keyword. In this case, Porcelain has two near-identical bookmarks in different folders, but with the same keyword (I'll pass on which one).

In FF 56, we're more lenient as we changed the importing methods to use the newer keyword cache, and due to bug 1313188, we now allow the same keyword for a url/postdata pair, and hence the import can succeed.


Note that bug 1199094 also reported something similar, but on a developer-initiated profile.

I'll also note that in this case the keywords don't appear to be useful: they have spaces in them, and they aren't for urls that are searches.

Marco, any suggestions for the way forward here?
Depends on: 1313188
Flags: needinfo?(mak77)

Comment 13

10 months ago
(In reply to Mark Banner (:standard8) from comment #12)
> I have discovered that in FF 55, the import will fail if there's two
> bookmarks with the same keyword. In this case, Porcelain has two
> near-identical bookmarks in different folders, but with the same keyword
> (I'll pass on which one).

I suppose the url differs? So it's the same keyword for 2 different urls?

> In FF 56, we're more lenient as we changed the importing methods to use the
> newer keyword cache, and due to bug 1313188, we now allow the same keyword
> for a url/postdata pair, and hence the import can succeed.

The fix for bug 1313188 is a schema migration removing duplicates, and then changing the way we store postData, we should store an empty string rather than NULL (because of the fact in SQL NULL != NULL).
A better solution to avoid losing user's data, may be to append a ref to the second url (#randomnumber) that will actually make it a different url, so both keywords can be retained.
On import we should be aware of cases where the same keyword is being wrongly assigned to 2 different bookmarks, basically we should always try/catch non-mandatory data like keywords, livemarks, annotations... We want to import as far as possible.

> I'll also note that in this case the keywords don't appear to be useful:
> they have spaces in them, and they aren't for urls that are searches.

The fact they aren't for searches may be fine, some users use keywords as aliases for specific urls. it's a secondary usage not intended at the beginning, but it happened in the wild.

We should probably remove any keywords containing spaces in the same schema migration. I think the new API does that check on input (TBV).

Now the problem is that we cannot uplift a schema migration, but looks like we don't need to, since 56 can import this file.
Flags: needinfo?(mak77)
It turns out I'd miss-read bug 1313188 - this isn't directly liked to that bug.

However, the good news is still that 56 can read the file - so there's nothing worth doing here. I suspect this was fixed by the switch of the json bookmarks import to the new APIs (bug 1095426), but I don't have a direct range, so I'll just mark this as WFM.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 months ago
No longer depends on: 1313188
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.