Bug 897250 - Fix the "Remove" context menu item to correctly remove either a bookmark or history entry. r=wesj

This commit is contained in:
Margaret Leibovic 2013-07-29 09:56:59 -07:00
parent 3284beab5e
commit d099695ad8
5 changed files with 121 additions and 58 deletions

View File

@ -19,7 +19,8 @@ import org.mozilla.gecko.ReaderModeUtils;
import org.mozilla.gecko.util.ThreadUtils;
import org.mozilla.gecko.util.UiAsyncTask;
import android.app.Activity;
import android.content.ContentResolver;
import android.content.Context;
import android.content.Intent;
import android.graphics.Bitmap;
import android.support.v4.app.Fragment;
@ -68,13 +69,17 @@ abstract class HomeFragment extends Fragment {
MenuInflater inflater = new MenuInflater(view.getContext());
inflater.inflate(R.menu.home_contextmenu, menu);
// Hide "Remove" item if there isn't a valid history ID
if (info.rowId < 0) {
menu.findItem(R.id.home_remove_history).setVisible(false);
}
menu.setHeaderTitle(info.title);
menu.findItem(R.id.home_remove_history).setVisible(false);
// Hide the "Edit" menuitem if this item isn't a bookmark.
if (info.bookmarkId < 0) {
menu.findItem(R.id.home_edit_bookmark).setVisible(false);
}
// Hide the "Remove" menuitem if this item doesn't have a bookmark or history ID.
if (info.bookmarkId < 0 && info.historyId < 0) {
menu.findItem(R.id.home_remove).setVisible(false);
}
final boolean canOpenInReader = (info.display == Combined.DISPLAY_READER);
menu.findItem(R.id.home_open_in_reader).setVisible(canOpenInReader);
@ -93,7 +98,7 @@ abstract class HomeFragment extends Fragment {
}
HomeContextMenuInfo info = (HomeContextMenuInfo) menuInfo;
final Activity activity = getActivity();
final Context context = getActivity().getApplicationContext();
switch(item.getItemId()) {
case R.id.home_share: {
@ -113,6 +118,7 @@ abstract class HomeFragment extends Fragment {
break;
}
// FIXME: bug 897772
Bitmap bitmap = null;
if (info.favicon != null && info.favicon.length > 0) {
bitmap = BitmapUtils.decodeByteArray(info.favicon);
@ -136,12 +142,12 @@ abstract class HomeFragment extends Fragment {
final String url = (info.inReadingList ? ReaderModeUtils.getAboutReaderForUrl(info.url, true) : info.url);
Tabs.getInstance().loadUrl(url, flags);
Toast.makeText(activity, R.string.new_tab_opened, Toast.LENGTH_SHORT).show();
Toast.makeText(context, R.string.new_tab_opened, Toast.LENGTH_SHORT).show();
return true;
}
case R.id.home_edit_bookmark: {
new EditBookmarkDialog(activity).show(info.url);
new EditBookmarkDialog(context).show(info.url);
return true;
}
@ -151,40 +157,84 @@ abstract class HomeFragment extends Fragment {
return true;
}
case R.id.home_remove_bookmark: {
final int rowId = info.rowId;
final String url = info.url;
final boolean inReadingList = info.inReadingList;
case R.id.home_remove: {
// Prioritize removing a history entry over a bookmark in the case of a combined item.
final int historyId = info.historyId;
if (historyId > -1) {
new RemoveHistoryTask(context, historyId).execute();
return true;
}
(new UiAsyncTask<Void, Void, Integer>(ThreadUtils.getBackgroundHandler()) {
@Override
public Integer doInBackground(Void... params) {
BrowserDB.removeBookmark(activity.getContentResolver(), rowId);
return BrowserDB.getReadingListCount(activity.getContentResolver());
}
@Override
public void onPostExecute(Integer aCount) {
int messageId = R.string.bookmark_removed;
if (inReadingList) {
messageId = R.string.reading_list_removed;
GeckoEvent e = GeckoEvent.createBroadcastEvent("Reader:Remove", url);
GeckoAppShell.sendEventToGecko(e);
e = GeckoEvent.createBroadcastEvent("Reader:ListCountUpdated", Integer.toString(aCount));
GeckoAppShell.sendEventToGecko(e);
}
Toast.makeText(activity, messageId, Toast.LENGTH_SHORT).show();
}
}).execute();
return true;
final int bookmarkId = info.bookmarkId;
if (bookmarkId > -1) {
new RemoveBookmarkTask(context, bookmarkId, info.url, info.inReadingList).execute();
return true;
}
}
}
return false;
}
private static class RemoveBookmarkTask extends UiAsyncTask<Void, Void, Void> {
private final Context mContext;
private final int mId;
private final String mUrl;
private final boolean mInReadingList;
public RemoveBookmarkTask(Context context, int id, String url, boolean inReadingList) {
super(ThreadUtils.getBackgroundHandler());
mContext = context;
mId = id;
mUrl = url;
mInReadingList = inReadingList;
}
@Override
public Void doInBackground(Void... params) {
ContentResolver cr = mContext.getContentResolver();
BrowserDB.removeBookmark(cr, mId);
if (mInReadingList) {
GeckoEvent e = GeckoEvent.createBroadcastEvent("Reader:Remove", mUrl);
GeckoAppShell.sendEventToGecko(e);
int count = BrowserDB.getReadingListCount(cr);
e = GeckoEvent.createBroadcastEvent("Reader:ListCountUpdated", Integer.toString(count));
GeckoAppShell.sendEventToGecko(e);
}
return null;
}
@Override
public void onPostExecute(Void result) {
int messageId = mInReadingList ? R.string.reading_list_removed : R.string.bookmark_removed;
Toast.makeText(mContext, messageId, Toast.LENGTH_SHORT).show();
}
}
private static class RemoveHistoryTask extends UiAsyncTask<Void, Void, Void> {
private final Context mContext;
private final int mId;
public RemoveHistoryTask(Context context, int id) {
super(ThreadUtils.getBackgroundHandler());
mContext = context;
mId = id;
}
@Override
public Void doInBackground(Void... params) {
BrowserDB.removeHistoryEntry(mContext.getContentResolver(), mId);
return null;
}
@Override
public void onPostExecute(Void result) {
Toast.makeText(mContext, R.string.history_removed, Toast.LENGTH_SHORT).show();
}
}
@Override
public void setUserVisibleHint (boolean isVisibleToUser) {
super.setUserVisibleHint(isVisibleToUser);

View File

@ -8,7 +8,7 @@ package org.mozilla.gecko.home;
import org.mozilla.gecko.R;
import org.mozilla.gecko.db.BrowserContract.Bookmarks;
import org.mozilla.gecko.db.BrowserContract.Combined;
import org.mozilla.gecko.db.BrowserDB.URLColumns;
import org.mozilla.gecko.db.BrowserContract.URLColumns;
import org.mozilla.gecko.home.HomePager.OnUrlOpenListener;
import android.content.Context;
@ -87,15 +87,19 @@ public class HomeListView extends ListView
* A ContextMenuInfo for HomeListView that adds details from the cursor.
*/
public static class HomeContextMenuInfo extends AdapterContextMenuInfo {
public int rowId;
public int bookmarkId;
public int historyId;
public String url;
public byte[] favicon;
public String title;
public String keyword;
public int display;
public boolean isFolder;
public boolean inReadingList;
/**
* This constructor assumes that the cursor was generated from a query
* to either the combined view or the bookmarks table.
*/
public HomeContextMenuInfo(View targetView, int position, long id, Cursor cursor) {
super(targetView, position, id);
@ -110,28 +114,42 @@ public class HomeListView extends ListView
isFolder = false;
}
// We don't show a context menu for folders, so return early.
if (isFolder) {
return;
}
int keywordCol = cursor.getColumnIndex(URLColumns.KEYWORD);
if (keywordCol != -1) {
keyword = cursor.getString(keywordCol);
url = cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.URL));
title = cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.TITLE));
final int bookmarkIdCol = cursor.getColumnIndex(Combined.BOOKMARK_ID);
if (bookmarkIdCol == -1) {
// If there isn't a bookmark ID column, this must be a bookmarks cursor,
// so the regular ID column will correspond to a bookmark ID.
bookmarkId = cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks._ID));
} else if (cursor.isNull(bookmarkIdCol)) {
// If this is a combined cursor, we may get a history item without a
// bookmark, in which case the bookmarks ID column value will be null.
bookmarkId = -1;
} else {
keyword = null;
bookmarkId = cursor.getInt(bookmarkIdCol);
}
final int faviconCol = cursor.getColumnIndex(URLColumns.FAVICON);
final int historyIdCol = cursor.getColumnIndex(Combined.HISTORY_ID);
if (historyIdCol != -1) {
historyId = cursor.getInt(historyIdCol);
} else {
historyId = -1;
}
final int faviconCol = cursor.getColumnIndex(Combined.FAVICON);
if (faviconCol != -1) {
favicon = cursor.getBlob(faviconCol);
} else {
favicon = null;
}
rowId = cursor.getInt(cursor.getColumnIndexOrThrow(Bookmarks._ID));
url = cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.URL));
title = cursor.getString(cursor.getColumnIndexOrThrow(URLColumns.TITLE));
// We only have the parent column in cursors from getBookmarksInFolder.
final int parentCol = cursor.getColumnIndex(Bookmarks.PARENT);
if (parentCol != -1) {
inReadingList = (cursor.getInt(parentCol) == Bookmarks.FIXED_READING_LIST_ID);

View File

@ -213,8 +213,7 @@ size. -->
<!ENTITY contextmenu_open_new_tab "Open in New Tab">
<!ENTITY contextmenu_open_private_tab "Open in Private Tab">
<!ENTITY contextmenu_open_in_reader "Open in Reader">
<!ENTITY contextmenu_remove_history "Remove">
<!ENTITY contextmenu_remove_bookmark "Remove">
<!ENTITY contextmenu_remove "Remove">
<!ENTITY contextmenu_add_to_launcher "Add to Home Screen">
<!ENTITY contextmenu_share "Share">
<!ENTITY contextmenu_pasteandgo "Paste &amp; Go">

View File

@ -20,11 +20,8 @@
<item android:id="@+id/home_edit_bookmark"
android:title="@string/contextmenu_edit_bookmark"/>
<item android:id="@+id/home_remove_history"
android:title="@string/contextmenu_remove_history"/>
<item android:id="@+id/home_remove_bookmark"
android:title="@string/contextmenu_remove_bookmark"/>
<item android:id="@+id/home_remove"
android:title="@string/contextmenu_remove"/>
<item android:id="@+id/home_add_to_launcher"
android:title="@string/contextmenu_add_to_launcher"/>

View File

@ -197,8 +197,7 @@
<string name="contextmenu_open_new_tab">&contextmenu_open_new_tab;</string>
<string name="contextmenu_open_private_tab">&contextmenu_open_private_tab;</string>
<string name="contextmenu_open_in_reader">&contextmenu_open_in_reader;</string>
<string name="contextmenu_remove_history">&contextmenu_remove_history;</string>
<string name="contextmenu_remove_bookmark">&contextmenu_remove_bookmark;</string>
<string name="contextmenu_remove">&contextmenu_remove;</string>
<string name="contextmenu_add_to_launcher">&contextmenu_add_to_launcher;</string>
<string name="contextmenu_share">&contextmenu_share;</string>
<string name="contextmenu_pasteandgo">&contextmenu_pasteandgo;</string>