Closed Bug 701380 Opened 13 years ago Closed 13 years ago

Implement a native about:home (start page)

Categories

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

ARM
Android
defect

Tracking

(firefox11 fixed, fennec11+)

VERIFIED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: madhava, Assigned: blassey)

References

Details

(Keywords: uiwanted)

Attachments

(6 files, 6 obsolete files)

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.
Assignee: nobody → madhava
Priority: -- → P1
Attached file WIP patch (obsolete) —
Attached patch WIP patch (obsolete) — Splinter Review
Attachment #574240 - Attachment is obsolete: true
Attached image screen shot (obsolete) —
Attached patch WIP patch (obsolete) — Splinter Review
Attachment #574799 - Attachment is obsolete: true
Attached patch WIP patch (obsolete) — Splinter Review
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.
Attachment #574802 - Attachment is obsolete: true
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.
(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
OS: Mac OS X → Android
Hardware: x86 → ARM
Summary: Native Fennec should have a start page → Implement a native about:home (start page)
Attached patch patch (obsolete) — Splinter Review
Assignee: madhava → blassey.bugs
Attachment #575730 - Attachment is obsolete: true
Attachment #577042 - Flags: review?(mark.finkle)
Attached image screen shot
Attachment #574800 - Attachment is obsolete: true
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.
(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 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
Attachment #577042 - Flags: review?(mark.finkle) → review-
Attached patch patchSplinter Review
Attachment #577042 - Attachment is obsolete: true
Attachment #577474 - Flags: review?(mark.finkle)
> Now I kinda understand the mShow usage. You are hiding the view. Adding more
> comments to the above code...

any particular suggestions?
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
Attachment #577474 - Flags: review?(mark.finkle) → review+
pushed https://hg.mozilla.org/projects/birch/rev/beea574981d2
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
I had to back this out.
https://hg.mozilla.org/projects/birch/rev/86bc0eabb94d
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Blocks: 704588
Depends on: 706460
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!
brad relanded: https://hg.mozilla.org/projects/birch/rev/e08c46f33a16
Status: REOPENED → RESOLVED
Closed: 13 years ago13 years ago
Resolution: --- → FIXED
Attached image 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.
Depends on: 706644
Thumbnail shadow and list arrow images are included here.
Depends on: 706663
Depends on: 706703
Samsung Nexus S (Android 2.3.6)
20111201040252
http://hg.mozilla.org/projects/birch/rev/d71c91775f9b
Status: RESOLVED → VERIFIED
Depends on: 707718
tracking-fennec: --- → 11+
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: