Last Comment Bug 676293 - Add ability to make "home screen shortcut" for Bookmarks
: Add ability to make "home screen shortcut" for Bookmarks
Status: VERIFIED FIXED
: uiwanted
Product: Fennec Graveyard
Classification: Graveyard
Component: Bookmarks (show other bugs)
: Trunk
: ARM Android
: -- normal (vote)
: Firefox 8
Assigned To: Mark Finkle (:mfinkle) (use needinfo?)
:
Mentors:
Depends on: 677975
Blocks: 677630
  Show dependency treegraph
 
Reported: 2011-08-03 10:13 PDT by Chris Jones [:cjones] inactive; ni?/f?/r? if you need me
Modified: 2011-08-15 07:40 PDT (History)
7 users (show)
aaron.train: in‑testsuite?
aaron.train: in‑litmus+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 (34.25 KB, patch)
2011-08-05 13:22 PDT, Mark Finkle (:mfinkle) (use needinfo?)
blassey.bugs: review-
fabrice: review+
Details | Diff | Review
patch v2 (38.75 KB, patch)
2011-08-07 21:40 PDT, Mark Finkle (:mfinkle) (use needinfo?)
blassey.bugs: review+
Details | Diff | Review

Description Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 10:13:08 PDT
Use case: I exclusively use zimbra through FF for moz email on android.  I used FF's site menu to make zimbra an "app" (although I don't think it wants or needs any of the permissions that I could have granted it).  I want to take "app-ification" one step further and put a zimbra icon on my home screen, that links to zimbra.  I don't see a way to do that in FF.

Here's how I'd like the link to work
 - initially acts like a traditional URL bookmark.  Tapping it opens the link in android-browser or FF, whichever is default.  (I guess some sort of URL intent?)  Does android support that natively?

 - if the link goes to FF, then if FF is not running, it starts and loads the link.

 - if FF is running, and the link isn't already open, then the link opens in a new tab.

 - if the link is open but isn't designated as an "app" (through the site menu), then it opens in a new tab.  This probably isn't a great heuristic but should be fine for now.

 - if it *is* designated as an "app", then it just focuses the existing tab.

The use case for all this is that I want to click my zimbra icon and see zimbra load through the shortest path based on whatever the current FF/zimbra loaded-ness state is, but I don't care what that path is.  I never want to have clicking the home screen icon result in a second copy of zimbra opening.

Does this make sense?  Bug 669510 seems relevant, but I can't find UI for the code that mfinkle refers to.
Comment 1 Wesley Johnston (:wesj) 2011-08-03 10:26:56 PDT
This was implemented in bug 667530 (there's screenshots of the flow there).
Comment 2 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 10:33:24 PDT
Thanks.  That process was not at all obvious to me.

Two things I'd like to have
 (1) Add shortcut directly from FF, when I app-ify a page.  Is that possible?  If so, maybe it could be another checkbox with the permissions I can enable?

 (2) Not open a new tab if the "app" is already open; just focus the existing tab.  (Android won't open another instance of FF if it's already open.)
Comment 3 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 11:45:03 PDT
(In reply to comment #2)
> Thanks.  That process was not at all obvious to me.
> 
> Two things I'd like to have
>  (1) Add shortcut directly from FF, when I app-ify a page.  Is that
> possible?  If so, maybe it could be another checkbox with the permissions I
> can enable?

We will add a action from bookmarks to "add shortcut to home screen", but not for webapps, which are installed via a different mechanism. Webapps can also be added to the home screen too, but not from within Fennec. Use the Android long-tap-on-home-screen mechanism for that.

>  (2) Not open a new tab if the "app" is already open; just focus the
> existing tab.  (Android won't open another instance of FF if it's already
> open.)

We do this for webapps, but are not planning to do it with bookmarks.
Comment 4 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 11:46:06 PDT
Madhava just asked for this feature for bookmarks today. Morphing the bug to be specific to bookmarks. Webapps are slightly different in intent and impl.
Comment 5 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 11:52:56 PDT
(In reply to comment #3)
> (In reply to comment #2)
> > Thanks.  That process was not at all obvious to me.
> > 
> > Two things I'd like to have
> >  (1) Add shortcut directly from FF, when I app-ify a page.  Is that
> > possible?  If so, maybe it could be another checkbox with the permissions I
> > can enable?
> 
> We will add a action from bookmarks to "add shortcut to home screen", but
> not for webapps, which are installed via a different mechanism. Webapps can
> also be added to the home screen too, but not from within Fennec. Use the
> Android long-tap-on-home-screen mechanism for that.

Why make this harder/less discoverable for "web apps"?  That seems counterintuitive to me.  What would be coolest of all is if we could stick an app icon into the "All apps" android app-manager view, then I could drag the launcher from there to my home screen as I would for installed "native apps", in addition to browsing that hybrid "native app" / "web app" list.  Not sure if that's possible.

Is the theory that people usually add "normal" app shortcuts to the home screen through the long-tap-to-shortcut-menu?  If so, then fwiw, a datapoint from me is that I've never done that and would never think to.  Fwiw.  Do we have any data there?
Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 12:26:56 PDT
(In reply to comment #5)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Thanks.  That process was not at all obvious to me.
> > > 
> > > Two things I'd like to have
> > >  (1) Add shortcut directly from FF, when I app-ify a page.  Is that
> > > possible?  If so, maybe it could be another checkbox with the permissions I
> > > can enable?
> > 
> > We will add a action from bookmarks to "add shortcut to home screen", but
> > not for webapps, which are installed via a different mechanism. Webapps can
> > also be added to the home screen too, but not from within Fennec. Use the
> > Android long-tap-on-home-screen mechanism for that.
> 
> Why make this harder/less discoverable for "web apps"?  That seems
> counterintuitive to me.  What would be coolest of all is if we could stick
> an app icon into the "All apps" android app-manager view, then I could drag
> the launcher from there to my home screen as I would for installed "native
> apps", in addition to browsing that hybrid "native app" / "web app" list. 
> Not sure if that's possible.

Adding to "All app" requires a native APK AFAIK, which we may get to at some point. Adding native apps to the home screen on Android means using the long-tap-on-home-screen launcher and picking "Shortcuts" > "Applications". We are doing the same thing for "Web Apps". This allows you to add a shortcut to any home screen without needing to launch the browser. We can debate adding some "Add shortcut to Home" for webapps _inside_ Fennec as well - once we have a UI to view webapps in Fennec. It's coming.

I am pretty adamant about using Android OS standard launchers for adding webapps and bookmarks. Actions from within Fennec are secondary. We have a goal to add an action for bookmarks. Webapps are not bookmarks.

> Is the theory that people usually add "normal" app shortcuts to the home
> screen through the long-tap-to-shortcut-menu?  If so, then fwiw, a datapoint
> from me is that I've never done that and would never think to.  Fwiw.  Do we
> have any data there?

How do you add native apps to the home screen?
Comment 7 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 17:25:09 PDT
(In reply to comment #6)
> Adding to "All app" requires a native APK AFAIK, which we may get to at some
> point.

Hm that sounds kinda gross.  Guess it could be cast as app-cache++.

> I am pretty adamant about using Android OS standard launchers for adding
> webapps and bookmarks. Actions from within Fennec are secondary. We have a
> goal to add an action for bookmarks. Webapps are not bookmarks.

Fennec is the installer of the web app.  If the install doesn't do the same thing as the native-app installer (stick the app link in the app menu), which it doesn't, then I don't see why having an in-fennec helper for creating the shortcut is bad.

FWIW, I think we're both on the same page that we'd like the boundary between the native android apps and web apps to be blurred.  Not trying to obstruct that effort, just noting my personal experience with the current impl.

> > Is the theory that people usually add "normal" app shortcuts to the home
> > screen through the long-tap-to-shortcut-menu?  If so, then fwiw, a datapoint
> > from me is that I've never done that and would never think to.  Fwiw.  Do we
> > have any data there?
> 
> How do you add native apps to the home screen?

I go to the screen I want to add the shortcut to, open the app menu, then tap-and-hold the app I want to add to the start screen until it highlights.  Then I place it where I want it on the start screen.  Works great for me, I didn't realize this might be "non-standard".
Comment 8 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 17:55:58 PDT
(In reply to comment #7)

> FWIW, I think we're both on the same page that we'd like the boundary
> between the native android apps and web apps to be blurred.  Not trying to
> obstruct that effort, just noting my personal experience with the current
> impl.

You are right, we have the same goal. Once we get designs and UI for accessing webapps from inside Fennec, I'm sure we'll add the "Add shortcut to Home" just like we want to do for Bookmarks.

We just don't want to have an "Install as App" in Fennec anymore. That will be handled by websites like Google Chrome Store and Mozilla WebApp Store using the JS API Mozilla Labs is cooking up and Fennec is implementing.
Comment 9 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 18:05:40 PDT
(In reply to comment #8)
> We just don't want to have an "Install as App" in Fennec anymore. That will
> be handled by websites like Google Chrome Store and Mozilla WebApp Store
> using the JS API Mozilla Labs is cooking up and Fennec is implementing.

Agree that the current "Install as App" impl isn't something we want in the long run, since among other things it doesn't actually (can't) pin the app to cache.

But an installable app is just one with a manifest that allows it to be fully cached locally.  "App stores" are valuable, but having a simple button in the UI that allows me to pin random apps I come across on the internet is valuable too.  (That button would not be the current "Install as App" button, just to be clear.  In particular, only pages with the right manifest would get it.)  There's a place for both, I think.
Comment 10 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 18:28:56 PDT
(In reply to comment #9)

> But an installable app is just one with a manifest that allows it to be
> fully cached locally.  "App stores" are valuable, but having a simple button
> in the UI that allows me to pin random apps I come across on the internet is
> valuable too.  (That button would not be the current "Install as App"
> button, just to be clear.  In particular, only pages with the right manifest
> would get it.)  There's a place for both, I think.


We agree here too. I was surprised the app manifest work done by Mozilla and Google do not allow for <link type="app-manifest" ...> or something similar.

I think Mozilla Labs has plans to add such support to their spec.
Comment 11 Chris Jones [:cjones] inactive; ni?/f?/r? if you need me 2011-08-03 18:36:33 PDT
I thought that <html manifest="..."> was the way to say that.  Are we talking about different things?  (Honestly note sure.)
Comment 12 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-03 19:03:30 PDT
(In reply to comment #11)
> I thought that <html manifest="..."> was the way to say that.  Are we
> talking about different things?  (Honestly note sure.)

We are talking about different things. <html manifest="..."> is a way to create an application cache (offline resource cache), but is not considered a "webapp." It's useful for working offline or minimizing network resource fetching.

Groups working on specs for "webapps" are using manifests to spec out icons, root URL, permissions as well as optional bundles of HTML, JS and CSS. There is some overlap between appcache and webapps, and webapps can use appcache, but they are different concepts.
Comment 13 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-04 12:07:44 PDT
Madhava - just a ping for how we should expose this in the UI. Button on the popup? Action in the bookmark context menu? Both would be easy and should fit in alright. The menu would work for when you are browsing the list of bookmarks. The button on the popup would work for when you make the bookmark.
Comment 14 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-05 13:22:21 PDT
Created attachment 551139 [details] [diff] [review]
patch v1

This patch does a few things:
* Converts the mobile nsIPhoneSupport -> nsIShellService since desktop has a similar concept and mobile's was poorly named
* Update the one place using nsIPhoneSupport (in browser-ui.js) to use nsIShellSupport
* adds "createShortcut" to nsIShellService. Much of the implementation for this comes from the existing, but unused, nsIWebappSupport (only Android)
* Removes the now _really_ unused nsIWebappSupport code. See http://hg.mozilla.org/mozilla-central/rev/d30d0303cd0f for the commit that added it.
* Adds a Gecko BOOKMARK intent, so we can track opening bookmarks in Firefox in special ways. Currently we just open it like any URL.
* Adds a bookmark context menu action "Add Shortcut to Home" which uses nsIShellService.createShortcut

Most of the patch is removing the old nsIWebappSupport.

Brad - am I doing the intents right? I thought about not making BOOKMARK and just using intent.setData(URI) instead. We already have an intent handler for that. If we want to do anything cool with bookmarks, I figure we should make them special intents now.
Comment 15 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-05 13:23:00 PDT
Comment on attachment 551139 [details] [diff] [review]
patch v1

Asking Fabrice to look at the front-end code
Comment 16 [:fabrice] Fabrice Desré 2011-08-05 13:41:30 PDT
Comment on attachment 551139 [details] [diff] [review]
patch v1

Review of attachment 551139 [details] [diff] [review]:
-----------------------------------------------------------------

looks good to me.
Comment 17 Brad Lassey [:blassey] (use needinfo?) 2011-08-07 20:56:41 PDT
Comment on attachment 551139 [details] [diff] [review]
patch v1

Review of attachment 551139 [details] [diff] [review]:
-----------------------------------------------------------------

::: embedding/android/GeckoApp.java
@@ +322,5 @@
> +        else if (Intent.ACTION_VIEW.equals(action)) {
> +            String uri = intent.getDataString();
> +            GeckoAppShell.sendEventToGecko(new GeckoEvent(uri));
> +            Log.i("GeckoApp","onNewIntent: " + uri);
> +        }

why are you reordering this? Am I missing some change that isn't just reordering the logic?

@@ +328,5 @@
> +            String args = intent.getStringExtra("args");
> +            GeckoAppShell.sendEventToGecko(new GeckoEvent(args));
> +            Log.i("GeckoApp","Intent : WEBAPP - " + args);
> +        }
> +        else if (action.equals("org.mozilla.gecko.BOOKMARK")) {

action can be null, declare a static final string for the bookmark action and call equals on it (see bug 677088)

::: mobile/components/build/nsPhoneSupport.cpp
@@ +111,5 @@
> +    
> +  jEnv->CallStaticVoidMethod(jGeckoAppShellClass, jCreateShortcut, jstrTitle, jstrURI, jstrIconData, jstrIntent);
> +
> +  return NS_OK;
> +#else

move all of this into the AndroidBridge. We'd like all jni code to live there
Comment 18 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-07 21:39:30 PDT
(In reply to Brad Lassey [:blassey][blassey@mozilla.com] from comment #17)

> ::: embedding/android/GeckoApp.java
> @@ +322,5 @@
> > +        else if (Intent.ACTION_VIEW.equals(action)) {
> > +            String uri = intent.getDataString();
> > +            GeckoAppShell.sendEventToGecko(new GeckoEvent(uri));
> > +            Log.i("GeckoApp","onNewIntent: " + uri);
> > +        }
> 
> why are you reordering this? Am I missing some change that isn't just
> reordering the logic?

Just an OCD change. Moving the main action to be the "if", not the "else". Let me know if you want it reverted.

> action can be null, declare a static final string for the bookmark action
> and call equals on it (see bug 677088)

Fixed

> > +  jEnv->CallStaticVoidMethod(jGeckoAppShellClass, jCreateShortcut, jstrTitle, jstrURI, jstrIconData, jstrIntent);
> > +
> > +  return NS_OK;
> > +#else
> 
> move all of this into the AndroidBridge. We'd like all jni code to live there

Done
Comment 19 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-07 21:40:43 PDT
Created attachment 551388 [details] [diff] [review]
patch v2

Made Brad's Android changes. Front-end is the same.

Tested on Nexus One.
Comment 20 Madhava Enros [:madhava] 2011-08-08 14:33:20 PDT
Would be great, additionally to

- add the option to the Bookmark popup, as demonstrated in this mockup: http://cl.ly/3H0h0f0g2a2Y2U1W0s0T (thank you, Ian!)
- add the "Add to Home Screen" option to the menu on any awesomescreen item that is a bookmark -- so that you can use search to find the one you want to add to the desktop
Comment 21 Madhava Enros [:madhava] 2011-08-08 14:40:56 PDT
Also - let's get a good homescreen bookmark icon we can use.
Comment 22 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-08 19:40:38 PDT
pushed to inbound: http://hg.mozilla.org/integration/mozilla-inbound/rev/39c342b0c16d

Had to add the nsIWebappSupport.idl back, since WebappSupport.js implements it
Comment 23 Mark Finkle (:mfinkle) (use needinfo?) 2011-08-09 07:01:24 PDT
http://hg.mozilla.org/mozilla-central/rev/39c342b0c16d
Comment 24 Ian Barlow (:ibarlow) 2011-08-09 08:22:39 PDT
(In reply to Madhava Enros [:madhava] from comment #20)

> - add the option to the Bookmark popup, as demonstrated in this mockup:
> http://cl.ly/3H0h0f0g2a2Y2U1W0s0T (thank you, Ian!)

Upon further discussion, let's do something that looks a little more like a context menu, with an arrow coming out of the side, like this: http://cl.ly/2k242Q1e37280k1h2z18
Comment 25 Ian Barlow (:ibarlow) 2011-08-09 08:26:04 PDT
Actually, let's make that menu a little narrower so it feels less in the way http://cl.ly/1P3z27012u3Y00140C1U
Comment 26 :Ehsan Akhgari (out sick) 2011-08-09 09:03:14 PDT
http://hg.mozilla.org/mozilla-central/rev/39c342b0c16d
Comment 27 Aaron Train [:aaronmt] 2011-08-15 07:40:50 PDT
Verified Fixed
Mozilla/5.0 (Android; Linux armv7l; rv:8.0a1) Gecko/20110815 Firefox/8.0a1 Fennec/8.0a1

Litmus:
https://litmus.mozilla.org/show_test.cgi?id=25821
https://litmus.mozilla.org/show_test.cgi?id=25822
https://litmus.mozilla.org/show_test.cgi?id=25823

Note You need to log in before you can comment on or make changes to this bug.