67.13 KB, image/png
4.45 KB, text/plain
Created attachment 8828919 [details] about sync shows "Unable to process backup file" when the file is processed correctly.png Mozilla/5.0(Macintosh; Intel Mac OS X 10.11; rv:53.0)Gecko/20100101 Firefox/53.0 Precondition: Download AboutSync addon: https://addons.mozilla.org/en-US/firefox/addon/about-sync/ Step 1. Launch firefox with an existing sync account 2. Go to Menu Bar > Bookmarks > Show all bookmarks 3. Click on Backup from "Import and Backup" drop down list 4. Save Bookmarks as Json file 5. Add, Edit, Delete bookmarks from the account profile, trigger sync and wait for finish 6. In About Sync, verify that there are no bookmarks missing from the server 7. Close and reopen Firefox with the same profile 8. Go to Menu Bar > Bookmarks > Show all bookmarks 9. Click on "Import and Backup" and select Restore -> Choose file 10. Select backed up file from step 4 to restore 11. Click Ok 12. Trigger Sync and wait for it to finish 13. In About Sync, verify that there are no bookmarks missing from the server AR: About:Sync shows wrong message "Unable to process backup file" while the bookmarks are imported correctly (see the attached screenshot) ER: The backup file is imported correctly, About:Sync show successful message
I encountered this issue too one time on Windows 10. When I tried second time to restore same bookmarks json file, it got imported without any issue. Note: Backup json file is from some other old profile.
It's not clear to me what About Sync has to do with this. From the screenshot, it looks like that's the Library UI, and About Sync isn't involved at all? The code that shows the error is at http://searchfox.org/mozilla-central/rev/30fcf167af036aeddf322de44a2fadd370acfd2f/browser/components/places/content/places.js#515-521, but we don't log why `BookmarkJSONUtils.importFromFile` threw. Mak, do you think it's worth adding a `Cu.reportError` call here? It's also interesting that Stefan and Kanchan's bookmarks were imported successfully, despite the error, and that importing again makes the error go away.
Component: WebExtensions: General → Bookmarks & History
Flags: needinfo?(mark.page) → needinfo?(mak77)
Product: Toolkit → Firefox
Summary: About Sync shows "Unable to process the backup file" message when importing bookmarks from JSON backup file while the file is imported correctly → "Unable to process the backup file" when importing bookmarks from JSON backup, but the bookmarks are imported correctly
(In reply to Kit Cambridge [:kitcambridge] from comment #2) > involved at all? The code that shows the error is at > http://searchfox.org/mozilla-central/rev/ > 30fcf167af036aeddf322de44a2fadd370acfd2f/browser/components/places/content/ > places.js#515-521, but we don't log why `BookmarkJSONUtils.importFromFile` > threw. > > Mak, do you think it's worth adding a `Cu.reportError` call here? That method uses importFromFile that already has a reportError: Cu.reportError("Failed to restore bookmarks from " + aFilePath + ": " + ex); It would be useful to know what it printed to the console. > It's also > interesting that Stefan and Kanchan's bookmarks were imported successfully, > despite the error, and that importing again makes the error go away. It depends on what we mean by imported successfully, it's possible the process worked 99%, but missed something that may not be easily detectable through the ui. For example, it may have failed to create a keyword? OR maybe something was just busy and the first time it threw? Many things may go wrong without being clearly visible in the UI.
stefan, could you please report the Browser console output?
Created attachment 8830101 [details] Importing Bookmarks 'Failed' browser console outoput.txt Please find the requested Browser Console output as an attached txt file
UNIQUE constraint failed: moz_keywords.keyword BookmarkJSONUtils.jsm:111
Looks like the restore tries to recreate an already existing keyword. I think we may want to ignore keyword insertion errors, regardless "Unable to process backup file" is not a good error to show to the user for this case. We should probably differentiate a complete failure (invalid file) from a partial failure ("We were unable to import some of the items"). This bug could cover figuring out a better error message. That said, there is probably a race condition here. We remove all the bookmarks to start restoring, keywords observe bookmarks removal and start their removal. The restore process may begin before the keywords work is done, and then effectively we could miss restoring a keyword because the insertion fails and there's a pending removal. We could: 1. find a way to wait for pending keywords work, but that sounds hackish. 2. explicitly remove all the keywords before starting removing bookmarks. The problem is that once keywords are decoupled from bookmarks (bug 648398), this may end up removing too much. 3. in the keywords bookmarks observer abort a keyword deletion if in the meanwhile someone tries to insert the same keyword OR if there are pending observer operations yield on those before doing the insertion. Solution 3 looks like the best one, so far.
Priority: -- → P2
You need to log in before you can comment on or make changes to this bug.