724045 - createMobileBookmarksFolder doesn't set title or parent

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Richard Newman [:rnewman]]

private void createMobileBookmarksFolder(SQLiteDatabase db) {
            ContentValues values = new ContentValues();
            values.put(Bookmarks.GUID, Bookmarks.MOBILE_FOLDER_GUID);
            values.put(Bookmarks.IS_FOLDER, 1);
            values.put(Bookmarks.POSITION, 0);

            long now = System.currentTimeMillis();
            values.put(Bookmarks.DATE_CREATED, now);
            values.put(Bookmarks.DATE_MODIFIED, now);

            db.insertOrThrow(TABLE_BOOKMARKS, Bookmarks.GUID, values);
        }

Parent should be 0L, which Sync assumes is the root.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

This sounds like a beta blocker in that we will be screwing up bookmark data.

Margaret - Can you look at this?

Comment 2

[:Margaret Leibovic]

Attachment: patch

This actually makes my patch in bug 716918 easier. (That bug seems to be exposing additional problems, like this one.)

Building with my patch for that bug on top of this, I can confirm that we're getting the right title for the folder.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 594295 [details] [diff] [review]
patch

Thanks Margaret and Richard!

Comment 4

[Richard Newman [:rnewman]]

Comment on attachment 594295 [details] [diff] [review]
patch

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

Drive-by!

Please still code defensively in your UX patch; I'll try to work in a way to have Sync quietly fix up bad Fennec DBs as I go, but people whose databases were busted by this method could still trip you up.

Comment 5

[:Margaret Leibovic]

(In reply to Richard Newman [:rnewman] from comment #4)

> Please still code defensively in your UX patch; I'll try to work in a way to
> have Sync quietly fix up bad Fennec DBs as I go, but people whose databases
> were busted by this method could still trip you up.

Yes, sir! I'll assume the worst of all possible situations ;)

Comment 6

[:Margaret Leibovic]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c6d94101af73

Comment 7

[:Margaret Leibovic]

Comment on attachment 594295 [details] [diff] [review]
patch

[Approval Request Comment]
We're going to want this on aurora and beta to ensure the mobile bookmarks folder is initialized properly.

(However, this will only affect new installs, so as Richard mentions, we'll still need to be careful dealing with old DBs.)

Comment 8

[Marco Bonardo [:mak]]

https://hg.mozilla.org/mozilla-central/rev/c6d94101af73

Comment 9

[Alex Keybl [:akeybl]]

Comment on attachment 594295 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.

Comment 10

[Richard Newman [:rnewman]]

Margaret: I pushed this to Aurora, because I was doing a bunch of Aurora landings myself.

https://hg.mozilla.org/releases/mozilla-aurora/rev/9fb0c06ceb49

I might also get around to landing it on Beta, but please don't count on it :D

Comment 11

[Brad Lassey [:blassey] (use needinfo?)]

https://hg.mozilla.org/releases/mozilla-beta/rev/55756493911f