createMobileBookmarksFolder doesn't set title or parent

RESOLVED FIXED in Firefox 11

Status

()

defect
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: rnewman, Assigned: Margaret)

Tracking

unspecified
Firefox 13
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

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

Details

Attachments

(1 attachment)

Reporter

Description

8 years ago
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
Assignee

Comment 2

8 years ago
Posted 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+
Reporter

Comment 4

8 years ago
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+
Assignee

Comment 5

8 years ago
(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 ;)
Assignee

Comment 7

8 years ago
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: 8 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+
Reporter

Comment 10

8 years ago
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+
You need to log in before you can comment on or make changes to this bug.