786926 - updateBookmark doesn't set DATE_MODIFIED

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Data Providers, defect)

Description

[Richard Newman [:rnewman]]

public void updateBookmark(ContentResolver cr, int id, String uri, String title, String keyword) {
        ContentValues values = new ContentValues();
        values.put(Browser.BookmarkColumns.TITLE, title);
        values.put(Bookmarks.URL, uri);
        values.put(Bookmarks.KEYWORD, keyword);

        cr.update(mBookmarksUriWithProfile,
                  values,
                  Bookmarks._ID + " = ?",
                  new String[] { String.valueOf(id) });
    }

This means that changes to title etc. during bookmark edits don't propagate to other devices.

(Reported by user on IRC.)

Comment 1

[Richard Newman [:rnewman]]

Flagging the tap-and-hold, "Edit Bookmark" path for future QA.

Comment 2

[Richard Newman [:rnewman]]

Attachment: Draft patch. v1

Oh hey, look! A one line fix and fifty lines of tests.

This builds, but I can't actually run the test until Bug 787931 is resolved. Feedback welcome on that :D

Comment 3

[Richard Newman [:rnewman]]

Oh, and I did a tiny amount of cleanup in that test file, too. Couldn't resist.

Comment 4

[Richard Newman [:rnewman]]

try, Y U NO RUN ROBOTIUM SO I CAN SEE?

https://tbpl.mozilla.org/?tree=Try&rev=08d85dc1bc4d

Comment 5

[Richard Newman [:rnewman]]

Let's have a try build that will actually run tests.

https://tbpl.mozilla.org/?tree=Try&rev=fcbf98f585d6

Comment 6

[Richard Newman [:rnewman]]

Comment on attachment 658149 [details] [diff] [review]
Draft patch. v1

Let's just land the fix, and I'll land the test when I get a chance to finish it!

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 658149 [details] [diff] [review]
Draft patch. v1

>diff --git a/mobile/android/base/db/LocalBrowserDB.java b/mobile/android/base/db/LocalBrowserDB.java

>         values.put(Bookmarks.KEYWORD, keyword);
>+        values.put(Bookmarks.DATE_MODIFIED, System.currentTimeMillis());

I assume you'll only land this part and not the test changes. The test changes will be a separate patch.

Comment 8

[Richard Newman [:rnewman]]

Code-only change:

https://hg.mozilla.org/integration/mozilla-inbound/rev/77b4e5ec2e78

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77b4e5ec2e78