In History context menu, do not show "Add bookmark" for bookmarks.

Fix for http://b/issue?id=1688867.  Instead of providing an option
to add a bookmark to bookmarks, provide an option to remove the
item from bookmarks.  Also display the item as the context menu
header.

diff --git a/res/values/strings.xml b/res/values/strings.xml
index f2220c4..b108bbc 100644 (file)
--- a/res/values/strings.xml
+++ b/res/values/strings.xml
@@ -144,6 +144,8 @@
     <string name="open_bookmark">Open</string>
     <!-- Menu item to remove the currently highlighted bookmark-->
     <string name="remove_bookmark">Delete bookmark</string>
+    <!-- Context menu item to remove a history item from bookmarks -->
+    <string name="remove_from_bookmarks">Remove from bookmarks</string>
     <!-- Menu item to remove the currently highlighted history entry from the list of previously visited sites -->
     <string name="remove_history_item">Remove from history</string>
     <!-- Context menu item for setting the bookmark/history item as the homepage -->

diff --git a/src/com/android/browser/BrowserHistoryPage.java b/src/com/android/browser/BrowserHistoryPage.java
index d5e7049..335d8fe 100644 (file)
--- a/src/com/android/browser/BrowserHistoryPage.java
+++ b/src/com/android/browser/BrowserHistoryPage.java
@@ -62,6 +62,7 @@ public class BrowserHistoryPage extends ExpandableListActivity {
     private HistoryAdapter          mAdapter;
     private DateSorter              mDateSorter;
     private boolean                 mMaxTabsOpen;
+    private HistoryItem             mContextHeader;
 
     private final static String LOGTAG = "browser";
 
@@ -166,7 +167,7 @@ public class BrowserHistoryPage extends ExpandableListActivity {
         }  
         return super.onOptionsItemSelected(item);
     }
-    
+
     @Override
     public void onCreateContextMenu(ContextMenu menu, View v,
             ContextMenuInfo menuInfo) {
@@ -181,12 +182,25 @@ public class BrowserHistoryPage extends ExpandableListActivity {
         MenuInflater inflater = getMenuInflater();
         inflater.inflate(R.menu.historycontext, menu);
 
+        HistoryItem historyItem = (HistoryItem) i.targetView;
+
         // Setup the header
-        menu.setHeaderTitle(((HistoryItem)i.targetView).getUrl());
+        if (mContextHeader == null) {
+            mContextHeader = new HistoryItem(this);
+        } else if (mContextHeader.getParent() != null) {
+            ((ViewGroup) mContextHeader.getParent()).removeView(mContextHeader);
+        }
+        historyItem.copyTo(mContextHeader);
+        menu.setHeaderView(mContextHeader);
 
         // Only show open in new tab if we have not maxed out available tabs
         menu.findItem(R.id.new_window_context_menu_id).setVisible(!mMaxTabsOpen);
         
+        // For a bookmark, provide the option to remove it from bookmarks
+        if (historyItem.isBookmark()) {
+            MenuItem item = menu.findItem(R.id.save_to_bookmarks_menu_id);
+            item.setTitle(R.string.remove_from_bookmarks);
+        }
         // decide whether to show the share link option
         PackageManager pm = getPackageManager();
         Intent send = new Intent(Intent.ACTION_SEND);
@@ -201,8 +215,9 @@ public class BrowserHistoryPage extends ExpandableListActivity {
     public boolean onContextItemSelected(MenuItem item) {
         ExpandableListContextMenuInfo i = 
             (ExpandableListContextMenuInfo) item.getMenuInfo();
-        String url = ((HistoryItem)i.targetView).getUrl();
-        String title = ((HistoryItem)i.targetView).getName();
+        HistoryItem historyItem = (HistoryItem) i.targetView;
+        String url = historyItem.getUrl();
+        String title = historyItem.getName();
         switch (item.getItemId()) {
             case R.id.open_context_menu_id:
                 loadUrl(url, false);
@@ -211,7 +226,12 @@ public class BrowserHistoryPage extends ExpandableListActivity {
                 loadUrl(url, true);
                 return true;
             case R.id.save_to_bookmarks_menu_id:
-                Browser.saveBookmark(this, title, url);
+                if (historyItem.isBookmark()) {
+                    Bookmarks.removeFromBookmarks(this, getContentResolver(),
+                            url);
+                } else {
+                    Browser.saveBookmark(this, title, url);
+                }
                 return true;
             case R.id.share_link_context_menu_id:
                 Browser.sendString(this, url);

diff --git a/src/com/android/browser/HistoryItem.java b/src/com/android/browser/HistoryItem.java
index 8a994f3..e8f15b1 100644 (file)
--- a/src/com/android/browser/HistoryItem.java
+++ b/src/com/android/browser/HistoryItem.java
@@ -55,7 +55,7 @@ import android.widget.TextView;
         };
     }
     
-    void copyTo(HistoryItem item) {
+    /* package */ void copyTo(HistoryItem item) {
         item.mTextView.setText(mTextView.getText());
         item.mUrlText.setText(mUrlText.getText());
         item.setIsBookmark(mStar.isChecked());
@@ -63,10 +63,17 @@ import android.widget.TextView;
     }
 
     /**
+     * Whether or not this item represents a bookmarked site
+     */
+    /* package */ boolean isBookmark() {
+        return mStar.isChecked();
+    }
+
+    /**
      *  Set whether or not this represents a bookmark, and make sure the star
      *  behaves appropriately.
      */
-    void setIsBookmark(boolean isBookmark) {
+    /* package */ void setIsBookmark(boolean isBookmark) {
         mStar.setOnCheckedChangeListener(null);
         mStar.setChecked(isBookmark);
         mStar.setOnCheckedChangeListener(mListener);