701380 - Implement a native about:home (start page)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Description

[Madhava Enros [:madhava]]

We should have a start page, built mostly native so it can be visible and useful while Gecko is starting. We will often (sometimes?) not have an image of a previously open site to show, or may even not want to use one, if it is old enough. A start page also gives us a place to communicate with the user, and is especially useful on first run.

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: WIP patch

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: WIP patch

Comment 3

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: screen shot

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: WIP patch

Comment 5

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: WIP patch

putting the start page in its own activity means we can't start it until well into the life cycle of our main activity. This changes course a bit and makes it a view that we show instead of the layer controller's view. Same look and feel (screenshot should be identical) but, its displayed much sooner.

Comment 6

[Ian Barlow (:ibarlow)]

Attachment: Wireframe of native fennec start page

Our start page should contain:
* Top Sites (which is the top results from your Awesomescreen)
** "more" link that opens Awesomescreen
* Remote Tabs
* Tabs from last sessions
* Add-ons

The attached mockup shows the start page based on different times that a user lands there.

First time
* Sync setup link
* Add-ons

After browsing for a while (but no sync setup)
* Top Sites
* Sync setup link
* Tabs from last session
* Add-ons

After browsing for a while AND setting up sync
* Top Sites
* Remote Tabs
* Tabs from last session
* Add-ons

------------

There will be more polished mockups coming soon, but this wireframe can be used to start building out the page's functionality for now.

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Ian Barlow (:ibarlow) from comment #6)
> Created attachment 576217 [details]
> Wireframe of native fennec start page
> 
> Our start page should contain:
> * Top Sites (which is the top results from your Awesomescreen)
> ** "more" link that opens Awesomescreen
currently the top sites view is scrollable. Do you want to make it not scrollable and replace it with that more link? or keep it scrollable and loose the more link?
> * Remote Tabs
not gonna have this until we have sync, I'd suggest a follow up bug
> * Tabs from last sessions
not going to have this until we have session restore, I'd suggest a follow up bug
> * Add-ons
> 
> The attached mockup shows the start page based on different times that a
> user lands there.
> 
> First time
> * Sync setup link
again, no sync
> * Add-ons
Also, since we're using system history and bookmarks we'll still have top sites for the first run

Comment 8

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 9

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: screen shot

Comment 10

[Steffen Wilberg]

So if the user enters about:home in the location bar, or clicks the about:home link on the about:about page, you load aboutHome.xhtml, which then messages Browser:ShowAboutHome.

Can you send that message from AboutRedirector.js, or register a custom component from MobileComponents.manifest, and get rid of aboutHome.xhtml?

Anyway, if you remove (or hardcode) the page title from aboutHome.xhtml, you can remove aboutHome.dtd and aboutHome.css.

Comment 11

[Mark Finkle (:mfinkle) (use needinfo?)]

(In reply to Steffen Wilberg from comment #10)
> So if the user enters about:home in the location bar, or clicks the
> about:home link on the about:about page, you load aboutHome.xhtml, which
> then messages Browser:ShowAboutHome.
> 
> Can you send that message from AboutRedirector.js, or register a custom
> component from MobileComponents.manifest, and get rid of aboutHome.xhtml?

I suggested the same thing! But Brad points out that having a dummy page helps maintain browser session history (back/forward).

> Anyway, if you remove (or hardcode) the page title from aboutHome.xhtml, you
> can remove aboutHome.dtd and aboutHome.css.

Yes, good catch.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 577042 [details] [diff] [review]
patch

>diff --git a/mobile/android/app/recommended-addons.json b/mobile/android/app/recommended-addons.json

This file is really too big. I thought our latest releases produced a much smaller recommended-addons.json file

>diff --git a/mobile/android/base/AboutHomeContent.java b/mobile/android/base/AboutHomeContent.java

This file uses 2 space indents, but we use 4 space indents for Java files

>+public class AboutHomeContent extends LinearLayout {

>+  private static final String LOG_NAME = "AboutHome";

nit: We have been using LOGTAG and "GeckoAboutHome"

>+  public void onActivityContentChanged(Activity activity) {

>+    // we want to do this: mGrid.setNumColumns(GridView.AUTO_FIT); but it doesn't work
>+    Display display = ((WindowManager) activity.getSystemService(Context.WINDOW_SERVICE)).getDefaultDisplay();
>+    int width = display.getWidth();
>+    mGrid.setNumColumns((int) Math.floor(width / 122));

Can we use a constant for the 122?

>+    if (mFinishedStart) {
>+      mGrid.setAdapter(mGridAdapter);
>+    }
>+
>+    mFinishedStart = true;

the mFinishedStart confuses me. Can we kill it?

>+  void init(final Activity activity) {

>+    GeckoAppShell.getHandler().post(new Runnable() {
>+      public void run() {
>+        mCursor = activity.managedQuery(Browser.BOOKMARKS_URI,
>+                                        null, kAbouthomeWhereClause, null, null);
>+        activity.startManagingCursor(mCursor);
>+
>+	onActivityContentChanged(activity);
>+	mAddonAdapter = new ArrayAdapter<String>(activity, R.layout.abouthome_addon_list_item);

remove the TABs

>+	    mAddonList.setAdapter(mAddonAdapter);
>+	  }});
>+	readRecommendedAddons(activity);
>+        
>+      }});

Remove TABs
Remove the blank line and can you use the }}) -> }\n}); to maintain indents

>+  void readRecommendedAddons(final Activity activity) {
>+    GeckoAppShell.getHandler().post(new Runnable() {
>+      public void run() {

Can we try to load the file from the profile first? If it is missing, we should load the packaged file.

>+  private boolean updateUrl(View view, Cursor cursor, int urlIndex) {
>+    String title = cursor.getString(urlIndex);
>+    TextView urlView = (TextView)view;
>+    if (title != null) {
>+      if (title.startsWith("http://"))
>+        title = title.substring(7);

Can we use "://"" since this will skip https:// and ftp://


>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>+    public void showAboutHome(boolean show) {
>+        Runnable r = new ShowAboutHomeRunnable(show);

Why do we want to support show == False ? (mfinkle: We do it to hide the view)

>+    public class  ShowAboutHomeRunnable implements Runnable {

>+        public void run() {
>+            if (mAboutHomeContent == null) {
>+                mAboutHomeContent = new AboutHomeContent(GeckoApp.mAppContext, null);
>+                mAboutHomeContent.init(GeckoApp.mAppContext);
>+                mAboutHomeContent.setUriLoadCallback(new AboutHomeContent.UriLoadCallback() {
>+                    public void callback(String url) {
>+                        mBrowserToolbar.setProgressVisibility(true);
>+                        loadUrl(url, AwesomeBar.Type.EDIT);
>+                    }});

}\n});

>+            if (mLayerController != null && mLayerController.getView() != null)
>+                mLayerController.getView().setVisibility(mShow ? View.GONE : View.VISIBLE);
>+            mAboutHomeContent.setVisibility(mShow ? View.VISIBLE : View.GONE);

Again, curious about the need for mShow (mfinkle: we do it to hide the view)

>+    public void onContentChanged() {
>+        super.onContentChanged();
>+        if (mAboutHomeContent == null)
>+            return;
>+        mAboutHomeContent = (AboutHomeContent) findViewById(R.id.abouthome_content);
>+        mAboutHomeContent.onActivityContentChanged(this);

When does this get called? Docs say when the screen changes, but does that mean rotation?

>     public void loadUrl(String url, AwesomeBar.Type type) {
>+        showAboutHome(false);

nit: add a blank line please

Now I kinda understand the mShow usage. You are hiding the view. Adding more comments to the above code...

How are we hiding the view when selecting tabs?

>diff --git a/mobile/android/base/GeckoThread.java b/mobile/android/base/GeckoThread.java

>     public void run() {
>         final GeckoApp app = GeckoApp.mAppContext;
>         Intent intent = mIntent;
>+        String uri = intent.getDataString();
>+        String title = uri;
>+        if (!app.mUserDefinedProfile &&
>+            (uri == null || uri.length() == 0)) {

plenty of room to not need to wrap...

>+            SharedPreferences prefs = app.getSharedPreferences("GeckoApp", app.MODE_PRIVATE);
>+                uri = prefs.getString("last-uri", "");
>+                title = prefs.getString("last-title", uri);

Fix indent

>+        }
>+        if (uri == null || uri.equals("") || uri.equals("about:home")) {
>+            Log.i("GeckoThread", "showing about:home");
>+            app.showAboutHome(true);
>+        } else {
>+            Log.i("GeckoThread", "not showing about:home");
>+        }

We already have LOGTAG. Could you use it here

>         // and then fire us up
>+
>+        
>+        final String activityTitle = title;

Remove the 2 blank lines

>+        app.mMainHandler.post(new Runnable() {
>+            public void run() {
>+                app.mBrowserToolbar.setTitle(activityTitle);
>+            }});

}\n});

>diff --git a/mobile/android/base/resources/drawable/rounded_grey_border.xml b/mobile/android/base/resources/drawable/rounded_grey_border.xml

>+<shape xmlns:android="http://schemas.android.com/apk/res/android">
>+  <solid android:color="#FFFFFFFF" />
>+  <stroke  android:width="2dip" android:color="#ffD0D0D0"/>
>+    <padding android:left="7dp" android:top="7dp"
>+            android:right="7dp" android:bottom="7dp" />
>+    <corners android:radius="4dp" />
>+</shape> 

nits on <stroke  android  and <padding attr indent

>diff --git a/mobile/android/base/resources/drawable/rounded_grey_box.xml b/mobile/android/base/resources/drawable/rounded_grey_box.xml

Same nits

>diff --git a/mobile/android/base/resources/layout/abouthome_content.xml b/mobile/android/base/resources/layout/abouthome_content.xml

nit: put firstandroid attr on the same line as the tag name
nit: put /> on same line as last attr

>diff --git a/mobile/android/base/resources/layout/abouthome_grid_box.xml b/mobile/android/base/resources/layout/abouthome_grid_box.xml

nit: fix off-by-one indents for android attr
nit: put /> on same line as last attr

>diff --git a/mobile/android/chrome/content/aboutHome.xhtml b/mobile/android/chrome/content/aboutHome.xhtml

>-  <link rel="stylesheet" href="chrome://browser/skin/aboutHome.css" type="text/css"/>

let's remove this file too (aboutHome.css)

>+<body dir="&locale.dir;" onload="init();" onunload="uninit();">

remove dir and onunload

>+  <script type="application/javascript;version=1.8"><![CDATA[

>+    let Cu = Components.utils;
>+    let Cr = Components.results;

remove these if not used

>+    function init() {
>+      var bridge = Cc["@mozilla.org/android/bridge;1"].getService(Ci.nsIAndroidBridge);
>+      var obj = {gecko: {

var -> let

{ gecko:

>+            type: "Browser:ShowAboutHome"
>+          }};

}\n};

>+      var str = JSON.stringify(obj);
>+      dump(str);

remove

>+      bridge.handleGeckoMessage(str);

just pass directly JSON.stringify(obj)

>+      Cu.reportError("sent Browser:ShowAboutHome");

remove

>+    function uninit() {}

Just remove it

r- for the select other tab issue and the cleanup. Using the profile recommended-addons.json could be a followup bug

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: patch

Comment 14

[Brad Lassey [:blassey] (use needinfo?)]

> Now I kinda understand the mShow usage. You are hiding the view. Adding more
> comments to the above code...

any particular suggestions?

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 577474 [details] [diff] [review]
patch


>diff --git a/mobile/android/base/AboutHomeContent.java b/mobile/android/base/AboutHomeContent.java

>+    void init(final Activity activity) {

>+                        mGrid.setAdapter(gridAdapter);
>+                        gridAdapter.setViewBinder(new AwesomeCursorViewBinder());
>+                        mAddonList.setAdapter(mAddonAdapter);
>+                    }});

}\n});

>+                readRecommendedAddons(activity);
>+
>+            }

Remove the blank line

>+    void readRecommendedAddons(final Activity activity) {

Just file a bug for adding code to read from the profile, if it exists

>+    public boolean setViewValue(View view, Cursor cursor, int columnIndex) {

>+        int urlIndex = cursor.getColumnIndexOrThrow(Browser.BookmarkColumns.URL);
>+
>+        if (columnIndex == urlIndex) {

Remove the blank line

>+        int thumbIndex = cursor.getColumnIndexOrThrow("thumbnail");
>+
>+        if (columnIndex == thumbIndex) {

Remove the blank line

>diff --git a/mobile/android/base/GeckoApp.java b/mobile/android/base/GeckoApp.java

>+    public void showAboutHome(boolean show) {
>+        Runnable r = new ShowAboutHomeRunnable(show);
>+        mMainHandler.postAtFrontOfQueue(r);
>+    }

I know what would make this easier to understand: make separate methods for show and hide

showAboutHome()
hideAboutHome()


>+    public class  ShowAboutHomeRunnable implements Runnable {

Rename to AboutHomeRunnable, since it handles showing and hiding

>+        GeckoAppShell.registerGeckoEventListener("Browser:ShowAboutHome", GeckoApp.mAppContext);

Would you hate me for suggesting "AboutHome:Show"  ?

>diff --git a/mobile/android/chrome/content/aboutHome.xhtml b/mobile/android/chrome/content/aboutHome.xhtml
> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
>-  "http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd" [
>-<!ENTITY % brandDTD SYSTEM "chrome://branding/locale/brand.dtd" >
>-%brandDTD;
>-<!ENTITY % globalDTD SYSTEM "chrome://global/locale/global.dtd" >
>-%globalDTD;
>-<!ENTITY % preferenceDTD SYSTEM "chrome://browser/locale/preferences.dtd" >
>-%preferenceDTD;
>-<!ENTITY % aboutDTD SYSTEM "chrome://browser/locale/aboutHome.dtd" >
>-%aboutDTD;
>-]>

Keep the aboutHome DTD so we can pull the homepage.title

>-<head>
>-  <title>&homepage.default;</title>
>-  <meta name="viewport" content="width=480; initial-scale=.6667; user-scalable=0" />
>-  <link rel="icon" type="image/png" href="chrome://branding/content/favicon32.png" />
>-  <link rel="stylesheet" href="chrome://browser/skin/aboutHome.css" type="text/css"/>
>-</head>

Keep the <title> and the <link rel="icon"> to keep the tabs list and the URL bar updated with a title and favicon

>+    function init() {
>+      let bridge = Cc["@mozilla.org/android/bridge;1"].getService(Ci.nsIAndroidBridge);
>+      let obj = { gecko: {
>+            type: "Browser:ShowAboutHome"
>+          }
>+      };

Only need 2 space indent here

NOTE: You can remove all the strings from aboutHome.dtd except homepage.title

r+ with these fixes and filing the bug for reading the profile recommended-addons.json

Comment 16

[Brad Lassey [:blassey] (use needinfo?)]

pushed https://hg.mozilla.org/projects/birch/rev/beea574981d2

Comment 17

[Mark Finkle (:mfinkle) (use needinfo?)]

I had to back this out.
https://hg.mozilla.org/projects/birch/rev/86bc0eabb94d

Comment 18

[Ian Barlow (:ibarlow)]

Attachment: Start Page mockups with more polish

Hey guys, here is a more finished mockup of the start page. 

The screens on the left show what it should look like with our current functionality: Top Sites and Add-ons

The screens on the right show what it should look like once we get Sync and Session restore hooked up. 

NOTE: The whole page should scroll as a single unit, rather than its current implementation where top sites scrolls as its own window.


---

Who is working on cleaning up the front end UI for this? Ping me in #mobile and we'll get going on specs and visual assets. Shouldn't take us long to get this looking good!

Comment 19

[Mark Finkle (:mfinkle) (use needinfo?)]

brad relanded: https://hg.mozilla.org/projects/birch/rev/e08c46f33a16

Comment 20

[Ian Barlow (:ibarlow)]

Attachment: UI Specs

Specs for the first version of the start page that includes Top Sites and Add-ons. Assets to follow in the next comment. 

NOTE: The logo header area is being reworked, so just focus on the actual content, and we can plug in an updated header when I have it ready.

Comment 21

[Ian Barlow (:ibarlow)]

Attachment: Graphic assets for start page

Thumbnail shadow and list arrow images are included here.

Comment 22

[Aaron Train [:aaronmt]]

Samsung Nexus S (Android 2.3.6)
20111201040252
http://hg.mozilla.org/projects/birch/rev/d71c91775f9b