698828 - Adding bookmarks doesn't work (Honeycomb, maybe ICS?)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P3)

Description

[Kartikaya Gupta (email:[e-mail])]

1. Go to a page that is not in the bookmarks
2. Bookmark the page by hitting the "star" icon (you may need to tap the menu to make the star icon appear - see bug 698748)
3. Note that the toast notifications indicates the page has been bookmarked, and the star icon is updated to match.
4. Bring up the awesome bar bookmarks list and look for the page you just bookmarked; it is not there. Expected result: the page should be there.
5. Close the awesome bar and reload the page you were on. Note that the "star" icon again indicates an un-bookmarked page. Expected result: the star icon should indicate the page is bookmarked.

The attachments show a dump of the Browser.BOOKMARKS_URI table as obtained via this line of code:
android.database.DatabaseUtils.dumpCursor(getContentResolver().query(android.provider.Browser.BOOKMARKS_URI, null, null, null, null));

The dump from the "before" attachment was taken after I (1) cleared the history and (2) went to google.com, which redirected me to google.ca. The "after" dump was obtained after hitting the star icon at that point.

However, I observed that if I go to the page, THEN clear the history, and THEN star the page, bookmarking works as expected.

Comment 1

[Kartikaya Gupta (email:[e-mail])]

Attachment: Dump of BOOKMARKS_URI before starring page

Comment 2

[Kartikaya Gupta (email:[e-mail])]

Attachment: Dump of BOOKMARKS_URI after starring page

Comment 3

[Aaron Train [:aaronmt]]

Confirming that on my Samsung Galaxy SII (Android 2.3.4), bookmarks added under native Fennec, are not added in the default "Internet" browser under TouchWiz 4.0.

Comment 4

[Aaron Train [:aaronmt]]

My mistake, this looks like a bug in the TouchWiz 4.0 browser; or just an implementation detail. After adding a bookmark in native Fennec successfully, you can access the bookmark by querying it in the browser; albeit for some reason the bookmark is not displayed in their "Thumbnail View" (where I had first looked).

Comment 6

[Lucas Rocha (:lucasr)]

Taking as I'm working on bug 701913 which is directly related.

Comment 7

[Lucas Rocha (:lucasr)]

Attachment: Fix add bookmark operation in Honeycomb/ICS

Build on top of changes in bug 704490.

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 579094 [details] [diff] [review]
Fix add bookmark operation in Honeycomb/ICS

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

::: mobile/android/base/db/AndroidBrowserDB.java
@@ +183,5 @@
> +        values.put(Browser.BookmarkColumns.BOOKMARK, "1");
> +        values.put(Browser.BookmarkColumns.TITLE, title);
> +        values.put(Browser.BookmarkColumns.URL, uri);
> +
> +        cr.insert(Browser.BOOKMARKS_URI, values);

why don't you have to check that the value exists post 11?

Comment 9

[Lucas Rocha (:lucasr)]

(In reply to Brad Lassey [:blassey] from comment #8)
> Comment on attachment 579094 [details] [diff] [review]
> Fix add bookmark operation in Honeycomb/ICS
> 
> Review of attachment 579094 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/db/AndroidBrowserDB.java
> @@ +183,5 @@
> > +        values.put(Browser.BookmarkColumns.BOOKMARK, "1");
> > +        values.put(Browser.BookmarkColumns.TITLE, title);
> > +        values.put(Browser.BookmarkColumns.URL, uri);
> > +
> > +        cr.insert(Browser.BOOKMARKS_URI, values);
> 
> why don't you have to check that the value exists post 11?

Misread Android's source code a bit. Submitting the correct patch now.

Comment 10

[Lucas Rocha (:lucasr)]

Attachment: Fix add bookmark operation in Honeycomb/ICS

Comment 11

[Lucas Rocha (:lucasr)]

Pushed: http://hg.mozilla.org/mozilla-central/rev/71a7af64da86

Comment 12

[Aaron Train [:aaronmt]]

Samsung Galaxy Tab 10.1 (Android 3.1)
20111213061518
http://hg.mozilla.org/mozilla-central/rev/e79b3396889c

Looking to also verify on ICS.

Comment 13

[Cameron Campbell]

Only way to add bookmarks is to drag a link to the toolbar. Aside from ... This will open your file browser to the current Firefox profile. 

Cameron
https://www.ukacademicwriters.co.uk/