Bug 1243558 - Add Screenshots smart folder & unpolished screenshot views. r=sebastian draft
authorMichael Comella <michael.l.comella@gmail.com>
Fri, 26 Feb 2016 18:09:48 -0800
changeset 335635 393684f11e2d69f98f49eff9859e1ab21738979f
parent 335634 7521f7d902e9a70adfb49d0b4dfa2be4917c9287
child 335636 bef412dd8ad5ad80f721c3b6417db5c9295267d1
child 335640 79aaede891025761122e154a9b8c51623eebcc53
push id11839
push usermichael.l.comella@gmail.com
push dateTue, 01 Mar 2016 01:32:00 +0000
reviewerssebastian
bugs1243558
milestone47.0a1
Bug 1243558 - Add Screenshots smart folder & unpolished screenshot views. r=sebastian Modifying the Bookmarks Adapter & wrapped Cursor felt all-over-the place so I'm concerned the resulting code may be fragile. However, I can't see a better solution off the top of my head, meaning it's probably not worth the time to improve. MozReview-Commit-ID: QgnsMQ3smB
mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
mobile/android/base/java/org/mozilla/gecko/home/BookmarksListAdapter.java
mobile/android/base/locales/en-US/android_strings.dtd
mobile/android/base/resources/layout/bookmark_screenshot_row.xml
mobile/android/base/strings.xml.in
--- a/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/BrowserContract.java
@@ -145,25 +145,27 @@ public class BrowserContract {
         public static final String TABLE_NAME = "bookmarks";
 
         public static final String VIEW_WITH_FAVICONS = "bookmarks_with_favicons";
 
         public static final int FIXED_ROOT_ID = 0;
         public static final int FAKE_DESKTOP_FOLDER_ID = -1;
         public static final int FIXED_READING_LIST_ID = -2;
         public static final int FIXED_PINNED_LIST_ID = -3;
+        public static final int FIXED_SCREENSHOT_FOLDER_ID = -4;
 
         public static final String MOBILE_FOLDER_GUID = "mobile";
         public static final String PLACES_FOLDER_GUID = "places";
         public static final String MENU_FOLDER_GUID = "menu";
         public static final String TAGS_FOLDER_GUID = "tags";
         public static final String TOOLBAR_FOLDER_GUID = "toolbar";
         public static final String UNFILED_FOLDER_GUID = "unfiled";
         public static final String FAKE_DESKTOP_FOLDER_GUID = "desktop";
         public static final String PINNED_FOLDER_GUID = "pinned";
+        public static final String SCREENSHOT_FOLDER_GUID = "screenshots";
 
         public static final int TYPE_FOLDER = 0;
         public static final int TYPE_BOOKMARK = 1;
         public static final int TYPE_SEPARATOR = 2;
         public static final int TYPE_LIVEMARK = 3;
         public static final int TYPE_QUERY = 4;
 
         public static final Uri CONTENT_URI = Uri.withAppendedPath(AUTHORITY_URI, "bookmarks");
--- a/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
+++ b/mobile/android/base/java/org/mozilla/gecko/db/LocalBrowserDB.java
@@ -783,61 +783,62 @@ public class LocalBrowserDB implements B
             cr.delete(mHistoryUriWithProfile, null, null);
         }
     }
 
     @Override
     @RobocopTarget
     public Cursor getBookmarksInFolder(ContentResolver cr, long folderId) {
         final boolean addDesktopFolder;
+        final boolean addScreenshotsFolder;
 
         // We always want to show mobile bookmarks in the root view.
         if (folderId == Bookmarks.FIXED_ROOT_ID) {
             folderId = getFolderIdFromGuid(cr, Bookmarks.MOBILE_FOLDER_GUID);
 
             // We'll add a fake "Desktop Bookmarks" folder to the root view if desktop
             // bookmarks exist, so that the user can still access non-mobile bookmarks.
             addDesktopFolder = desktopBookmarksExist(cr);
+            addScreenshotsFolder = true;
         } else {
             addDesktopFolder = false;
+            addScreenshotsFolder = false;
         }
 
         final Cursor c;
         if (folderId == Bookmarks.FAKE_DESKTOP_FOLDER_ID) {
             // Since the "Desktop Bookmarks" folder doesn't actually exist, we
             // just fake it by querying specifically certain known desktop folders.
             c = cr.query(mBookmarksUriWithProfile,
                          DEFAULT_BOOKMARK_COLUMNS,
                          Bookmarks.GUID + " = ? OR " +
                          Bookmarks.GUID + " = ? OR " +
                          Bookmarks.GUID + " = ?",
                          new String[] { Bookmarks.TOOLBAR_FOLDER_GUID,
                                         Bookmarks.MENU_FOLDER_GUID,
                                         Bookmarks.UNFILED_FOLDER_GUID },
                          null);
+        } else if (folderId == Bookmarks.FIXED_SCREENSHOT_FOLDER_ID) {
+            c = getUrlAnnotations().getScreenshots(cr);
         } else {
             // Right now, we only support showing folder and bookmark type of
             // entries. We should add support for other types though (bug 737024)
             c = cr.query(mBookmarksUriWithProfile,
                          DEFAULT_BOOKMARK_COLUMNS,
                          Bookmarks.PARENT + " = ? AND " +
                          "(" + Bookmarks.TYPE + " = ? OR " +
                             "(" + Bookmarks.TYPE + " = ? AND " + Bookmarks.URL + " IS NOT NULL))",
                          new String[] { String.valueOf(folderId),
                                         String.valueOf(Bookmarks.TYPE_FOLDER),
                                         String.valueOf(Bookmarks.TYPE_BOOKMARK) },
                          null);
         }
 
-        if (addDesktopFolder) {
-            // Wrap cursor to add fake desktop bookmarks and reading list folders
-            return new SpecialFoldersCursorWrapper(c, addDesktopFolder);
-        }
-
-        return c;
+        // Wrap cursor to add fake desktop bookmarks and reading list folders
+        return new SpecialFoldersCursorWrapper(c, addDesktopFolder, addScreenshotsFolder);
     }
 
     // Returns true if any desktop bookmarks exist, which will be true if the user
     // has set up sync at one point, or done a profile migration from XUL fennec.
     private boolean desktopBookmarksExist(ContentResolver cr) {
         if (mDesktopBookmarksExist != null) {
             return mDesktopBookmarksExist;
         }
@@ -1485,86 +1486,107 @@ public class LocalBrowserDB implements B
     }
 
     // This wrapper adds a fake "Desktop Bookmarks" folder entry to the
     // beginning of the cursor's data set.
     private static class SpecialFoldersCursorWrapper extends CursorWrapper {
         private int mIndexOffset;
 
         private int mDesktopBookmarksIndex = -1;
+        private int mScreenshotsFolderIndex = -1;
 
         private boolean mAtDesktopBookmarksPosition;
+        private boolean mAtScreenshotsFolderPosition;
 
-        public SpecialFoldersCursorWrapper(Cursor c, boolean showDesktopBookmarks) {
+        public SpecialFoldersCursorWrapper(final Cursor c, final boolean showDesktopBookmarks,
+                final boolean showScreenshotsFolder) {
             super(c);
 
             if (showDesktopBookmarks) {
                 mDesktopBookmarksIndex = mIndexOffset;
                 mIndexOffset++;
             }
+
+            if (showScreenshotsFolder) {
+                mScreenshotsFolderIndex = mIndexOffset;
+                mIndexOffset++;
+            }
         }
 
         @Override
         public int getCount() {
             return super.getCount() + mIndexOffset;
         }
 
         @Override
         public boolean moveToPosition(int position) {
             mAtDesktopBookmarksPosition = (mDesktopBookmarksIndex == position);
+            mAtScreenshotsFolderPosition = (mScreenshotsFolderIndex == position);
 
-            if (mAtDesktopBookmarksPosition) {
+            if (isAtSpecialFolderPosition()) {
                 return true;
             }
 
             return super.moveToPosition(position - mIndexOffset);
         }
 
         @Override
         public long getLong(int columnIndex) {
-            if (!mAtDesktopBookmarksPosition) {
+            if (!isAtSpecialFolderPosition()) {
                 return super.getLong(columnIndex);
             }
 
             if (columnIndex == getColumnIndex(Bookmarks.PARENT)) {
                 return Bookmarks.FIXED_ROOT_ID;
             }
 
             return -1;
         }
 
         @Override
         public int getInt(int columnIndex) {
-            if (!mAtDesktopBookmarksPosition) {
+            if (!isAtSpecialFolderPosition()) {
                 return super.getInt(columnIndex);
             }
 
             if (columnIndex == getColumnIndex(Bookmarks._ID) && mAtDesktopBookmarksPosition) {
                 return Bookmarks.FAKE_DESKTOP_FOLDER_ID;
             }
 
+            if (columnIndex == getColumnIndex(Bookmarks._ID) && mAtScreenshotsFolderPosition) {
+                return Bookmarks.FIXED_SCREENSHOT_FOLDER_ID;
+            }
+
             if (columnIndex == getColumnIndex(Bookmarks.TYPE)) {
                 return Bookmarks.TYPE_FOLDER;
             }
 
             return -1;
         }
 
         @Override
         public String getString(int columnIndex) {
-            if (!mAtDesktopBookmarksPosition) {
+            if (!isAtSpecialFolderPosition()) {
                 return super.getString(columnIndex);
             }
 
             if (columnIndex == getColumnIndex(Bookmarks.GUID) && mAtDesktopBookmarksPosition) {
                 return Bookmarks.FAKE_DESKTOP_FOLDER_GUID;
             }
 
+            if (columnIndex == getColumnIndex(Bookmarks.GUID) && mAtScreenshotsFolderPosition) {
+                return Bookmarks.SCREENSHOT_FOLDER_GUID;
+            }
+
             return "";
         }
+
+        private boolean isAtSpecialFolderPosition() {
+            return mAtDesktopBookmarksPosition || mAtScreenshotsFolderPosition;
+        }
     }
 
     @Override
     public void pinSite(ContentResolver cr, String url, String title, int position) {
         ContentValues values = new ContentValues();
         final long now = System.currentTimeMillis();
         values.put(Bookmarks.TITLE, title);
         values.put(Bookmarks.URL, url);
--- a/mobile/android/base/java/org/mozilla/gecko/home/BookmarksListAdapter.java
+++ b/mobile/android/base/java/org/mozilla/gecko/home/BookmarksListAdapter.java
@@ -5,34 +5,38 @@
 
 package org.mozilla.gecko.home;
 
 import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
 
 import org.mozilla.gecko.R;
+import org.mozilla.gecko.db.BrowserContract;
 import org.mozilla.gecko.db.BrowserContract.Bookmarks;
 
 import android.content.Context;
 import android.content.res.Resources;
 import android.database.Cursor;
 import android.os.Parcel;
 import android.os.Parcelable;
 import android.view.View;
+import android.widget.TextView;
 
 /**
  * Adapter to back the BookmarksListView with a list of bookmarks.
  */
 class BookmarksListAdapter extends MultiTypeCursorAdapter {
-    private static final int VIEW_TYPE_ITEM = 0;
+    private static final int VIEW_TYPE_BOOKMARK_ITEM = 0;
     private static final int VIEW_TYPE_FOLDER = 1;
+    private static final int VIEW_TYPE_SCREENSHOT = 2;
 
-    private static final int[] VIEW_TYPES = new int[] { VIEW_TYPE_ITEM, VIEW_TYPE_FOLDER };
-    private static final int[] LAYOUT_TYPES = new int[] { R.layout.bookmark_item_row, R.layout.bookmark_folder_row };
+    private static final int[] VIEW_TYPES = new int[] { VIEW_TYPE_BOOKMARK_ITEM, VIEW_TYPE_FOLDER, VIEW_TYPE_SCREENSHOT };
+    private static final int[] LAYOUT_TYPES =
+            new int[] { R.layout.bookmark_item_row, R.layout.bookmark_folder_row, R.layout.bookmark_screenshot_row };
 
     public enum RefreshType implements Parcelable {
         PARENT,
         CHILD;
 
         @Override
         public int describeContents() {
             return 0;
@@ -99,23 +103,35 @@ class BookmarksListAdapter extends Multi
 
     // A listener that knows how to refresh the list for a given folder id.
     // This is usually implemented by the enclosing fragment/activity.
     public static interface OnRefreshFolderListener {
         // The folder id to refresh the list with.
         public void onRefreshFolder(FolderInfo folderInfo, RefreshType refreshType);
     }
 
+    /**
+     * The type of data a bookmarks folder can display. This can be used to
+     * distinguish bookmark folders from "smart folders" that contain non-bookmark
+     * entries but still appear in the Bookmarks panel.
+     */
+    public enum FolderType {
+        BOOKMARKS,
+        SCREENSHOTS,
+    }
+
     // mParentStack holds folder info instances (id + title) that allow
     // us to navigate back up the folder hierarchy.
     private final LinkedList<FolderInfo> mParentStack;
 
     // Refresh folder listener.
     private OnRefreshFolderListener mListener;
 
+    private FolderType openFolderType = FolderType.BOOKMARKS;
+
     public BookmarksListAdapter(Context context, Cursor cursor, List<FolderInfo> parentStack) {
         // Initializing with a null cursor.
         super(context, cursor, VIEW_TYPES, LAYOUT_TYPES);
 
         if (parentStack == null) {
             mParentStack = new LinkedList<FolderInfo>();
         } else {
             mParentStack = new LinkedList<FolderInfo>(parentStack);
@@ -172,16 +188,17 @@ class BookmarksListAdapter extends Multi
     }
 
     private boolean isCurrentFolder(FolderInfo folderInfo) {
         return (mParentStack.size() > 0 &&
                 mParentStack.peek().id == folderInfo.id);
     }
 
     public void swapCursor(Cursor c, FolderInfo folderInfo, RefreshType refreshType) {
+        updateOpenFolderType(folderInfo);
         switch(refreshType) {
             case PARENT:
                 if (!isCurrentFolder(folderInfo)) {
                     mParentStack.removeFirst();
                 }
                 break;
 
             case CHILD:
@@ -192,35 +209,47 @@ class BookmarksListAdapter extends Multi
 
             default:
                 // Do nothing;
         }
 
         swapCursor(c);
     }
 
+    private void updateOpenFolderType(final FolderInfo folderInfo) {
+        if (folderInfo.id == Bookmarks.FIXED_SCREENSHOT_FOLDER_ID) {
+            openFolderType = FolderType.SCREENSHOTS;
+        } else {
+            openFolderType = FolderType.BOOKMARKS;
+        }
+    }
+
     @Override
     public int getItemViewType(int position) {
         // The position also reflects the opened child folder row.
         if (isShowingChildFolder()) {
             if (position == 0) {
                 return VIEW_TYPE_FOLDER;
             }
 
             // Accounting for the folder view.
             position--;
         }
 
+        if (openFolderType == FolderType.SCREENSHOTS) {
+            return VIEW_TYPE_SCREENSHOT;
+        }
+
         final Cursor c = getCursor(position);
         if (c.getInt(c.getColumnIndexOrThrow(Bookmarks.TYPE)) == Bookmarks.TYPE_FOLDER) {
             return VIEW_TYPE_FOLDER;
         }
 
         // Default to returning normal item type.
-        return VIEW_TYPE_ITEM;
+        return VIEW_TYPE_BOOKMARK_ITEM;
     }
 
     /**
      * Get the title of the folder given a cursor moved to the position.
      *
      * @param context The context of the view.
      * @param cursor A cursor moved to the required position.
      * @return The title of the folder at the position.
@@ -239,16 +268,18 @@ class BookmarksListAdapter extends Multi
         if (guid.equals(Bookmarks.FAKE_DESKTOP_FOLDER_GUID)) {
             return res.getString(R.string.bookmarks_folder_desktop);
         } else if (guid.equals(Bookmarks.MENU_FOLDER_GUID)) {
             return res.getString(R.string.bookmarks_folder_menu);
         } else if (guid.equals(Bookmarks.TOOLBAR_FOLDER_GUID)) {
             return res.getString(R.string.bookmarks_folder_toolbar);
         } else if (guid.equals(Bookmarks.UNFILED_FOLDER_GUID)) {
             return res.getString(R.string.bookmarks_folder_unfiled);
+        } else if (guid.equals(Bookmarks.SCREENSHOT_FOLDER_GUID)) {
+            return res.getString(R.string.screenshot_folder_label_in_bookmarks);
         }
 
         // If for some reason we have a folder with a special GUID, but it's not one of
         // the special folders we expect in the UI, just return the title from the DB.
         return c.getString(c.getColumnIndexOrThrow(Bookmarks.TITLE));
     }
 
     /**
@@ -279,17 +310,21 @@ class BookmarksListAdapter extends Multi
                 // Accounting for the folder view.
                 position--;
                 cursor = getCursor(position);
             }
         } else {
             cursor = getCursor(position);
         }
 
-        if (viewType == VIEW_TYPE_ITEM) {
+        if (viewType == VIEW_TYPE_SCREENSHOT) {
+            // TODO: Update view in a more robust way.
+            ((TextView) view.findViewById(R.id.title)).setText(cursor.getString(cursor.getColumnIndexOrThrow(BrowserContract.UrlAnnotations.URL)));
+            ((TextView) view.findViewById(R.id.date)).setText(Long.toString(cursor.getLong(cursor.getColumnIndexOrThrow(BrowserContract.UrlAnnotations.DATE_CREATED))));
+        } else if (viewType == VIEW_TYPE_BOOKMARK_ITEM) {
             final TwoLinePageRow row = (TwoLinePageRow) view;
             row.updateFromCursor(cursor);
         } else {
             final BookmarkFolderView row = (BookmarkFolderView) view;
             if (cursor == null) {
                 final Resources res = context.getResources();
                 row.setText(res.getString(R.string.home_move_up_to_filter, mParentStack.get(1).title));
                 row.open();
--- a/mobile/android/base/locales/en-US/android_strings.dtd
+++ b/mobile/android/base/locales/en-US/android_strings.dtd
@@ -64,16 +64,20 @@
 <!-- Localization note (bookmark_already_added) : This string is
      used as a label in a toast. It is the verb "to bookmark", not
      the noun "a bookmark". -->
 <!ENTITY bookmark_already_added "Already bookmarked">
 <!ENTITY bookmark_removed "Bookmark removed">
 <!ENTITY bookmark_updated "Bookmark updated">
 <!ENTITY bookmark_options "Options">
 <!ENTITY screenshot_added_to_bookmarks "Screenshot added to bookmarks">
+<!-- Localization note (screenshot_folder_label_in_bookmarks): We save links to screenshots
+     the user takes. The folder we store these links in is located in the bookmarks list
+     and is labeled by this String. -->
+<!ENTITY screenshot_folder_label_in_bookmarks "Screenshots">
 
 <!ENTITY history_today_section "Today">
 <!ENTITY history_yesterday_section "Yesterday">
 <!ENTITY history_week_section3 "Last 7 days">
 <!ENTITY history_this_month_section "This month">
 <!ENTITY history_older_section3 "Older than 6 months">
 
 <!ENTITY go "Go">
new file mode 100644
--- /dev/null
+++ b/mobile/android/base/resources/layout/bookmark_screenshot_row.xml
@@ -0,0 +1,23 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!-- This Source Code Form is subject to the terms of the Mozilla Public
+   - License, v. 2.0. If a copy of the MPL was not distributed with this
+   - file, You can obtain one at http://mozilla.org/MPL/2.0/. -->
+
+<!-- TODO: create proper layout -->
+<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
+              android:orientation="vertical"
+              android:layout_width="match_parent"
+              android:layout_height="wrap_content"
+              android:padding="4dp">
+
+    <TextView android:id="@+id/title"
+              android:layout_width="wrap_content"
+              android:layout_height="wrap_content"
+              />
+
+    <TextView android:id="@+id/date"
+              android:layout_width="wrap_content"
+              android:layout_height="wrap_content"
+              />
+
+</LinearLayout>
\ No newline at end of file
--- a/mobile/android/base/strings.xml.in
+++ b/mobile/android/base/strings.xml.in
@@ -98,16 +98,17 @@
   <string name="bookmark">&bookmark;</string>
   <string name="bookmark_remove">&bookmark_remove;</string>
   <string name="bookmark_added">&bookmark_added;</string>
   <string name="bookmark_already_added">&bookmark_already_added;</string>
   <string name="bookmark_removed">&bookmark_removed;</string>
   <string name="bookmark_updated">&bookmark_updated;</string>
   <string name="bookmark_options">&bookmark_options;</string>
   <string name="screenshot_added_to_bookmarks">&screenshot_added_to_bookmarks;</string>
+  <string name="screenshot_folder_label_in_bookmarks">&screenshot_folder_label_in_bookmarks;</string>
 
   <string name="history_today_section">&history_today_section;</string>
   <string name="history_yesterday_section">&history_yesterday_section;</string>
   <string name="history_week_section">&history_week_section3;</string>
   <string name="history_this_month_section">&history_this_month_section;</string>
   <string name="history_older_section">&history_older_section3;</string>
 
   <string name="share">&share;</string>