Closed Bug 752444 Opened 8 years ago Closed 8 years ago

Mobile and default bookmarks are missing after migration

Categories

(Firefox for Android :: General, defect, critical)

ARM
Android
defect
Not set
critical

Tracking

()

VERIFIED FIXED
Firefox 15
Tracking Status
firefox14 --- verified
firefox15 --- verified
blocking-fennec1.0 --- beta+

People

(Reporter: xti, Assigned: gcp)

References

Details

(Keywords: dataloss)

Attachments

(4 files)

Device: Samsung Captivate
OS: Android 2.2

Steps to reproduce:
1. Install the latest Nightly XUL build
2. Open Fennec
3. Go to cnn.com and bookmark the page
4. Close Fennec
5. Upgrade to the latest Native Nightly build
6. Open Fennec and go to AwesomeBar > Bookmarks

Expected result:
After step 6, besides all default bookmarks (about:home, about:firefox, AMO and Support links), there is listed the bookmarked page at step 3 as well. Desktop Bookmarks folder should not be displayed after migration since Sync was not involved at all.

Actual result:
After step 6:
- all default bookmarks are missing except AMO
- Desktop Bookmarks directory was created, even if Sync was not involved. All sub-directories are empty.
- the page bookmarked at step 3, is not listed in Bookmarks tab. However, if I load the page from Fennec History, the bookmark star is yellow colored, so the page is still bookmarked.
On HTC Desire (Android 2.2)  Nightly 15.0a1 XUL 2012-05-06 / Nightly 15.0a1 Native 2012-05-06, I get the same behavior if I don't have sync set up. However if sync is set up before migration all mobile bookmarks are gone including the AMO page. The Desktop bookmarks are correctly displayed.
Attached image screenshot
Christian, logcat output would be useful here btw.
Attached file captivate_log
Keywords: dataloss
blocking-fennec1.0: --- → ?
FWIW, I can reproduce this as well.
I think the problem is this: Profile Migration checks moz_bookmarks_roots. It then remaps those places root folders to the root folders in LocalDB, and drops everything that doesn't have a known root folder. 

However, the "Mobile" folder is not listed in moz_bookmarks_roots. This is perhaps a bug in XUL Fennec?
The Desktop Bookmarks folder is displayed because the other root folders are migrated, and LocalBrowserDB checks for their presence to present the Desktop Bookmarks subfolder.

Because these are present in the old places db, they will end up existing in every XUL profile, even if there were no "real" bookmarks added below them.
QAwanted to retest on a couple devices and see how wide the scope of this is.
Keywords: qawanted
It's dataloss for every user who has mobile bookmarks. I'll patch the missing Mobile root in Profile Migrator. 

Not sure where to best fix the Desktop Bookmarks issue, doesn't look so easy in Migrator, maybe better in LocalBrowserDB or even on the UI side. But that's not dataloss so much less impact.
OK that is what we thought on triage. We just wanted to make sure this was as bad as the bug read.
Keywords: qawanted
Taking, though I'll split the cosmetic part (empty Desktop Bookmarks still showing) to a separate bug.
Assignee: nobody → gpascutto
Split the display issue in Bug 753534.
Summary: Default bookmarks are missing (except AMO) and Desktop Bookmarks folder is created after migration from XUL to Native → Default bookmarks are missing (except AMO)
(In reply to Gian-Carlo Pascutto (:gcp) from comment #9)

> Not sure where to best fix the Desktop Bookmarks issue, doesn't look so easy
> in Migrator, maybe better in LocalBrowserDB or even on the UI side. But
> that's not dataloss so much less impact.

Instead of checking if there are bookmarks with parents other than the mobile/root folders, we could explicitly check if there are bookmarks with the toolbar/menu/unsorted parents, since that's actually what we're using in the UI. That would just involve editing this query:
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/LocalBrowserDB.java#351
(In reply to Margaret Leibovic [:margaret] from comment #13)
> (In reply to Gian-Carlo Pascutto (:gcp) from comment #9)
> 
> > Not sure where to best fix the Desktop Bookmarks issue, doesn't look so easy
> > in Migrator, maybe better in LocalBrowserDB or even on the UI side. But
> > that's not dataloss so much less impact.
> 
> Instead of checking if there are bookmarks with parents other than the
> mobile/root folders, we could explicitly check if there are bookmarks with
> the toolbar/menu/unsorted parents, since that's actually what we're using in
> the UI. That would just involve editing this query:
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/db/
> LocalBrowserDB.java#351

Actually, it's not completely straightforward, since we'd need to get the IDs for the toolbar/menu/unsorted folders to do that, but we're already doing that for the mobile bookmarks folder, so it's not impossible.
blocking-fennec1.0: ? → betaN+
Wierd.  This might be a separate bug.

STR:
1) have native nightly installed w/ Sync setup
2) uninstall any nightly
3) install XUL nightly 5/8/2012
4) go to ilm.com and bookmark
5) quit
6) adb install -r fennec-15.0a1.en-US.nightly_05_08_2012.apk 
7) look in awesome page for ilm.com bookmark

Expected: ilm.com bookmark in the bookmark category
Actual: Top sites and History shows www.ilm.com bookmarked; Bookmarks does not show www.ilm.com in any folder

It tried to Sync my desktop w/ nothing... I see a Desktop Bookmarks folder instead of my default bookmarks

Within my log I found this:
05-10 06:56:03.140: I/dalvikvm(30473): Could not find method android.database.sqlite.SQLiteDatabase.enableWriteAheadLogging, referenced from method org.mozilla.fennec.db.BrowserProvider$DatabaseHelper.onOpen
05-10 06:56:03.140: W/dalvikvm(30473): VFY: unable to resolve virtual method 277: Landroid/database/sqlite/SQLiteDatabase;.enableWriteAheadLogging ()Z
05-10 06:56:03.140: D/dalvikvm(30473): VFY: replacing opcode 0x6e at 0x0033

As well as this :
05-10 06:56:03.605: V/Telemetry(30473): Sending telemetry: {"value":11,"name":"BROWSERPROVIDER_XUL_IMPORT_BOOKMARKS"}
05-10 06:56:03.605: I/ProfileMigrator(30473): Flushing 1 DB operations

~~~~~
1) is Sync uncoupling itself completely?
2) should migration create folders for Desktop Bookmarks if Sync isn't attached?
>1) is Sync uncoupling itself completely?

Your Sync migration failed because of bug 753175.

>2) should migration create folders for Desktop Bookmarks if Sync isn't attached?

Migration just migrates what's in the profile, and the profile has them. Displaying those empty folders or not is discussed in bug 753534 :)
(In reply to Gian-Carlo Pascutto (:gcp) from comment #16)
> >1) is Sync uncoupling itself completely?
> 
> Your Sync migration failed because of bug 753175.

The problem is that I didn't use sync at all in the migration.

> >2) should migration create folders for Desktop Bookmarks if Sync isn't attached?
> 
> Migration just migrates what's in the profile, and the profile has them.
> Displaying those empty folders or not is discussed in bug 753534 :)

When it has a fresh profile and has no sync installed?
>The problem is that I didn't use sync at all in the migration.

My mistake. The bug I mentioned stops Sync preferences from getting read, which is indistinguishable from not having any :P

>When it has a fresh profile and has no sync installed?

The display of the Desktop Bookmarks folder is triggered by the existence of some other folders, which are always present in XUL Fennec DB's and get migrated.
Ok, thanks for the explanation.  So it was only confusing because the end user didn't see the same structure that they saw in XUL; so in a sense it's more or less a minor UI bug.
[Approval Request Comment]
User impact if declined: Loss of all mobile bookmarks if Sync is not used.
Attachment #622588 - Flags: review?(lucasr.at.mozilla)
Attachment #622588 - Flags: approval-mozilla-aurora?
(In reply to Gian-Carlo Pascutto (:gcp) from comment #20)
> Created attachment 622588 [details] [diff] [review]
> Patch 1. Fix and add missing Mobile bookmarks root.
> 
> [Approval Request Comment]
> User impact if declined: Loss of all mobile bookmarks if Sync is not used.

Including user created ones, correct?  We should update the bug summary if that is the case and consider this for a beta respin.
Summary: Default bookmarks are missing (except AMO) → Mobile and default bookmarks are missing after migration
Flagged to release-drivers for possible beta 1 respin.
Comment on attachment 622588 [details] [diff] [review]
Patch 1. Fix and add missing Mobile bookmarks root.

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

Looks good.

::: mobile/android/base/ProfileMigrator.java
@@ +819,5 @@
>                  }
>                  cursor.close();
> +
> +                // XUL Fennec doesn't mark the Mobile folder as a root,
> +                // so fix that up here.

Maybe a file a bug for that? Sounds like something that needs fixing on desktop Firefox.
Attachment #622588 - Flags: review?(lucasr.at.mozilla) → review+
>Maybe a file a bug for that? Sounds like something that needs fixing on desktop 
>Firefox.

Desktop Places DBs don't have a Mobile root folder. (That's one of the main reasons why this slipped by - I did a lot of my testing with huge desktop profiles)

Maybe it's worth fixing in XUL Fennec.
Filed Bug 753789 for the root cause.
We'll need this in a beta 1 respin.
blocking-fennec1.0: betaN+ → beta+
QA, please smoke test this.
Keywords: qawanted
Comment on attachment 622588 [details] [diff] [review]
Patch 1. Fix and add missing Mobile bookmarks root.

For beta 1 respin.
Attachment #622588 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/mozilla-central/rev/1df2b3ab08cf
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 15
Blocks: 754236
This bug is not reproducible any more on the latest Nightly and Native Beta build 3: http://s13.postimage.org/ay6hih8j9/device_2012_05_11_142027.png

The only noticeable thing is that the keywords are not migrated on Native ( Bug 754224 ). Also AMO/mobile seems that's still present for Native ( Bug 754236 ). Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-11), Firefox 14.b3
Samsung Galaxy Nexus
Android: 4.0.2
Status: RESOLVED → VERIFIED
Keywords: qawanted
You need to log in before you can comment on or make changes to this bug.