Closed Bug 775104 Opened 12 years ago Closed 3 years ago

Export bookmarks and history

Categories

(Firefox for Android Graveyard :: General, defect, P5)

Product:
Firefox for Android Graveyard
Component:
General
Version:
17 Branch
Platform:
ARM
Android
Type:
defect

Tracking

(Not tracked)

Status:
RESOLVED INCOMPLETE

People

(Reporter: aaronmt, Assigned: nalexander)

References

Details

(Whiteboard: [lang=java])

Attachments

(2 files, 9 obsolete files)

bookmarks_exporter.patch
38.88 KB, patch
nalexander
: review+
Details | Diff | Splinter Review
export_bookmarks_sumo.patch
9.47 KB, patch
Details | Diff | Splinter Review 
It's a feature which we might want to add at some point, for the unfathomable case where someone wants to use another browser than Firefox.
(In reply to Gian-Carlo Pascutto (:gcp) from comment #1)
> It's a feature which we might want to add at some point, for the
> unfathomable case where someone wants to use another browser than Firefox.

How about for non-sync/manual backup cases, as with desktop Ff?

How about for working around:
Bug 761206 - [meta] Support for multiple simultaneous Fennec versions
>How about for non-sync/manual backup cases, as with desktop Ff?

Doesn't work because the Android history/bookmarks system is far too limited to represent what you have in Firefox.
The Android Sync team gets a trickle of support requests for help getting bookmarks off a phone and onto a desktop device.  Luckily Sync does this, but since the internals of Sync are opaque it's nerve-wracking for our users.  I've looked for an add-on that does this, with no success.  (Not surprising, since I know of no Javascript API for accessing bookmarks or history on Android.)  So I'd like a simple import/export feature.
Jamie, are you interested in working on part of this ticket with me?  What I want to do is write an exporter that dumps the Fennec bookmarks database to a JSON file (in whatever format desktop expects).  There's another ticket, Bug 429583, which is open to improve the desktop import, but it seems to have stalled.  Since you did a great job unblocking Bug 849072, are you interested in unblocking Bug 429583 and pushing on this with me?
Flags: needinfo?(jhewland)
>Not surprising, since I know of no Javascript API for accessing bookmarks or 
>history on Android.

If desktop allows this, it seems worthy of a bug to fix this on Android as well.
Hey Nick. I would be keen to work on this bug. I'd be interested on working on something a bit larger than the last one.
But there is one problem: I've got exams coming up and I don't want to over-commit to things. I am, however, free from the 2nd week of June (my winter holidays start) so if you'd be willing to postpone things till then I'd be glad to help.

Thanks for all your help on Bug 849072. Happy to have my first patch in.
Flags: needinfo?(jhewland)
Margret's Copy Profile extension might contain some helpful code. https://addons.mozilla.org/en-US/android/addon/copy-profile/
(In reply to Kevin Brosnan [:kbrosnan] from comment #8)
> Margret's Copy Profile extension might contain some helpful code.
> https://addons.mozilla.org/en-US/android/addon/copy-profile/

Thanks for the thought, but I'm quite certain copy-profile dumps the raw data (SQLite DBs) to the SD card.  We could expand on the idea to provide complete profile backup and restore, but that's not what I want to do with this ticket.
(In reply to Jamie Hewland from comment #7)
> Hey Nick. I would be keen to work on this bug. I'd be interested on working
> on something a bit larger than the last one.
> But there is one problem: I've got exams coming up and I don't want to
> over-commit to things. I am, however, free from the 2nd week of June (my
> winter holidays start) so if you'd be willing to postpone things till then
> I'd be glad to help.

We're in no rush here.  If I have time, I'll move on this and keep you updated on the ticket.  In the meantime, ace your exams, enjoy your holidays, and talk to you in a while.

> Thanks for all your help on Bug 849072. Happy to have my first patch in.

You're welcome, but really you did all the work.  It's a good feeling to have that first patch committed :)
Just had another SUMO request that would be served by this functionality.  Jamie, any interest here?
Whiteboard: [mentor=nalexander][lang=java][good first bug]
This is my first time giving this a shot. I have Mercurial set-up, but how would I even get started here?
Flags: needinfo?(nalexander)
Are you able to get the source downloaded and a build compiled? The webpage with instructions on how to build Firefox for Android is here:

https://wiki.mozilla.org/Mobile/Fennec/Android
I have downloaded the source, but I'm having a little trouble building it. In the .mozconfig document, I specified the location of my NDK toolchain folder, but the compiler is still issuing an error. Do I need to specify the exact toolchain?
I have fixed that problem, but receive this error now: error: installation or configuration problem: C compiler cannot create executables.

checking for ranlib... no
checking for ar... no

Does that have anything to do with it?
Can you get on IRC #mobile? Bugzilla isn't really the place to discuss unrelated compiler problems.
(In reply to Seth from comment #15)
> I have fixed that problem, but receive this error now: error: installation
> or configuration problem: C compiler cannot create executables.
> 
> checking for ranlib... no
> checking for ar... no
> 
> Does that have anything to do with it?

My guess is that your paths are incorrect, so we can't find the C compiler.  As gcp says, #mobile or the mobile-firefox-dev mailing list is a better forum.  Or you can contact me via email.
Flags: needinfo?(nalexander)
Can I work on this bug? Can anyone guide me through it as this will be my first bug :)
(In reply to jacksonisaac2008 from comment #18)
> Can I work on this bug? Can anyone guide me through it as this will be my
> first bug :)

You can certainly work on this bug!  Can you start with https://wiki.mozilla.org/Mobile/Get_Involved and especially the build instructions at https://wiki.mozilla.org/Mobile/Fennec/Android#Building_Fennec to get a Fennec build environment up and running?  You can join #mobile on irc.mozilla.org to get assistance.

I have a work-in-progress patch that I will dust off and attach to this ticket.
Hi, can I work on this bug too? It is also my first bug.
(In reply to Eduardo Ramirez from comment #20)
> Hi, can I work on this bug too? It is also my first bug.

Hi Eduardo! I believe nalexander is on vacation right now, but I'm sure he'll be willing to help you when he gets back. In the meantime, you may want to find another bug to work on, though, since I'm not sure when he'll be back.
Flags: needinfo?(nalexander)
(In reply to :Margaret Leibovic from comment #21)
> (In reply to Eduardo Ramirez from comment #20)
> > Hi, can I work on this bug too? It is also my first bug.
> 
> Hi Eduardo! I believe nalexander is on vacation right now, but I'm sure
> he'll be willing to help you when he gets back. In the meantime, you may
> want to find another bug to work on, though, since I'm not sure when he'll
> be back.

Thanks for jumping in, Margaret!  Eduardo, I would love if you would jump on this ticket.  Since it's your first bug, you might want to join #mobile to help get a Fennec build set up.
Flags: needinfo?(nalexander)
Hi
nalexander: I am looking for a first bug to work on. Is anyone working on this bug or can I take this up and work on it? I have pulled the source code form mercurial and have successfully built and installed fennec..:)
(In reply to saurav1991 from comment #23)
> Hi
> nalexander: I am looking for a first bug to work on. Is anyone working on
> this bug or can I take this up and work on it? I have pulled the source code
> form mercurial and have successfully built and installed fennec..:)

Yes, absolutely!  There are a bunch of steps to this bug; the major two are:

1) write an exporter that dumps the Fennec bookmark database to JSON, in a format similar to desktop.  I have some work in progress on this, which I will try to excavate, but you can start by looking in mobile/android/base/db/* to understand how to query the database.   See also https://bugzilla.mozilla.org/show_bug.cgi?id=775104#c5

2) write some Android UI to guide users through importing and exporting.  There are lots of examples in the source; for example, you could start with

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/PrivateDataPreference.java

and try to duplicate that to add an "export" and "import" pref screen.

Thanks for trying to move this forward!
(In reply to saurav1991 from comment #23)
> Hi
> nalexander: I am looking for a first bug to work on. Is anyone working on
> this bug or can I take this up and work on it? I have pulled the source code
> form mercurial and have successfully built and installed fennec..:)

Hey Saurav, any progress here?  Can you tell me what you've done/tried?  I can break this bug down into smaller pieces if that would help.
Hi Nick,
Sorry for the late response. I was busy for the past couple of weeks so didn't get much time to work on this. I was going through the files in mobile/android/base/db to know how to access the sqlite database. Do I need to develop a class similar to PasswordsProvider or TabsProvider to fetch the bookmarks and put them in a json object? It will also help if you could break down the bug to help me get started.
(In reply to saurav1991 from comment #26)
> Hi Nick,
> Sorry for the late response. I was busy for the past couple of weeks so
> didn't get much time to work on this.

No trouble.

> I was going through the files in
> mobile/android/base/db to know how to access the sqlite database. Do I need
> to develop a class similar to PasswordsProvider or TabsProvider to fetch the
> bookmarks and put them in a json object? It will also help if you could
> break down the bug to help me get started.

Nope, we don't need a {Data}Provider -- that's an Android concept that exposes a DB to the larger world.  We need to query such a provider (or the DB directly) and iterate through all the rows, dumping the bookmark information.  For example, the AndroidImporter does something like what we want (with bookmarks).  See

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/AndroidImport.java#47

You could also look at the following, which iterates Remote Tabs (from Sync) and builds a display out of them:

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/TabsAccessor.java#88

For now, though, try to write a method that just iterates all your bookmarks and logs them to the ADB log (title, url, date created, etc).  Then we can work on getting that into a JSON format.  Sound good?
(In reply to Nick Alexander :nalexander from comment #27)
> (In reply to saurav1991 from comment #26)
> > Hi Nick,
> > Sorry for the late response. I was busy for the past couple of weeks so
> > didn't get much time to work on this.
> 
> No trouble.
> 
> > I was going through the files in
> > mobile/android/base/db to know how to access the sqlite database. Do I need
> > to develop a class similar to PasswordsProvider or TabsProvider to fetch the
> > bookmarks and put them in a json object? It will also help if you could
> > break down the bug to help me get started.
> 
> Nope, we don't need a {Data}Provider -- that's an Android concept that
> exposes a DB to the larger world.  We need to query such a provider (or the
> DB directly) and iterate through all the rows, dumping the bookmark
> information.  For example, the AndroidImporter does something like what we
> want (with bookmarks).  See
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> AndroidImport.java#47
> 
> You could also look at the following, which iterates Remote Tabs (from Sync)
> and builds a display out of them:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> TabsAccessor.java#88
> 
> For now, though, try to write a method that just iterates all your bookmarks
> and logs them to the ADB log (title, url, date created, etc).  Then we can
> work on getting that into a JSON format.  Sound good?
Yeah okay. I'll try and work on that now
(In reply to Nick Alexander :nalexander from comment #27)
> (In reply to saurav1991 from comment #26)
> > Hi Nick,
> > Sorry for the late response. I was busy for the past couple of weeks so
> > didn't get much time to work on this.
> 
> No trouble.
> 
> > I was going through the files in
> > mobile/android/base/db to know how to access the sqlite database. Do I need
> > to develop a class similar to PasswordsProvider or TabsProvider to fetch the
> > bookmarks and put them in a json object? It will also help if you could
> > break down the bug to help me get started.
> 
> Nope, we don't need a {Data}Provider -- that's an Android concept that
> exposes a DB to the larger world.  We need to query such a provider (or the
> DB directly) and iterate through all the rows, dumping the bookmark
> information.  For example, the AndroidImporter does something like what we
> want (with bookmarks).  See
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> AndroidImport.java#47
> 
> You could also look at the following, which iterates Remote Tabs (from Sync)
> and builds a display out of them:
> 
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/
> TabsAccessor.java#88
> 
> For now, though, try to write a method that just iterates all your bookmarks
> and logs them to the ADB log (title, url, date created, etc).  Then we can
> work on getting that into a JSON format.  Sound good?

Should I create a new class and add the method to it or should I add to it an already existing class? In this case can you specify a class to which I should add the method?
I'd put it in a new class (AndroidExport? ExportBookmarksHistory?).

However, because figuring out how to hook up your feature in the UI may not be the most fun or interesting thing to start with, you could also start by simply reworking the existing AndroidImport.java (which already has a button in the UI) to actually export instead. Once it works, we can move it to a new file, restore AndroidImport.java and add the UI for the new export feature.
Can I pick this up to work on?
(In reply to swaroop.rao from comment #31)
> Can I pick this up to work on?

Hi Swaroop, I saw rnewman uplifted your patch to android-sync yesterday.  I'm glad you want to pick up another ticket!  Another contributor was talking to me about working on this yesterday in #mobile, so I'm not going to assign it to you just yet.  But there's a lot to this ticket:

* writing the JSON files;
* exposing a UI button somewhere to let the user actually do this in a sane way;
* saving the JSON somewhere, or sharing it via an Android share intent.

I've started collin on the JSON file bit.  The UI button probably needs UX feedback.  Can you look into wiring up an Android share intent to let people email themselves the resulting JSON text file, or upload it to the cloud, or whatever?  We have an example of something similar at

http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/BrowserApp.java#2021

Thanks!
OK, thanks, Nick. I took a quick look at the code that you pointed me to and I have a couple of questions. I haven't worked much with Android Java, so I read through the Android page on the ACTION_SEND intent and from what I could gather, you need to call startActivity() to actually trigger the intent. But, I don't see any call to startActivity() in the code you pointed me to. The other question I have is that triggering the intent will probably pop up a list of actions that are mapped to that intent. So, would that allow the user to send the JSON using some other means like SMS or WhatsApp or something? And, if so, is that OK? Should we, instead, somehow force the user to send the JSON via email? I'm not sure how, but I just thought we should take that into consideration.
One more follow-up question. Do you want me to write a separate / independent Android app that shows a text field and a 'Send' button, so when the user types something in the text field and hits 'Send', it will invoke the ACTION_SEND intent? Then, we can transplant some of that code into the Mozilla code at the appropriate place?
Regarding the Intent object that we get by calling provider.getIntent(), is it guaranteed to be created only on line 2022? Or, is it possible that it gets created elsewhere for a different action type (not Intent.ACTION_SEND)? If the latter is possible, then we should call shareIntent.setAction (Intent.ACTION_SEND) around line 2028.

So, I"ve now got a fair idea about how to write an Android application that:
a. creates an activity that exposes a UI with a text field and a 'Send' button
b. responds to a click of the 'Send' button by creating an Intent object for a SEND or SENDTO action

What I'm still a bit unclear about is what you want from me that's different than the code that you pointed me to. Is there something in the code starting at line 2021 that doesn't already do what is necessary?
Flags: needinfo?(nalexander)
I'd like to apologize to the mentees who I am abandoning, but I don't have time to mentor this bug right now.  I think it's proved to be a *bad* first or second, bug, too.  Sorry to Swaroop Rao in particular: Swaroop, I hope you pick up another ticket and we can work together again in the future.
Flags: needinfo?(nalexander)
Whiteboard: [mentor=nalexander][lang=java][good first bug] → [lang=java]
> * writing the JSON files;

I have WIP code for doing this; will post for review sometime soon.

> * exposing a UI button somewhere to let the user actually do this in a sane
> way;

ibarlow: there's no obvious way to add a "bookmarks home panel"-specific menu item, and I don't think we want to expose this on the bookmarks home panel at top-level (like how history has the bottom switcher for history and tabs-from-last time).  The other place I see to put it is buried in "Customize", next to "Import from Android".  So we'd have an "Export bookmarks" pseudo-pref.  It's a little code to implement this, 'cuz I want to have options for "Desktop bookmarks" and "Mobile bookmarks", like the import options "Bookmarks" and "History".  Before doing this configuration work, how do you feel about this placement and UI?

> * saving the JSON somewhere, or sharing it via an Android share intent.

I think sharing the text via Android is the right way to go here.
Flags: needinfo?(ibarlow)
> ibarlow: there's no obvious way to add a "bookmarks home panel"-specific
> menu item, and I don't think we want to expose this on the bookmarks home
> panel at top-level (like how history has the bottom switcher for history and
> tabs-from-last time).  The other place I see to put it is buried in
> "Customize", next to "Import from Android".  So we'd have an "Export
> bookmarks" pseudo-pref.  It's a little code to implement this, 'cuz I want
> to have options for "Desktop bookmarks" and "Mobile bookmarks", like the
> import options "Bookmarks" and "History".  Before doing this configuration
> work, how do you feel about this placement and UI?

ibarlow: we could stream-line this further by only offering the option to export desktop bookmarks when desktop bookmarks exist.  That is, if you don't have desktop bookmarks you'd see

Export all bookmarks
Cancel / Export

and if you did have desktop bookmarks

Export bookmarks
* This device [always checked]
* "Desktop Bookmarks" folder [checkable]
Cancel / Export

or similar.
I started to dig into providing the prefs-based UI for this, similar to Import from Android.  There are a few things that really need to tie into Fennec (rather than be done in Background Services).  Namely, we should use Fennec's ActivityChooserModel as much as possible to share the resulting data; see [1].

On a technical level, AndroidImportPreference does model more-or-less what we want to do.  But I propose that it's better to launch a separate BookmarkExportActivity from the prefs, to make that Activity a dialog, and then to spawn an AlertDialog from there.  It's just so much simpler than mucking about with SharedPreferences; and it's much easier to be dynamic (like how my previous comment would want to be).

[1] http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/widget/GeckoActionProvider.java#185
Assignee: nobody → nalexander
(In reply to Nick Alexander :nalexander from comment #37)
> > * writing the JSON files;
> 
> I have WIP code for doing this; will post for review sometime soon.
> 
> > * exposing a UI button somewhere to let the user actually do this in a sane
> > way;
> 
> ibarlow: there's no obvious way to add a "bookmarks home panel"-specific
> menu item, and I don't think we want to expose this on the bookmarks home
> panel at top-level (like how history has the bottom switcher for history and
> tabs-from-last time).  The other place I see to put it is buried in
> "Customize", next to "Import from Android".  So we'd have an "Export
> bookmarks" pseudo-pref.  It's a little code to implement this, 'cuz I want
> to have options for "Desktop bookmarks" and "Mobile bookmarks", like the
> import options "Bookmarks" and "History".  Before doing this configuration
> work, how do you feel about this placement and UI?
> 
> > * saving the JSON somewhere, or sharing it via an Android share intent.
> 
> I think sharing the text via Android is the right way to go here.


Can I take a step back for a moment and ask why we are prioritizing this work? Where is the request to export bookmarks coming from? I for one haven't seen any user feedback about it, nor anyone in product even mention it.
Flags: needinfo?(ibarlow)
> Can I take a step back for a moment and ask why we are prioritizing this
> work? Where is the request to export bookmarks coming from? I for one
> haven't seen any user feedback about it, nor anyone in product even mention
> it.

Yes, of course.  I happened to get a SUMO question that would have been served by this functionality.  These SUMO questions are not frequent, but they are consistent; I suppose you can say this was user feedback :)

I had a free moment and some old code that I wrote the last time such a question was asked, so I thought I would dust it off and give it a push forward.  That code handled the DB crawling and writing the files, so the next step is finding an integration point in the UI. 

If this is something we *don't* want, that's surprising to me, but it's not something I feel strongly about.  I'll point out that this is technically possible, but rather difficult, to do in an add-on; I suppose I can do it that way and just recommend the add-on on SUMO.
Ahmed: I wanted to point out some existing code, useful for debugging and to get started crawling the DB: https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/background/db/CursorDumper.java
Flags: needinfo?(ahmedibrahimkhali)
Hi Nick, Hope that you are enjoying your weekend.
I'd like to ask should the exported JSON be identical to the desktop one, for example the desktop one has a root with guid: root________, should I mimic that also in the mobile JSON ?
Flags: needinfo?(ahmedibrahimkhali) → needinfo?(nalexander)
(In reply to Ahmed Khalil from comment #43)
> Hi Nick, Hope that you are enjoying your weekend.
> I'd like to ask should the exported JSON be identical to the desktop one,
> for example the desktop one has a root with guid: root________, should I
> mimic that also in the mobile JSON ?

Hey!  I would definitely include a "root" node, but I don't know how important the precise GUID is.  Probably not important, but as things progress we can test importing on Desktop and get a Desktop engineer involved to answer such details.
Flags: needinfo?(nalexander)
Hi Nick,
I've written some code to crawl the DB via cursors and build the Bookmarks tree then export that in-memory tree to JSON format.
The resulting JSON was http://pastebin.com/F77fjWcx

Tell me what you think of that JSON.

Note: Sorry about the ordering the JSONObject inside the org.json.simple package does not support ordering unless I explicitly used a Map that preserves ordering like LinkedHashMap
Flags: needinfo?(nalexander)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
Hi Nick, I've submitted a patch that exports the bookmarks as we've agreed on the IRC.
Let me know what you think, and If everything was okay, we'd proceed on implementing it on the UI in a proper way, instead of the quick menu item that I've added locally in the code.
Flags: needinfo?(nalexander)
Attachment #8581189 - Flags: review?(nalexander)
Comment on attachment 8581189 [details] [diff] [review]
bookmarks_exporter.patch

Review of attachment 8581189 [details] [diff] [review]:
-----------------------------------------------------------------

This is a great first patch towards this, Ahmed!  I have a bunch of nits (easy) and then some substantive requests.  The next steps are:

1) write some JUnit 3 tests (Robocop tests that don't use Robocop).  This is tricky, but try adding a test file and some unit tests to mobile/android/tests/browser/junit3/src.  If you're using IntelliJ, such tests get compiled into robocop.apk and the individual test class can be run, on device, without running the other tests (which need a special harness to succeed).  Your tests won't need that, so maybe we can get some coverage and examples cheaply.  Please ping with progress on this -- it's tricky and I can steer much more easily than you can reverse engineer :)

2) write some tests for *Desktop* Firefox to make sure that the files you produce can be imported.  Maybe we can add some of your export files to the Desktop tree and use existing test code?  Dig into this; I'm not expert here.

3) get rnewman to respond to following queries:

a) Richard, I'm concerned about OOM for huge bookmark collections, but those are very rare.  How do you feel about this style of solution, which is conceptually simple?  Streaming tree-like JSON is not pleasant...

b) do other Bookmark TYPES arise?  Are they always from Sync?  Should we handle them?  (Presumably, yes.)

c) can you explain what is the right thing to do re: roots?  Should we export each root separately?  Only export mobile (probably not).

4) fix nits and issues.  (Tests and rnewman's opinion is more valuable than polish, since we don't know direction quite yet.)

Again, thanks for digging in to a vague ticket.  I apologize for the delayed (!) review: I'm way underwater on my own workload.  You're always welcome to ping or ask questions in #mobile.

::: mobile/android/base/db/BookmarksExporter.java
@@ +1,1 @@
> +package org.mozilla.gecko.db;

nit: license header (copy-paste from other files).

@@ +14,5 @@
> +import java.util.LinkedList;
> +import java.util.List;
> +import java.util.Queue;
> +
> +public class BookmarksExporter {

Let's make this class non-static, perhaps have the cursor be a constructor parameter, and provide the static helper like you have that queries and constructs the exporter.

@@ +16,5 @@
> +import java.util.Queue;
> +
> +public class BookmarksExporter {
> +
> +    public static String exportBookmarks(Context context) {

nit: let's return the JSON object rather than Stringifying.

@@ +17,5 @@
> +
> +public class BookmarksExporter {
> +
> +    public static String exportBookmarks(Context context) {
> +        ContentResolver contentResolver = context.getContentResolver();

nit (throughout): in general, final everything you can.  Makes life easier to reason about, and in theory the compiler can do better.

@@ +19,5 @@
> +
> +    public static String exportBookmarks(Context context) {
> +        ContentResolver contentResolver = context.getContentResolver();
> +
> +        Cursor cursor = contentResolver.query(BrowserContract.Bookmarks.CONTENT_URI, null, null, null, null);

A little logging would be nice -- maybe "Trying to export " + cursor.count() + " bookmarks."

I'm quite worried about OOM kills during this, but perhaps I'm paranoid.  Let's make this take a cursor, too, for testing.

@@ +23,5 @@
> +        Cursor cursor = contentResolver.query(BrowserContract.Bookmarks.CONTENT_URI, null, null, null, null);
> +
> +        try {
> +            Bookmark root = null;
> +            if (cursor.moveToFirst()) {

Extract the case where we have a bad cursor (can't moveToFirst, say) or can't get the root as an error case (that throws something, either a custom exception (good) or IllegalStateException (less good)).

Also, how do you know the root bookmark comes first?  You should try to order the results in some way, or handle root coming at some arbitrary level.  (My Map of parents would help with this.  See below.)

@@ +27,5 @@
> +            if (cursor.moveToFirst()) {
> +                root = getBookmarkFromCursor(cursor);
> +            }
> +
> +            if (root != null) {

No sense hiding failure here.

@@ +33,5 @@
> +                    Bookmark aBookmark = getBookmarkFromCursor(cursor);
> +                    // FF desktop has no root defined for the mobile folder, so we add the mobile folder
> +                    // under a fake version of the "Bookmarks Menu" that copies the desktop attributes
> +                    // so that it could be visible inside the bookmarks view.
> +                    if (aBookmark.guid.equals(BrowserContract.Bookmarks.MOBILE_FOLDER_GUID)) {

Reverse the clauses here -- guid *should* not be null, but MOBILE_FOLDER_GUID *is* never null.  This can avoid an NPE.

@@ +44,5 @@
> +                        // Since we have added a fake desktop "Menu Bookmarks" node, then there is no
> +                        // need for the mobile version of this node.
> +                        continue;
> +                    }
> +                    Bookmark foundBookmark = root.findChild(aBookmark.parentId);

This will work, at the cost of traversing the tree (via BFS) for every child.  I think most bookmark hierarchies are very flat, so BFS looks like a linear search across a list of children.  Can we do better?  I would consider maintaining a Map<Integer, Bookmark> where keys are parent IDs and values are parents.  Then you can add to the list of parent's children very quickly.

@@ +59,5 @@
> +    }
> +
> +    private static Bookmark getBookmarkFromCursor(Cursor cursor) {
> +        Bookmark bookmark = new Bookmark();
> +        bookmark.guid = cursor.getString(cursor.getColumnIndex(BrowserContract.Bookmarks.GUID));

Make sure to use getColumnIndexOrThrow -- no sense silently failing in some way.

This may be unnecessary, but Fennec always tries to extract these getColumnIndex calculations out of the loop.  I know this is counter-intuitive but can you merge this helper into the loop code, yielding something like a collectBookmarksFromCursor method (which returns an Iterable<Bookmark> or something similar)?

@@ +87,5 @@
> +        bookmark.title = "Bookmarks Menu";
> +        bookmark.index = 1;
> +        bookmark.dateAdded = now.getTime();
> +        bookmark.lastModified = now.getTime();
> +        // Generate arbitrary id

nit: full sentence.

This business with the roots is definitely not right, but I don't know what is right.  I'll flag rnewman for comment.

@@ +95,5 @@
> +        return bookmark;
> +    }
> +
> +    private static class Bookmark {
> +        String guid;

nit: (really small): annotate these all with protected, public, or whatever.

@@ +110,5 @@
> +
> +        int type;
> +        int index;
> +
> +        List<Bookmark> children = new ArrayList<>();

nit: most Bookmark's are not folders, they have no children.  Save the allocation in this case by allowing null.

Also, you're appending a lot (most bookmarks are just in the mobile folder) and then iterating once; ArrayList doesn't have great performance in this case.  Consider a LinkedList instead.

@@ +129,5 @@
> +                if (aBookmark.id == id) {
> +                    return aBookmark;
> +                }
> +                for(Bookmark childBookmark : aBookmark.children) {
> +                    if (!visitedArray.get((int)childBookmark.id)) {

nit: (int) child..., here and below.

@@ +138,5 @@
> +            }
> +            return null;
> +        }
> +
> +        private JSONObject toJsonObject() {

nit: toJSONObjectWithoutChildren.

@@ +150,5 @@
> +            if (root != null) {
> +                jsonObject.put("root", root);
> +            }
> +
> +            if (type == BrowserContract.Bookmarks.TYPE_FOLDER) {

I see some other types at https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/db/BrowserContract.java#163.  For now, let's ignore them explicitly.

@@ +164,5 @@
> +            jsonObject.put("dateAdded", dateAdded * 1000 );
> +            return jsonObject;
> +        }
> +
> +        private JSONObject buildJSONTree() {

nit: toJSONObjectWithChildren, or just toJSONObject.

@@ +171,5 @@
> +                return root;
> +            }
> +            root.put("children", new JSONArray());
> +            JSONArray jsonArray = (JSONArray) root.get("children");
> +            for(Bookmark aBookmark : children) {

nit: for (
Attachment #8581189 - Flags: feedback?(rnewman)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
Hi Nick,
I've updated the patch with most of your comments and I've made the class as non static, and the Ctor to take a Context a as parameter.
I think that it is not logical to take a Cursor as I'm already querying the ContentProvider for a cursor, so why do we need another cursor ? Please correct me if I'm wrong. There is surely something I'm not seeing in your suggestion.

I've also used SparseArray<Bookmark> instead of Map<Integer, Bookmark> as it is more memory efficient.
Attachment #8581189 - Attachment is obsolete: true
Attachment #8581189 - Flags: review?(nalexander)
Attachment #8581189 - Flags: feedback?(rnewman)
Attachment #8587564 - Flags: review?(nalexander)
Attachment #8587564 - Flags: feedback?(rnewman)
(In reply to Ahmed Khalil from comment #48)
> Created attachment 8587564 [details] [diff] [review]
> bookmarks_exporter.patch
> 
> Hi Nick,
> I've updated the patch with most of your comments and I've made the class as
> non static, and the Ctor to take a Context a as parameter.
> I think that it is not logical to take a Cursor as I'm already querying the
> ContentProvider for a cursor, so why do we need another cursor ? Please
> correct me if I'm wrong. There is surely something I'm not seeing in your
> suggestion.

The idea is to separate the actual data from the process; it might be useful to have a bunch of different cursors during testing.  Or to export, say, only Desktop or only Mobile bookmarks.
 
> I've also used SparseArray<Bookmark> instead of Map<Integer, Bookmark> as it
> is more memory efficient.

Neat.
Comment on attachment 8587564 [details] [diff] [review]
bookmarks_exporter.patch

Review of attachment 8587564 [details] [diff] [review]:
-----------------------------------------------------------------

This looks pretty much right for v1.  But we will definitely want some tests.  Can you investigate some?  See, say, https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/tests/testBookmarkFolders.java for examples of how to clear the DB and insert folders and children.  Then you can use your code to test against.  You'll need to edit mobile/android/base/mach.build to add your new file; and you'll want to add @RobocopTarget so your code doesn't get stripped by Proguard.  I can help with these things if you run into trouble.

rnewman, can you answer the points in part 3) of https://bugzilla.mozilla.org/show_bug.cgi?id=775104#c47.

::: mobile/android/base/db/BookmarksExporter.java
@@ +172,5 @@
> +            if (root != null) {
> +                jsonObject.put("root", root);
> +            }
> +
> +            if (type == BrowserContract.Bookmarks.TYPE_FOLDER) {

Make sure to drop items that aren't folders or bookmarks (livemarks, separators).

@@ +195,5 @@
> +
> +            root.put("children", new JSONArray());
> +            JSONArray jsonArray = (JSONArray) root.get("children");
> +
> +            // Sort the children to preserve there order.

nit: explain what the sort does: "Sort the children by index."  Here and below, next to compareTo.
Attachment #8587564 - Flags: review?(nalexander) → review+
> 3) get rnewman to respond to following queries:
> 
> a) Richard, I'm concerned about OOM for huge bookmark collections, but those
> are very rare.  How do you feel about this style of solution, which is
> conceptually simple?  Streaming tree-like JSON is not pleasant...

Huge bookmark collections are rare, but not that rare: more than 10% of Sync users have more than a thousand bookmarks. About 0.3% have more than 10,000 (obviously not on Android!).

Also, people with large bookmark collections are arguably more likely to be users of this feature.

I'd draw the line at about 2,000. If that makes us bomb out on, say, a Galaxy S3, then we should revisit.

To be clear, though: anything is better than nothing, so I wouldn't block landing because of inefficiencies.


> b) do other Bookmark TYPES arise?  Are they always from Sync?  Should we
> handle them?  (Presumably, yes.)

Local-only users will have only bookmarks in Mobile Bookmarks. The only folders we'd see would be roots. Anything else is from Sync.

For the majority of our other users -- those who started with FxA Sync -- they'll see bookmarks, folders, queries, separators.

There is a chance that we'll encounter microsummaries and livemarks in existing databases. Microsummaries are dead, but they should turn into normal bookmarks.

Note that livemarks, microsummaries, queries, and separators are encoded via encodeUnsupportedTypeURI, and thus need special handling during export.


> c) can you explain what is the right thing to do re: roots?  Should we
> export each root separately?  Only export mobile (probably not).

We should probably export all; part of the idea is to rescue all of your data. I don't know what the right thing to do is, because I don't have the bookmark.jsonlz4 format to hand.
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
Hi Nick ! I've changed the BookmarksExporter as per last review and changed the Ctor to take a cursor.

Also I've added testBookmarksExporter.java to test the exporter and added to the test case, testing a nested bookmarks folder under the mobile folder. Which works as expected.

I believe there is still much room for improvement especially at what you've told me at the IRC channel 
> "make it easier to see the input <-> expected output."

Which unfortunately I didn't get it, despite you trying to explain it to me :(
Attachment #8587564 - Attachment is obsolete: true
Attachment #8587564 - Flags: feedback?(rnewman)
Attachment #8594274 - Flags: review?(nalexander)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
I've added "BookmarksExporter.java" to mobile/android/base/moz.build, and added the "textBookmarksExporter" to robocop.ini as I forgot to add them.
Attachment #8594274 - Attachment is obsolete: true
Attachment #8594274 - Flags: review?(nalexander)
Attachment #8594322 - Flags: review?(nalexander)
Comment on attachment 8594322 [details] [diff] [review]
bookmarks_exporter.patch

Review of attachment 8594322 [details] [diff] [review]:
-----------------------------------------------------------------

This is basically ready to land.  We'll file follow-up tickets for exposing the UI in some way, and to land additional desktop tests.
 
Great work, Ahmed!
Attachment #8594322 - Flags: review?(nalexander) → review+
Ahmed: I just talked to antlam; let's put this in the Tools menu for now.  (Under "New Guest Session", I think it's called.)  Can you prepare a patch that adds the strings for this item, the menu item, prompts for confirmation, and then launches the Share Intent?
rolandtanglao: if I wanted to get a SUMO page documenting how to use the feature on Android, and helping people import the resulting JSON files on Desktop, do you know who I would talk to?  Is there already a SUMO page for backing up/importing bookmark JSON files on Desktop that we could add this feature to?
Flags: needinfo?(rtanglao)
(In reply to Nick Alexander :nalexander from comment #55)
> Ahmed: I just talked to antlam; let's put this in the Tools menu for now. 
> (Under "New Guest Session", I think it's called.)  Can you prepare a patch
> that adds the strings for this item, the menu item, prompts for
> confirmation, and then launches the Share Intent?

Perhaps "Export bookmarks" as the title
(In reply to Nick Alexander :nalexander from comment #55)
> Ahmed: I just talked to antlam; let's put this in the Tools menu for now. 
> (Under "New Guest Session", I think it's called.)  Can you prepare a patch
> that adds the strings for this item, the menu item, prompts for
> confirmation, and then launches the Share Intent?

Yeah, Sure I'll do that ASAP, I think it will be ready by tomorrow.
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
Hi Nick, I've managed to link the BookmarksExporter with the UI as we've agreed.
Also I've hidden the "Export bookmarks" button when in Guest Session.

Note: There are some formatting changes that are included with the patch, and I don't know why it was included as I never touched these parts.
Attachment #8594322 - Attachment is obsolete: true
Attachment #8597371 - Flags: review?(nalexander)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
Added missing string resources.
Attachment #8597371 - Attachment is obsolete: true
Attachment #8597371 - Flags: review?(nalexander)
Attachment #8597377 - Flags: review?(nalexander)
(In reply to Nick Alexander :nalexander from comment #56)
> rolandtanglao: if I wanted to get a SUMO page documenting how to use the
> feature on Android, and helping people import the resulting JSON files on
> Desktop, do you know who I would talk to?  Is there already a SUMO page for
> backing up/importing bookmark JSON files on Desktop that we could add this
> feature to?

joni savage is our content enditor for FF desktop and firefox android and firefox ios and firefox os 
nick please talk to joni (i've CC'd her on this bug)
i've CC'ed her
Flags: needinfo?(rtanglao)
(In reply to Nick Alexander :nalexander from comment #56)
> rolandtanglao: if I wanted to get a SUMO page documenting how to use the
> feature on Android, and helping people import the resulting JSON files on
> Desktop, do you know who I would talk to?  Is there already a SUMO page for
> backing up/importing bookmark JSON files on Desktop that we could add this
> feature to?

nick wrote:"Is there already a SUMO page for backing up/importing bookmark JSON files on Desktop that we could add this feature to? "
joni and nick: yup the desktop article is:
https://support.mozilla.org/kb/export-firefox-bookmarks-to-backup-or-transfer 

we probably want a separate android article but joni and i will figure it out!
Comment on attachment 8597377 [details] [diff] [review]
bookmarks_exporter.patch

Review of attachment 8597377 [details] [diff] [review]:
-----------------------------------------------------------------

I have some nits and some re-organization requests, but this is basically ready.  Needs a try build and a few test runs -- do you have try access, Ahmed?  Docs at https://www.mozilla.org/en-US/about/governance/policies/commit/access-policy/.  I will vouch for you!

::: mobile/android/base/BrowserApp.java
@@ +942,5 @@
>              GeckoAppShell.gracefulExit();
>              return;
>          }
>  
> +        EventDispatcher.getInstance().unregisterGeckoThreadListener((GeckoEventListener) this,

Whatever these spurious changes are, get rid of them.

@@ +3357,5 @@
>  
>          ps.show(res.getString(titleString), res.getString(msgString), null, ListView.CHOICE_MODE_NONE);
>      }
>  
> +    private void showExportBookmarksDialog() {

Let's extract this into a little helper class, "BookmarksExporterHelper" or something.  I never want to add to BrowserApp if we can help it.

@@ +3547,5 @@
>              }
>          }).execute();
>      }
>  
> +    private void doExportBookmarks() {

Put this in the Helper class too.

@@ +3554,5 @@
> +        try {
> +            bookmarksCursor = getContentResolver()
> +                    .query(BrowserContract.Bookmarks.CONTENT_URI, null, null, null, null);
> +
> +            BookmarksExporter exporter = new BookmarksExporter(bookmarksCursor);

Please final everything that can be final.

@@ +3561,5 @@
> +
> +            String fileName = "bookmarks_" + GeckoProfile.getDefaultProfileName(this) + "_" +
> +                    System.currentTimeMillis() + ".json";
> +
> +            // Some application adds security check to the attached file (like Gmail),

Good comment!

@@ +3563,5 @@
> +                    System.currentTimeMillis() + ".json";
> +
> +            // Some application adds security check to the attached file (like Gmail),
> +            // as it refuses to upload any file in private directories so we have to write the file
> +            // in external directories that could be accessed by public.

... can be accessed by other Apps on the system.

@@ +3578,5 @@
> +
> +        } catch (NoMozillaDirectoryException noMozillaDirectoryException) {
> +            Log.e(LOGTAG, "Failed to retrieve default profile name " + noMozillaDirectoryException);
> +        } catch (IOException ioException ) {
> +            Log.e(LOGTAG, "Failed to write to output file " + ioException);

Worth showing a toast in these failure situations?

::: mobile/android/base/locales/en-US/android_strings.dtd
@@ +613,5 @@
>  <!ENTITY exit_guest_session_text "The browsing data from this session will be deleted.">
>  
> +<!-- Export Bookmarks -->
> +<!ENTITY export_bookmarks "Export bookmarks">
> +<!ENTITY export_bookmarks_text "Are you sure that you want to export your bookmarks in a format that could be exported to Firefox Desktop ?">

Let's get antlam's input on this.

::: mobile/android/base/resources/menu-xlarge-v11/browser_app_menu.xml
@@ +118,5 @@
>                    android:visible="false"
>                    android:title="@string/exit_guest_session"/>
>  
> +            <item android:id="@+id/export_bookmarks"
> +                android:title="@string/export_bookmarks" />

nit: make the indentation follow the rest of the file.

::: mobile/android/base/strings.xml.in
@@ +469,5 @@
>    <string name="bookmarkhistory_import_both">&bookmarkhistory_import_both;</string>
>    <string name="bookmarkhistory_import_bookmarks">&bookmarkhistory_import_bookmarks;</string>
>    <string name="bookmarkhistory_import_history">&bookmarkhistory_import_history;</string>
>    <string name="bookmarkhistory_import_wait">&bookmarkhistory_import_wait;</string>
> +  <string name="export_bookmarks">&export_bookmarks;</string>

Let's make this string a little more descriptive, like export_bookmarks_description or something.
Attachment #8597377 - Flags: review?(nalexander) → review+
Hi Anthony,

Could you please provide me with an alternative message that I could show to the user when he is about to export his bookmarks? 

Right now it is "Are you sure that you want to export your bookmarks in a format that could be exported to Firefox Desktop ?"

And since I'm not a native English speaker, you will surely provide a better sentence.
Flags: needinfo?(alam)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
I've resolved your comments, also added a BookmarksExporterHelper that contains a static method that takes an Activity context. 

I chose the static method approach instead of a Singleton, because we need to show a dialog with an activity context and usually singletons take an application context when it gets created for the first time, and application contexts cannot be used to create dialogs.
Attachment #8597377 - Attachment is obsolete: true
Attachment #8600104 - Flags: review?(nalexander)
Attached patch bookmarks_exporter.patch (obsolete) — — Splinter Review 
I forgot to reflect the changes that is done in strings.dtd file, to the res/menu/*.xml files.
Attachment #8600104 - Attachment is obsolete: true
Attachment #8600104 - Flags: review?(nalexander)
Attachment #8600186 - Flags: review?(nalexander)
(In reply to Ahmed Khalil from comment #64)
> Hi Anthony,
> 
> Could you please provide me with an alternative message that I could show to
> the user when he is about to export his bookmarks? 
> 
> Right now it is "Are you sure that you want to export your bookmarks in a
> format that could be exported to Firefox Desktop ?"
> 
> And since I'm not a native English speaker, you will surely provide a better
> sentence.

Sure, let me think of something.

Can you show me a screenshot of what this currently looks like? It'll help me.
Attached image device-2015-05-01-235606.png (obsolete) — 
Yeah sure
Thanks Ahmed, try this!

Export your bookmarks?

Your bookmarks will be exported and shared through an application of your choice. You can import this to Firefox for Desktop after.

                              CANCEL     EXPORT
Flags: needinfo?(alam)
Flags: needinfo?(ahmedibrahimkhali)
^ also, we can right align the two actions in that dialog for L? a la http://www.google.com/design/spec/components/dialogs.html#dialogs-content
Replaced the strings with antlam's suggestion.
Attachment #8600186 - Attachment is obsolete: true
Attachment #8600186 - Flags: review?(nalexander)
> ^ also, we can right align the two actions in that dialog for L? a la
> http://www.google.com/design/spec/components/dialogs.html#dialogs-content


It is doable but I think that this UX improvement is out of scope of this bug. 

Perhaps If someone filed a a separate bug for replacing android.app.AlertDialog with android.support.v7.app.AlertDialog I can make this improvement.
Flags: needinfo?(ahmedibrahimkhali)
Attachment #8601153 - Flags: review?(nalexander)
(In reply to Ahmed Khalil from comment #72)
> > ^ also, we can right align the two actions in that dialog for L? a la
> > http://www.google.com/design/spec/components/dialogs.html#dialogs-content
> 
> 
> It is doable but I think that this UX improvement is out of scope of this
> bug. 
> 
> Perhaps If someone filed a a separate bug for replacing
> android.app.AlertDialog with android.support.v7.app.AlertDialog I can make
> this improvement.

Filed bug 1161286.

Thanks Ahmed! Although out of the scope of this bug, we should try keep a consistent UX experience going where possible. This kind of stuff gets lost and forgotten about easily :)
Comment on attachment 8601153 [details] [diff] [review]
bookmarks_exporter.patch

Review of attachment 8601153 [details] [diff] [review]:
-----------------------------------------------------------------

This is looking great: we're into the meaty bits of making this work smoothly across platforms.  For the next patch, let's have a second patch *on top* to make it easier to see the changes.

::: mobile/android/base/BookmarksExporterHelper.java
@@ +2,5 @@
> + * This Source Code Form is subject to the terms of the Mozilla Public
> + * License, v. 2.0. If a copy of the MPL was not distributed with this
> + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
> +
> + package org.mozilla.gecko;

nit: no space (several places throughout).

@@ +40,5 @@
> +            public void onPromptFinished(String jsonResult) {
> +                try {
> +                    final int itemId = new JSONObject(jsonResult).getInt("button");
> +                    if (itemId == 0) {
> +                        doExportBookmarks(context);

Do this on a background thread to avoid the strict mode violation using ThreadUtils.postToBackgroundThread.  I'd like to open a tab pointing at the SUMO article for how to import on Desktop after we send the Intent, so we may need a callback here after we complete.

@@ +54,5 @@
> +                res.getString(R.string.button_export),
> +                res.getString(R.string.button_cancel)
> +        });
> +
> +        ps.show(res.getString(R.string.export_bookmarks_dialog_title), res.getString(R.string.export_bookmarks_dialog_text),

nit: newline after ,.

@@ +71,5 @@
> +            final BookmarksExporter exporter = new BookmarksExporter(bookmarksCursor);
> +
> +            final org.json.simple.JSONObject exportedJSON = exporter.exportBookmarks();
> +
> +            final String fileName = "bookmarks_" + GeckoProfile.getDefaultProfileName(context) + "_" +

Extract a naming helper.  When using this, I found the name really user hostile.  Let's make it something like:

Bookmarks from Firefox on GTI9505 at 2011-01-18 00:00.json

where Firefox is R.string.brand_short_name, where GTI9505 is the device name (https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/SysInfo.java#204), and where the time is formatted using some locale appropriate date format, with hours and minutes but not seconds.

@@ +77,5 @@
> +
> +            // Some applications adds security checks to the attached file (like Gmail),
> +            // as it refuses to upload any files that are in private directories
> +            // so we have to write the file in external directories that can be
> +            // accessed by other Apps on the system.

Make a note that this causes a StrictMode violation that we know and don't care about this (at least right now).

@@ +89,5 @@
> +            shareIntent.putExtra(Intent.EXTRA_STREAM, Uri.fromFile(bookmarksJSONFile));
> +            context.startActivity(Intent.createChooser(shareIntent,
> +                    resources.getString(R.string.export_bookmarks_menu_title)));
> +
> +        } catch (GeckoProfileDirectories.NoMozillaDirectoryException noMozillaDirectoryException) {

In general, just call exceptions e.  No need for description and length.

::: mobile/android/base/db/BookmarksExporter.java
@@ +37,5 @@
> +            for (Bookmark aBookmark : iterableBookmarks) {
> +                // FF desktop has no root defined for the mobile folder, so we add the mobile folder
> +                // under a fake version of the "Bookmarks Menu" that copies the desktop attributes
> +                // so that it could be visible inside the bookmarks view.
> +                if (BrowserContract.Bookmarks.MOBILE_FOLDER_GUID.equals(aBookmark.guid)) {

OK, it's time to make this work with Sync back and forth between Desktop and Mobile.  We need to export the top-level root (id 0, I think), or all of the known roots (there are lists) like "mobile", "desktop", "menu".

To work with this, add some bookmarks on Mobile and some on Desktop, connect a Firefox Account (test accounts using mockmyid.com are great), Sync both ways, and then test exporting on Mobile and then importing the backup on Desktop.  We should get exactly the same bookmarks as there were before.

I can explain more if needed here.
Attachment #8601153 - Flags: review?(nalexander) → review+
> OK, it's time to make this work with Sync back and forth between Desktop and
> Mobile.  We need to export the top-level root (id 0, I think), or all of the
> known roots (there are lists) like "mobile", "desktop", "menu".
> 
> To work with this, add some bookmarks on Mobile and some on Desktop, connect
> a Firefox Account (test accounts using mockmyid.com are great), Sync both
> ways, and then test exporting on Mobile and then importing the backup on
> Desktop.  We should get exactly the same bookmarks as there were before.

For the record: right now the mobile bookmarks end up in the wrong place (not in the toplevel mobile folder) on Desktop.  So if you then Sync to Mobile you get duplicates, etc.  This will be about examining the output of the Desktop backup and making the output from Mobile agree.  (Like you've been doing.)
Depends on: 1162717
This patch is generated on top of bookmarks_exporter.patch, I added a callback that is called when the user presses "Export" and I open a SUMO page that guides the user how to import on FF Desktop.
Attachment #8600456 - Attachment is obsolete: true
Attachment #8602997 - Flags: review?(nalexander)
Comment on attachment 8602997 [details] [diff] [review]
export_bookmarks_sumo.patch

It's not clear if and when we'd get back to this, so I'm removing my long-pending r?.
Attachment #8602997 - Flags: review?(nalexander)
Re-triaging per https://bugzilla.mozilla.org/show_bug.cgi?id=1473195

Needinfo :susheel if you think this bug should be re-triaged.
Priority: -- → P5
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: