Closed Bug 724045 Opened 9 years ago Closed 9 years ago

createMobileBookmarksFolder doesn't set title or parent

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set
normal

Tracking

(firefox11 fixed, firefox12 fixed, blocking-fennec1.0 beta+)

RESOLVED FIXED
Firefox 13
Tracking Status
firefox11 --- fixed
firefox12 --- fixed
blocking-fennec1.0 --- beta+

People

(Reporter: rnewman, Assigned: Margaret)

References

Details

Attachments

(1 file)

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.
This sounds like a beta blocker in that we will be screwing up bookmark data.

Margaret - Can you look at this?
Assignee: nobody → margaret.leibovic
Attached patch patchSplinter Review
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.
Attachment #594295 - Flags: review?(mark.finkle)
Comment on attachment 594295 [details] [diff] [review]
patch

Thanks Margaret and Richard!
Attachment #594295 - Flags: review?(mark.finkle) → review+
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.
Attachment #594295 - Flags: review+
(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 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.)
Attachment #594295 - Flags: approval-mozilla-beta?
Attachment #594295 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/c6d94101af73
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 13
Comment on attachment 594295 [details] [diff] [review]
patch

[Triage Comment]
Mobile only - approved for Aurora 12 and Beta 11.
Attachment #594295 - Flags: approval-mozilla-beta?
Attachment #594295 - Flags: approval-mozilla-beta+
Attachment #594295 - Flags: approval-mozilla-aurora?
Attachment #594295 - Flags: approval-mozilla-aurora+
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
blocking-fennec1.0: --- → beta+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.