Bug 1234967 - Open correct bookmark in the Edit Bookmark dialog r?rnewman
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.
MozReview-Commit-ID: DRMJBhKixc
--- 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 = GeckoProfile.get(mContext).getDB();
(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
@@ -262,17 +262,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;
(new RemoveItemByUrlTask(context, info, position)).execute();