Last Comment Bug 724045 - createMobileBookmarksFolder doesn't set title or parent
: createMobileBookmarksFolder doesn't set title or parent
Status: RESOLVED FIXED
:
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: -- normal (vote)
: Firefox 13
Assigned To: :Margaret Leibovic
:
:
Mentors:
Depends on:
Blocks: 716918
  Show dependency treegraph
 
Reported: 2012-02-03 11:06 PST by Richard Newman [:rnewman]
Modified: 2012-02-24 11:09 PST (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
fixed
beta+


Attachments
patch (2.06 KB, patch)
2012-02-03 13:47 PST, :Margaret Leibovic
mark.finkle: review+
rnewman: review+
akeybl: approval‑mozilla‑aurora+
akeybl: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Richard Newman [:rnewman] 2012-02-03 11:06:14 PST
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?) 2012-02-03 12:13:36 PST
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 2012-02-03 13:47:20 PST
Created attachment 594295 [details] [diff] [review]
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?) 2012-02-03 14:00:53 PST
Comment on attachment 594295 [details] [diff] [review]
patch

Thanks Margaret and Richard!
Comment 4 Richard Newman [:rnewman] 2012-02-03 14:01:36 PST
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 2012-02-03 14:29:44 PST
(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 7 :Margaret Leibovic 2012-02-03 14:36:39 PST
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] 2012-02-04 02:39:44 PST
https://hg.mozilla.org/mozilla-central/rev/c6d94101af73
Comment 9 Alex Keybl [:akeybl] 2012-02-05 13:47:07 PST
Comment on attachment 594295 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.
Comment 10 Richard Newman [:rnewman] 2012-02-05 17:35:02 PST
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?) 2012-02-06 13:48:01 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/55756493911f

Note You need to log in before you can comment on or make changes to this bug.