Fix problems with bookmarks

- Fixed incorrect detection of non-bookmarked pages showing
as bookmarked.

- Changed bookmarks to allow any scheme the engine can support

- Changed bookmarks to only look for page url not title
when looking up or adding bookmarks

- Bookmarks will now use new apis to "fix" urls. These are the
same api's the engine uses. This ensures that the bookmarks
db treats urls exactly like the rest of the browser

- Fixed crashes when launching bookmarks via an intent
with wrong parameters

Change-Id: I18d1f5b55c65e6fecf731128cef6d02222fd68a9

4 files changed, 49 insertions, 57 deletions

diff --git a/src/src/com/android/browser/AddBookmarkPage.java b/src/src/com/android/browser/AddBookmarkPage.java
index 00b892ab..d067d192 100644
--- a/src/src/com/android/browser/AddBookmarkPage.java
+++ b/src/src/com/android/browser/AddBookmarkPage.java
@@ -67,6 +67,8 @@ import com.android.browser.platformsupport.WebAddress;
 import com.android.browser.platformsupport.BrowserContract.Accounts;
 import com.android.browser.reflect.ReflectHelper;
 
+import org.codeaurora.swe.util.SWEUrlUtils;
+
 import java.net.URI;
 import java.net.URLEncoder;
 import java.net.URISyntaxException;
@@ -108,7 +110,7 @@ public class AddBookmarkPage extends Activity
     private boolean     mEditingFolder;
     private Bundle      mMap;
     private String      mTouchIconUrl;
-    private String      mOriginalUrl;
+    private String      mUrl;
     private FolderSpinner mFolder;
     private View mDefaultView;
     private View mFolderSelector;
@@ -669,6 +671,7 @@ public class AddBookmarkPage extends Activity
 
         if (mMap != null) {
             Bundle b = mMap.getBundle(EXTRA_EDIT_BOOKMARK);
+            boolean existing = mMap.getBoolean(CHECK_FOR_DUPE, false);
             if (b != null) {
                 mEditingFolder = mMap.getBoolean(EXTRA_IS_FOLDER, false);
                 mMap = b;
@@ -679,6 +682,8 @@ public class AddBookmarkPage extends Activity
                 } else {
                     showRemoveButton();
                 }
+            } else if (existing) {
+                showRemoveButton();
             } else {
                 int gravity = mMap.getInt("gravity", -1);
                 if (gravity != -1) {
@@ -688,7 +693,7 @@ public class AddBookmarkPage extends Activity
                 }
             }
             title = mMap.getString(BrowserContract.Bookmarks.TITLE);
-            url = mOriginalUrl = mMap.getString(BrowserContract.Bookmarks.URL);
+            url = mUrl = UrlUtils.fixUpUrl(mMap.getString(BrowserContract.Bookmarks.URL));
             mTouchIconUrl = mMap.getString(TOUCH_ICON_URL);
             mCurrentFolder = mMap.getLong(BrowserContract.Bookmarks.PARENT, DEFAULT_FOLDER_ID);
 
@@ -920,7 +925,7 @@ public class AddBookmarkPage extends Activity
 
     private void onDeleteWithConfirm() {
         final String title = mTitle.getText().toString().trim();
-        final String unfilteredUrl = UrlUtils.fixUrl(mAddress.getText().toString());
+        final String unfilteredUrl = UrlUtils.fixUpUrl(mAddress.getText().toString());
         final String url = unfilteredUrl.trim();
         new AlertDialog.Builder(this)
                 .setIconAttribute(android.R.attr.alertDialogIcon)
@@ -931,9 +936,9 @@ public class AddBookmarkPage extends Activity
                         ContentResolver cr = getContentResolver();
                         Cursor cursor = cr.query(BrowserContract.Bookmarks.CONTENT_URI,
                                 BookmarksLoader.PROJECTION,
-                                "title = ? OR url = ?",
+                                "url = ?",
                                 new String[] {
-                                        title, url
+                                        url
                                 },
                                 null);
 
@@ -965,18 +970,19 @@ public class AddBookmarkPage extends Activity
     }
 
     private void onSaveWithConfirm() {
-        String title = mTitle.getText().toString().trim();
-        String unfilteredUrl = UrlUtils.fixUrl(mAddress.getText().toString());
+        String unfilteredUrl = UrlUtils.fixUpUrl(mAddress.getText().toString());
         String url = unfilteredUrl.trim();
-        Long id = mMap.getLong(BrowserContract.Bookmarks._ID);
+        Long id = (mMap == null) ?
+                -1 :
+                mMap.getLong(BrowserContract.Bookmarks._ID);
         int duplicateCount;
         final ContentResolver cr = getContentResolver();
 
         Cursor cursor = cr.query(BrowserContract.Bookmarks.CONTENT_URI,
                 BookmarksLoader.PROJECTION,
-                "( title = ? OR url = ? ) AND parent = ?",
+                "url = ? AND parent = ?",
                 new String[] {
-                        title, url, Long.toString(mCurrentFolder)
+                        url, Long.toString(mCurrentFolder)
                 },
                 null);
 
@@ -1032,7 +1038,7 @@ public class AddBookmarkPage extends Activity
         createHandler();
 
         String title = mTitle.getText().toString().trim();
-        String unfilteredUrl = UrlUtils.fixUrl(mAddress.getText().toString());
+        String unfilteredUrl = UrlUtils.fixUpUrl(mAddress.getText().toString());
 
         boolean emptyTitle = title.length() == 0;
         boolean emptyUrl = unfilteredUrl.trim().length() == 0;
@@ -1053,17 +1059,9 @@ public class AddBookmarkPage extends Activity
                 // fail URI parsing, so don't try it if that's the kind of bookmark we have.
 
                 if (!url.toLowerCase().startsWith("javascript:")) {
-                    String encodedUrl = URLEncoder.encode(url, "UTF-8");
-                    URI uriObj = new URI(encodedUrl);
+                    URI uriObj = new URI(url);
                     String scheme = uriObj.getScheme();
-                    if (!Bookmarks.urlHasAcceptableScheme(url)) {
-                        // If the scheme was non-null, let the user know that we
-                        // can't save their bookmark. If it was null, we'll assume
-                        // they meant http when we parse it in the WebAddress class.
-                        if (scheme != null) {
-                            mAddress.setError(r.getText(R.string.bookmark_cannot_save_url));
-                            return false;
-                        }
+                    if (scheme == null) { // SWE will allow bookmarking any scheme
                         WebAddress address;
                         try {
                             address = new WebAddress(unfilteredUrl);
@@ -1079,9 +1077,6 @@ public class AddBookmarkPage extends Activity
             } catch (URISyntaxException e) {
                 mAddress.setError(r.getText(R.string.bookmark_url_not_valid));
                 return false;
-            } catch (UnsupportedEncodingException e) {
-                mAddress.setError(r.getText(R.string.bookmark_url_not_valid));
-                return false;
             }
         }
 
@@ -1089,7 +1084,7 @@ public class AddBookmarkPage extends Activity
             mEditingExisting = false;
         }
 
-        boolean urlUnmodified = url.equals(mOriginalUrl);
+        boolean urlUnmodified = url.equals(mUrl);
 
         if (mEditingExisting) {
             Long id = mMap.getLong(BrowserContract.Bookmarks._ID);
@@ -1266,7 +1261,7 @@ public class AddBookmarkPage extends Activity
         public EditBookmarkInfoLoader(Context context, Bundle bundle) {
             super(context);
             mContext = context.getApplicationContext();
-            mMap = bundle;
+            mMap = (bundle==null) ? new Bundle() : bundle;
         }
 
         @Override
diff --git a/src/src/com/android/browser/Bookmarks.java b/src/src/com/android/browser/Bookmarks.java
index afc99c37..4b740463 100644
--- a/src/src/com/android/browser/Bookmarks.java
+++ b/src/src/com/android/browser/Bookmarks.java
@@ -43,17 +43,6 @@ import java.io.ByteArrayOutputStream;
  *  This class is purely to have a common place for adding/deleting bookmarks.
  */
 public class Bookmarks {
-    // We only want the user to be able to bookmark content that
-    // the browser can handle directly.
-    private static final String acceptableBookmarkSchemes[] = {
-            "http:",
-            "https:",
-            "about:",
-            "data:",
-            "javascript:",
-            "file:",
-            "content:"
-    };
 
     private final static String LOGTAG = "Bookmarks";
     /**
@@ -142,19 +131,6 @@ public class Bookmarks {
         return os.toByteArray();
     }
 
-    /* package */ static boolean urlHasAcceptableScheme(String url) {
-        if (url == null) {
-            return false;
-        }
-
-        for (int i = 0; i < acceptableBookmarkSchemes.length; i++) {
-            if (url.startsWith(acceptableBookmarkSchemes[i])) {
-                return true;
-            }
-        }
-        return false;
-    }
-
     static final String QUERY_BOOKMARKS_WHERE =
             Combined.URL + " == ? OR " +
             Combined.URL + " == ?";
diff --git a/src/src/com/android/browser/Controller.java b/src/src/com/android/browser/Controller.java
index cbc6ba05..fa77d415 100644
--- a/src/src/com/android/browser/Controller.java
+++ b/src/src/com/android/browser/Controller.java
@@ -1939,16 +1939,16 @@ public class Controller
         }
     }
 
-    private int lookupBookmark(String title, String url) {
+    private int lookupBookmark( String url) {
         final ContentResolver cr = getActivity().getContentResolver();
         int count = 0;
         Cursor cursor = null;
         try {
             cursor = cr.query(BrowserContract.Bookmarks.CONTENT_URI,
                     BookmarksLoader.PROJECTION,
-                    "title = ? OR url = ?",
+                    "url = ?",
                     new String[] {
-                        title, url
+                        url
                     },
                     null);
 
@@ -1973,7 +1973,7 @@ public class Controller
 
         String title = w.getTitle();
         String url = w.getUrl();
-        mCurrentPageBookmarked = (lookupBookmark(title, url) > 0);
+        mCurrentPageBookmarked = (lookupBookmark(url) > 0);
         if (title != null && url != null && mCurrentPageBookmarked) {
             bookmark_icon.setChecked(true);
         } else {
@@ -2032,12 +2032,12 @@ public class Controller
         boolean showDebugSettings = mSettings.isDebugEnabled();
         final MenuItem uaSwitcher = menu.findItem(R.id.ua_desktop_menu_id);
         uaSwitcher.setChecked(isDesktopUa);
-        menu.setGroupVisible(R.id.LIVE_MENU, isLive && isLiveScheme);
-        menu.setGroupVisible(R.id.NAV_MENU, isLive && isLiveScheme);
+        setMenuItemVisibility(menu, R.id.find_menu_id, isLive);
+        menu.setGroupVisible(R.id.NAV_MENU, isLive);
         setMenuItemVisibility(menu, R.id.find_menu_id, isLive && isLiveScheme);
         menu.setGroupVisible(R.id.SNAPSHOT_MENU, !isLive);
         setMenuItemVisibility(menu, R.id.add_to_homescreen,
-                isLive && isLiveScheme && isPageFinished);
+                isLive && isPageFinished);
         setMenuItemVisibility(menu, R.id.save_snapshot_menu_id,
                 isLive && ( isLiveScheme || isDistilled ) && isPageFinished && isSavable);
         // history and snapshots item are the members of COMBO menu group,
diff --git a/src/src/com/android/browser/UrlUtils.java b/src/src/com/android/browser/UrlUtils.java
index 4d3dee42..e1101605 100755
--- a/src/src/com/android/browser/UrlUtils.java
+++ b/src/src/com/android/browser/UrlUtils.java
@@ -17,9 +17,12 @@
 package com.android.browser;
 
 import android.net.Uri;
+import android.text.TextUtils;
 import android.util.Patterns;
 import android.webkit.URLUtil;
 
+import org.codeaurora.swe.util.SWEUrlUtils;
+
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 import java.util.HashSet;
@@ -30,6 +33,11 @@ import java.net.URI;
  * Utility methods for Url manipulation
  */
 public class UrlUtils {
+    // Urls defined by the browser should not be "fixedup" via the engine
+    // Urls are only handled by the browser
+    private static final String BROWSER_URLS[] = {
+            "about:debug",
+    };
     public static final String[] DOWNLOADABLE_SCHEMES_VALUES = new String[]
         { "data", "filesystem", "http", "https" };
 
@@ -171,6 +179,19 @@ public class UrlUtils {
         return null;
     }
 
+    public static String fixUpUrl(String url){
+        if (TextUtils.isEmpty(url))
+                return url;
+
+        for (String preDefined: BROWSER_URLS){
+            if (url.contains(preDefined)) {
+                return url;
+            }
+        }
+        return SWEUrlUtils.fixUpUrl(url);
+    }
+
+    @Deprecated // Use fixUpUrl instead
     public static String fixUrl(String inUrl) {
         // FIXME: Converting the url to lower case
         // duplicates functionality in smartUrlFilter().