Last Comment Bug 707150 - Add mechanism to enable/disable Fennec's local bookmarks/history DB
: Add mechanism to enable/disable Fennec's local bookmarks/history DB
Status: VERIFIED FIXED
: feature
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: All Android
: P1 normal (vote)
: Firefox 12
Assigned To: Lucas Rocha (:lucasr)
:
Mentors:
: 717396 (view as bug list)
Depends on: 708651 716089
Blocks: 717396
  Show dependency treegraph
 
Reported: 2011-12-02 05:50 PST by Lucas Rocha (:lucasr)
Modified: 2016-07-29 14:21 PDT (History)
13 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
fixed
verified


Attachments
Enable local bookmarks/history database (1.64 KB, patch)
2011-12-13 12:30 PST, Lucas Rocha (:lucasr)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review
Fix NullPointerException when getting IS_FOLDER (1.59 KB, patch)
2012-01-11 11:35 PST, Lucas Rocha (:lucasr)
blassey.bugs: review+
akeybl: approval‑mozilla‑aurora+
Details | Diff | Splinter Review

Description Lucas Rocha (:lucasr) 2011-12-02 05:50:50 PST

    
Comment 1 Lucas Rocha (:lucasr) 2011-12-08 08:12:21 PST
Context: we've decided to ship Fennec using local DB by default. This bug is about deciding how we're going to expose the ability to use Android's DB to users.
Comment 2 Lucas Rocha (:lucasr) 2011-12-08 09:33:17 PST
Guys, my initial plan is to add a checkbox prefs to switch Android DB on and off. The wording should be something like "Use bookmarks/history from Android" or "Use system's bookmarks/history". Not entirely happy the wording yet. What do you think?
Comment 3 Steffen Wilberg 2011-12-08 12:26:31 PST
Using a checkbox doesn't make it clear what the other option is, i.e. what happens if you don't "Use bookmarks/history from Android". Not using bookmarks at all or what?
(On Android, some checkboxes change their description when toggled, but it's often not clear whether it describes the current state or the action).

I'd suggest a dropdown menu, something like this:
  Bookmarks and History store:
  - Android (shared with other apps)
  - Firefox
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2011-12-08 13:03:08 PST
(In reply to Steffen Wilberg from comment #3)
> Using a checkbox doesn't make it clear what the other option is, i.e. what
> happens if you don't "Use bookmarks/history from Android". Not using
> bookmarks at all or what?
> (On Android, some checkboxes change their description when toggled, but it's
> often not clear whether it describes the current state or the action).
> 
> I'd suggest a dropdown menu, something like this:
>   Bookmarks and History store:
>   - Android (shared with other apps)
>   - Firefox

I think this is a better direction. Still not sure we even want to enable using Android system for v1 (yeah I said it)
Comment 5 Lucas Rocha (:lucasr) 2011-12-13 12:30:17 PST
Created attachment 581374 [details] [diff] [review]
Enable local bookmarks/history database

One-liner to enable local DB. This patch should only land once bug 708651 and bug 690901 are fixed. Otherwise Fennec will simply crash on when you run it for the first time with no existing profile.
Comment 6 Lucas Rocha (:lucasr) 2012-01-10 09:54:17 PST
This is a high priority bug as it's about enabling a core feature for 1.0. The profile directory is now created in Java so we can enable local DB now.
Comment 7 Lucas Rocha (:lucasr) 2012-01-11 10:01:48 PST
In Try now: https://tbpl.mozilla.org/?tree=Try&rev=2ac9167a6f11
Comment 8 Lucas Rocha (:lucasr) 2012-01-11 11:35:31 PST
Created attachment 587767 [details] [diff] [review]
Fix NullPointerException when getting IS_FOLDER

The comparison "== 1" when IS_FOLDER is null is causing a NullPointerException somehow. This patch fixes a regression caused by the patch in bug 716811.
Comment 10 Lucas Rocha (:lucasr) 2012-01-11 12:09:45 PST
Comment on attachment 587767 [details] [diff] [review]
Fix NullPointerException when getting IS_FOLDER

Fixes a ClassCastException and NullPointerException (caused by bug 716811). Adding bookmarks is broken without this patch. Small non-risky patch.
Comment 11 Lucas Rocha (:lucasr) 2012-01-11 12:12:16 PST
Comment on attachment 581374 [details] [diff] [review]
Enable local bookmarks/history database

Enables local DB for bookmarks and history. Hard requirement for 1.0 as Sync on native UI is only implemented for local DB (not for Android DB). Needs to be enabled as soon as possible for general testing.
Comment 12 Alex Keybl [:akeybl] 2012-01-11 13:17:28 PST
Leaving this in the queue to watch for fallout tomorrow on m-c.
Comment 13 Lucas Rocha (:lucasr) 2012-01-12 03:11:49 PST
*** Bug 717396 has been marked as a duplicate of this bug. ***
Comment 15 Aaron Train [:aaronmt] 2012-01-12 10:07:14 PST
What's the UI interaction here? Or is there none?
Comment 16 Lucas Rocha (:lucasr) 2012-01-12 10:12:18 PST
(In reply to Aaron Train [:aaronmt] from comment #15)
> What's the UI interaction here? Or is there none?

No UI interaction. However, one way to verify this is working is to do any set of operations that involve DB access:
- Add and remove bookmarks and see if it appears in awesomescreen's Bookmarks tab
- Access any webpage and see if it appears in awesomescreen's History tab
- Filter results in awesomescreen's search entry
- Clear history
- And so on.
Comment 17 Alex Keybl [:akeybl] 2012-01-12 22:22:07 PST
Comment on attachment 581374 [details] [diff] [review]
Enable local bookmarks/history database

[Triage Comment]
Mobile only - approving for Aurora.
Comment 19 Cristian Nicolae (:xti) 2012-03-02 09:25:58 PST
Verified fixed on:

Firefox 13.0a1 (2012-03-02)
20120302031112
http://hg.mozilla.org/mozilla-central/rev/3a7b9e61c263

--
Device: Samsung Galaxy S2
OS: Android 2.3.4

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