754224 - Add keyword support to Profile Migration

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Gian-Carlo Pascutto [:gcp]]

Bug 722184 added support for keywords in Native Fennec. We should add support to Profile Migration to migrate keywords as well.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Attachment: WIP. Add keyword support to ProfileMigration

This should fix the issue, but I have problems testing due to another bug in BrowserProvider that triggers with my profile.

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Also prolly needs rebasing due to Bug 754276.

Comment 3

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Support keywords in profile migration

Comment 4

[:Margaret Leibovic]

Comment on attachment 623344 [details] [diff] [review]
Patch 1. Support keywords in profile migration

>diff --git a/mobile/android/base/ProfileMigrator.java b/mobile/android/base/ProfileMigrator.java
>@@ -146,56 +146,60 @@ public class ProfileMigrator {

>     private static final String BOOKMARK_QUERY_TRAILER =
>-        "FROM ((moz_bookmarks AS bookmark "               +
>+        "FROM (((moz_bookmarks AS bookmark "              +
>+        "        LEFT OUTER JOIN moz_keywords AS keyword "+
>+        "        ON keyword.id = bookmark.keyword_id) "   +
>         "       LEFT OUTER JOIN moz_places AS places "    +
>         "       ON places.id = bookmark.fk) "             +
>-        "       LEFT OUTER JOIN moz_favicons AS favicon " +
>-        "       ON places.favicon_id = favicon.id) "      +
>+        "      LEFT OUTER JOIN moz_favicons AS favicon "  +
>+        "      ON places.favicon_id = favicon.id) "       +

So many joins! However, it doesn't look like there's a way around it, and the keyword table will likely be really small or even empty often, so this patch shouldn't add much more cost to this query.

Patch looks good to me!

Comment 5

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1f9d4590d2f0

Comment 6

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 623344 [details] [diff] [review]
Patch 1. Support keywords in profile migration

[Approval Request Comment]
User impact if declined: keywords will not transfer from XUL fennec
Testing completed (on m-c, etc.):  just landed

Comment 7

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/1f9d4590d2f0

Comment 8

[Cristian Nicolae (:xti)]

I am still able to reproduce this issue on the latest Nightly build.

Steps to reproduce:
1. Install and open the latest Nightly XUL build
2. Load and bookmark a webpage
3. Add to it a keyword by going to Awesomescreen > Bookmarks > long tap on the page opened at step 2 > Tap on Edit
4. Quit Fennec
5. Upgrade Fennec to the latest Native Nightly build and open it
6. Go to Awesomebar > Bookmarks and long tap on the bookmarked page at step 2
7. Tap on Edit option

Expected result:
After step 7, the keyword added at step 3 is still present in keyword textfield.

Actual result:
After step 7, the keyword textfield is blank.

--
Firefox 15.0a1 (2012-05-15)
Device: Galaxy Nexus
OS: Android 4.0.2

Comment 9

[Adrian Tamas (:AdrianT)]

I am also able to reproduce the issue described in Comment 8 on Nightly 15.0a1 2012-05-15 on HTC Desire Z(Android 2.3.3)

Comment 10

[Gian-Carlo Pascutto [:gcp]]

I don't see any way to add keywords with XUL Fennec? Doing the steps above just gives a field to add a tag, not a keyword. 

Keywords can be created on desktop Firefox by right-clicking a search field and selecting "Add a keyword for this search", and then sync-ing them over.

We currently don't have complete support tags for in Native Fennec (AFAIK?), so I don't believe they can be tested.

Comment 11

[Kevin Brosnan [:kbrosnan]]

You would need to use Sync to XUL to add keywords.

Comment 12

[Johnathan Nightingale [:johnath]]

I <3 me some keywords, but not a blocker for release.

Comment 13

[Cristian Nicolae (:xti)]

It's not reproducible anymore on the latest Nightly build.

Closing bug as verified fixed on:

Firefox 15.0a1 (2012-05-15)
Device: Samsung Galaxy Nexus
OS: Android 4.0.2

Comment 14

[Alex Keybl [:akeybl]]

Comment on attachment 623344 [details] [diff] [review]
Patch 1. Support keywords in profile migration

Not a Fennec 1.0 blocker, therefore it can ride the trains.

Comment 15

[Gian-Carlo Pascutto [:gcp]]

Note that XUL->Native migration bugs cannot really ride the trains in a meaningful manner. Fixing them after users have transitioned to native won't have any effect.

I believe you can only get keywords in XUL profiles by syncing them over, so users suffering from this are likely using Sync, and perhaps Sync is able to recover the lost keywords. In that case, WONTFIXing this bug should be quite reasonable. If not, perhaps we should consider the above.

Comment 16

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/4e6bac3417e0

Comment 17

[Cristian Nicolae (:xti)]

Verified fixed on:

Firefox 14.0a2 (2012-05-24)
Firefox 14.0b3 (2012-05-25)

Device: Galaxy Nexus
OS: Android 4.0.2