1833429 - Make it so that BookmarkHTMLUtils / BookmarkJSONUtils can return a count of imported bookmarks

Status: RESOLVED FIXED

(Toolkit :: Places, task, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

This is a requirement for the experience expected in bug 1833427. Importing bookmarks from HTML/JSON files works nicely, but the migration wizard also wants to know how many bookmarks actually got imported from the file.

Comment 1

[Tim Giles [:tgiles] | OOO Nov 25-29]

Hey mconley, I'm a bit confused by the ask of this bug. It seems like the new migration wizard already has a bookmark import option and shows the number of bookmarks that were imported. Let me make sure I understand this correctly, we just need to return the actual number of imported bookmarks so if I import the same bookmarks twice, the second time should return 0 bookmarks correct?

Also I want to make sure I understand the import logic correctly, we want to return the actual imported count from the following places:

• BookmarkImport.import in BookmarkJSONUtils.sys.mjs
• BookmarkImport._importBookmarks in BookmarkHTMLUtils.sys.mjs
  so that when we call restoreBookmarksFromFile in places.js places can handle the value accordingly correct?

Is there something else I'm missing that needs to be done in this bug or did I inadvertently add some scope to the original ask?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Hi tgiles,

So here's the way things work right now:

When importing bookmarks from other browsers, each subclass of MigratorBase does the work of reading the bookmarks in from the associated browsers, and then calling into MigrationUtils with a consistent data structure for bookmarks that are then passed over to Places for insertion into the database.

MigrationUtils does the work of remembering the quantity of bookmarks that were imported, because this is something that PlacesUtils.bookmarks.insertTree's Promise resolves with.

Now, fast-forward to our current problem - importing bookmarks from a file. This goes through a slightly different path, because Places has some existing utilities for importing from files (BookmarkJSONUtils and BookmarkHTMLUtils, which you've found). Neither of those mechanisms return a count of how many bookmarks were successfully imported. That means that the FileMigrator that we will eventually build out in bug 1833427 will not know how many were imported to report to the user in the wizard.

So yeah, the work here is to make it so that callers into BookmarkJSONUtils / BookmarkHTMLUtils' import mechanisms can get a count of how many bookmarks were successfully imported.

We don't have to worry about restoreBookmarksFromFile, because that mechanism overwrites the existing bookmarks with the ones being restored. We are, however, using the same underlying component (in that case, BookmarkJSONUtils) to bring in the bookmarks.

Does that clear things up?

Comment 3

[Tim Giles [:tgiles] | OOO Nov 25-29]

Yeah, that clears things up! Thanks for the pointers.

Comment 4

[Tim Giles [:tgiles] | OOO Nov 25-29]

Attachment: Bug 1833429 - Return the number of imported bookmarks when importing HTML files. r=mak

When calling importFromFile or importFromUrl, we return the count of
imported bookmarks from the HTML bookmarks file.

Comment 5

[Tim Giles [:tgiles] | OOO Nov 25-29]

Attachment: Bug 1833429 - Return the number of imported bookmarks when importing JSON files. r=mak

When calling importFromFile or importFromURL, we return the count of
imported bookmarks from JSON bookmark files.

Depends on D178463

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fd2ff33e7a71
Return the number of imported bookmarks when importing HTML files. r=mak
https://hg.mozilla.org/integration/autoland/rev/3b3a2a7319af
Return the number of imported bookmarks when importing JSON files. r=mak

Comment 7

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/fd2ff33e7a71
https://hg.mozilla.org/mozilla-central/rev/3b3a2a7319af