Bug 1234967 - Open correct bookmark in the Edit Bookmark dialog draft
authorAndrzej Hunt <andrzej@ahunt.org>
Tue, 29 Mar 2016 14:22:24 -0700
changeset 450176 e9cab14d0b4b318e1c1789a4c000fe8704319c85
parent 450175 f3dcdfe319b355f47f5944a7609ff593141eb560
child 450191 ebfa7d92ecdcd092084e15aefc342172e1ff9b5e
child 450193 a917d0bb164769d33bafe7a3039d8a881064ed37
push id38772
push userbmo:twointofive@gmail.com
push dateFri, 16 Dec 2016 03:01:04 +0000
bugs1234967, 1232439
milestone53.0a1
Bug 1234967 - Open correct bookmark in the Edit Bookmark dialog Bookmark URLs are not unique, we should be using the ID whenever possible. Note: we'll be cleaning up this dialog in Bug 1232439, we can restrict ourselves to just adding the id/URL differentiation for now. Rebased by Tom Klein. MozReview-Commit-ID: DRMJBhKixc
mobile/android/base/java/org/mozilla/gecko/EditBookmarkDialog.java
mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java
--- a/mobile/android/base/java/org/mozilla/gecko/EditBookmarkDialog.java
+++ b/mobile/android/base/java/org/mozilla/gecko/EditBookmarkDialog.java
@@ -11,16 +11,17 @@ import org.mozilla.gecko.util.ThreadUtil
 import org.mozilla.gecko.util.UIAsyncTask;
 
 import android.app.Activity;
 import android.content.ContentResolver;
 import android.content.Context;
 import android.app.AlertDialog;
 import android.content.DialogInterface;
 import android.database.Cursor;
+import android.support.annotation.NonNull;
 import android.support.design.widget.Snackbar;
 import android.text.Editable;
 import android.text.TextWatcher;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.widget.EditText;
 
 /**
@@ -126,32 +127,49 @@ public class EditBookmarkDialog {
         @Override
         public void onTextChanged(CharSequence s, int start, int before, int count) {
             // Disable if the keyword contains spaces
             mEnabled = (s.toString().trim().indexOf(' ') == -1);
             super.onTextChanged(s, start, before, count);
        }
     }
 
+    public void show(final int id) {
+        show(id, null);
+    }
+
     /**
-     * Show the Edit bookmark dialog for a particular url. If the url is bookmarked multiple times
-     * this will just edit the first instance it finds.
+     * Show the Edit bookmark dialog for a given URL. Use this only if you are able to guarantee
+     * that the URL uniquely identifies a bookmark. If there are multiple bookmarks for this URL,
+     * one of them will be selected at random. Use {@link #show(int) show} instead.
      *
      * @param url The url of the bookmark to edit. The dialog will look up other information like the id,
      *            current title, or keywords associated with this url. If the url isn't bookmarked, the
      *            dialog will fail silently. If the url is bookmarked multiple times, this will only show
      *            information about the first it finds.
+     *
+     * @deprecated Bookmarks should be referenced by id if at all possible.
      */
-    public void show(final String url) {
+    @Deprecated
+    public void show(@NonNull final String url) {
+        show(-1, url);
+    }
+
+    private void show(final int id, final String url) {
         final ContentResolver cr = mContext.getContentResolver();
         final BrowserDB db = BrowserDB.from(mContext);
         (new UIAsyncTask.WithoutParams<Bookmark>(ThreadUtils.getBackgroundHandler()) {
             @Override
             public Bookmark doInBackground() {
-                final Cursor cursor = db.getBookmarkForUrl(cr, url);
+                final Cursor cursor;
+                if (url != null) {
+                    cursor = db.getBookmarkForUrl(cr, url);
+                } else {
+                    cursor = db.getBookmarkForID(cr, id);
+                }
                 if (cursor == null) {
                     return null;
                 }
 
                 Bookmark bookmark = null;
                 try {
                     cursor.moveToFirst();
                     bookmark = new Bookmark(cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks._ID)),
--- a/mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/HomeFragment.java
@@ -286,17 +286,17 @@ public abstract class HomeFragment exten
 
             Telemetry.sendUIEvent(TelemetryContract.Event.LOAD_URL, TelemetryContract.Method.CONTEXT_MENU);
 
             return true;
         }
 
         if (itemId == R.id.home_edit_bookmark) {
             // UI Dialog associates to the activity context, not the applications'.
-            new EditBookmarkDialog(context).show(info.url);
+            new EditBookmarkDialog(context).show(info.bookmarkId);
             return true;
         }
 
         if (itemId == R.id.home_remove) {
             // For Top Sites grid items, position is required in case item is Pinned.
             final int position = info instanceof TopSitesGridContextMenuInfo ? info.position : -1;
 
             if (info.hasPartnerBookmarkId()) {