1485286 - Import Chrome's mobile bookmarks when running migration

Status: NEW

(Firefox :: Migration, enhancement, P3)

Description

[Mark Banner (:standard8)]

We received a report from a user that when running migration from Chrome (url), we import most bookmarks but we don't import the mobile ones. From searching around, these are in a special "mobile bookmarks" folder, similar to the Firefox implementation.

Looking at the code, we don't have any functionality currently for importing the mobile bookmarks.

I think it could be reasonable to add this, as it would further help a user's migration.

If we added the mobile bookmarks into our own mobile bookmark folder, then Sync would be able to sync them straight away to any mobile device and have them appear as the main bookmarks.

From an implementation perspective, the only other thing we'd need to do is to set the preference "browser.bookmarks.showMobileBookmarks" to true if any mobile bookmarks are imported from Chrome.

Comment 2

[Mark Banner (:standard8)]

I'd be happy to mentor this if someone wants to take it up. It is not a good first bug though. Having a Google Chrome set-up where you sync bookmarks would also help for testing, though not required.

Generally I think the changes here would be in the migrate() function that's returned from within GetBookmarksResource.

As a first step, we should de-dupe the Importing bookmark bar items and Importing bookmark menu items sections. They're virtually identical. We could have a constant that for example maps bookmarks_bar to PlacesUtils.bookmarks.toolbarGuid, and then pass that into a for of loop.

From the looks of Chrome's Bookmarks file, we need to see if there's a synced section and if it has any children. If it does, then we should:

• Set browser.bookmarks.showMobileBookmark to true
• Import the bookmarks.

We should probably set the preference first, though I don't think it actually matters.

We should also extend browser/components/migration/tests/unit/test_Chrome_bookmarks.js to test that additions to the mobile folder work as expected.

Comment 3

[Kaaira Gupta [:kaaira]]

Hey! Though u mentioned that this won't be a good first bug, I would really like to work on it and take it up as my first.
Can i go ahead with it?

Comment 4

[Kaaira Gupta [:kaaira]]

Also, please guide me on how to begin with it.

Comment 5

[Mark Banner (:standard8)]

Hi, thank you for the offer, but for this bug it would be better if you had at least some experience of producing patches and working with some of the code. For this bug, I'm not really prepared to take you through those as well as understanding the import code - that's too much, and there's much simpler bugs where you can learn the initial process - I generally find small goals are better for both sides and are more likely to be completed.

Comment 6

[Sam Foster [:sfoster] (he/him)]

Jim, is this something you want to track?

Comment 7

[Jayati Shrivastava]

Hi! May I work on this?

Comment 8

[Mark Banner (:standard8)]

Hi Jayati, yes sure, please do.

Comment 9

[Jayati Shrivastava]

Hi Mark! Could you please point me to the relevant functions? Would be a great help! I have worked on one other bug relating to deduping of bookmarks but am not sure how to proceed here.

Comment 10

[Mark Banner (:standard8)]

(In reply to Jayati Shrivastava from comment #9)

Hi Mark! Could you please point me to the relevant functions? Would be a great help! I have worked on one other bug relating to deduping of bookmarks but am not sure how to proceed here.

I think comment 2 should be pointing you to all the necessary changes. I think it should just be that one function.

Comment 11

[Mark Banner (:standard8)]

Jayati isn't working on this at the moment, so opening it up again.

Comment 12

[Ania]

Justine, can you please help me check if this is still reproducible? If it does, let's include it on our list of improvements for the new migratory.

Comment 13

[Justine Oliver (:joliver)]

(In reply to Ania from comment #12)

Justine, can you please help me check if this is still reproducible? If it does, let's include it on our list of improvements for the new migratory.

I just checked and it doesn't import mobile bookmarks. I'll add it to the list of improvements.