Closed Bug 1076777 Opened 10 years ago Closed 6 years ago

Implement Application Manager menu with default launch app selection

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: davidg, Unassigned)

References

Details

Attachments

(2 files)

In order to implement feature described in bug 1039386, a menu in settings is needed, so the user can change what application is open by default for a given activity.
The UX spec, suggests renaming App Permissions to App Manager, and to add there a submenu called "Default Launch App".
Blocks: 1039386
Comment on attachment 8578117 [details] [review]
[gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master

Hey Arthur, can you please take a look at the PR? 

Please ping me if you need any clarification on how the new panel is supposed to work
Attachment #8578117 - Flags: review?(arthur.chen)
Assignee: nobody → fernando.campo
Status: NEW → ASSIGNED
Comment on attachment 8578117 [details] [review]
[gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master

In general the changes to settings are looking good. However, before diving into a more detailed review, I would like to discuss more about the proposed implementation of handling the default selection.

Currently gecko filters candidates for handling an activity and provide the list to gaia, implementing the logic of default selection in gecko seems making more sense. It is also easier to handle app install/uninstall cases in gecko as it owns all information regarding apps.

Flagging Alive for his input as this involves the system app.
Attachment #8578117 - Flags: review?(alive)
Comment on attachment 8578117 [details] [review]
[gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master

1. What if an app decides not to support one of the activities it had ever claimed?
2. What if an app which is the default handler of certain activity is uninstalled?
3. The most important: the manifestURL of 3rd party app is not permanent. It will change every time you install it. Storing a meaningless string like app://dsfx-egdf-sdsf-sdad.gaiamobile.org in mozSettings db does not make any sense to me.

How could these not being challenged during technical discussion? Or there is not technical discussion?

IMO we should not just do this. Or, do it in gecko, implement some strict API for it like
mozApps.listAllHandler(activityType)
mozApp.setDefaultHandler(activityType)

And let gecko do the trick for us about the three questions above. Although I don't know how to achieve that.. and gecko has trouble to answer the questions, let's not do this.
Attachment #8578117 - Flags: review?(alive) → review-
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #4)
> How could these not being challenged during technical discussion? Or there
> is not technical discussion?

Yes, there was a technical discussion resulting in an UX spec in bug 1039386 (see attachment 8522777 [details]). If there are still some details not clear enough, I suggest to clarify what is missing and then update UX spec.
Comment on attachment 8578117 [details] [review]
[gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master

Questions in comment 4 should be addressed before the review. Cancel the request first.
Attachment #8578117 - Flags: review?(arthur.chen)
Already a few days without any advance. If we need to discuss the technical parts of this whole thing (Gecko VS Gaia treatment), we should start

But until the point where we reach any agreement (keep going, change implementation, stop everything), my recommendation is to back out bug 1073495 and bug 1141479, as at this point we are able to store a default app for launching when a certain activity is called but not to change it after that (unless we install a new application that can handle it).
Flags: needinfo?(marce)
Flags: needinfo?(jonas)
Flags: needinfo?(alive)
(In reply to Fernando Campo (:fcampo) from comment #7)
> Already a few days without any advance. If we need to discuss the technical
> parts of this whole thing (Gecko VS Gaia treatment), we should start
> 
> But until the point where we reach any agreement (keep going, change
> implementation, stop everything), my recommendation is to back out bug
> 1073495 and bug 1141479, as at this point we are able to store a default app
> for launching when a certain activity is called but not to change it after
> that (unless we install a new application that can handle it).

I need Jonas' opinion... but I prefer to pref-off this feature at first. Sorry for this.
Flags: needinfo?(alive)
While waiting for Jonas' opinion, I'm gonna try to answer some of your concerns...


(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #4)
> Comment on attachment 8578117 [details] [review]
> [gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master
> 
> 1. What if an app decides not to support one of the activities it had ever
> claimed?
If an app changes its manifest I'm assuming it needs an update (I don't know if there's an update event, or just an uninstall, install process?). If event is update, we can listen to it. If it's uninstall/install, that should trigger the 'applicationinstalled' listener in the code that nullifies the default app selected.
> 2. What if an app which is the default handler of certain activity is
> uninstalled?
Same as before, we can listen to the uninstall event and if matches the activities on the list, go through the same process.

> 3. The most important: the manifestURL of 3rd party app is not permanent. It
> will change every time you install it. Storing a meaningless string like
> app://dsfx-egdf-sdsf-sdad.gaiamobile.org in mozSettings db does not make any
> sense to me.
Every time we install an app that can handle the activity, we reset the default app, so the manifest would be sufficient for the time being.

> How could these not being challenged during technical discussion? Or there
> is not technical discussion?
As far as I know, there was a discussion, and few changes to the first UX proposal in bug 1039386. I didn't participate on it, but it is my understanding that an agreement was reached before I took care of the bug.
> 
> IMO we should not just do this. Or, do it in gecko, implement some strict
> API for it like
> mozApps.listAllHandler(activityType)
> mozApp.setDefaultHandler(activityType)
I agree with you that Gecko should be much more efficient on this, as doing it on Gaia is just a hijack of the process (Gecko sends a list of possible apps, we take over and choose the app directly after matching it with the content of mozSettings, and tell Gecko to open the app. Instead Gecko can do the matching and just open the selected app).
> 
> And let gecko do the trick for us about the three questions above. Although
> I don't know how to achieve that.. and gecko has trouble to answer the
> questions, let's not do this.
I understand that Gecko would be better for this, but if it can't be done at the moment, maybe we should try with Gaia until we find a better/faster way with gecko.

Anyway, waiting for Jonas' and Marce's opinion on the matter.
My understanding is that such cases are quite similar to installing a new app. This is the description of such case in page 7 of UX spec:

  After user set up default app, if user installs another app that can also complete the action, prompt user of this change and clear the previously set up default.

I would suggest to rephrase such paragraph to cover all these corner cases and move forward with the implementation.

Jenny, what do you think?
Flags: needinfo?(marce) → needinfo?(jelee)
(In reply to Fernando Campo (:fcampo) from comment #9)
> While waiting for Jonas' opinion, I'm gonna try to answer some of your
> concerns...
> 
> 
> (In reply to Alive Kuo [:alive][NEEDINFO!] from comment #4)
> > Comment on attachment 8578117 [details] [review]
> > [gaia] fcampo:app-manager-settings-1076777 > mozilla-b2g:master
> > 
> > 1. What if an app decides not to support one of the activities it had ever
> > claimed?
> If an app changes its manifest I'm assuming it needs an update (I don't know
> if there's an update event, or just an uninstall, install process?). If
> event is update, we can listen to it. If it's uninstall/install, that should
> trigger the 'applicationinstalled' listener in the code that nullifies the
> default app selected.
> > 2. What if an app which is the default handler of certain activity is
> > uninstalled?
> Same as before, we can listen to the uninstall event and if matches the
> activities on the list, go through the same process.
> 
> > 3. The most important: the manifestURL of 3rd party app is not permanent. It
> > will change every time you install it. Storing a meaningless string like
> > app://dsfx-egdf-sdsf-sdad.gaiamobile.org in mozSettings db does not make any
> > sense to me.
> Every time we install an app that can handle the activity, we reset the
> default app, so the manifest would be sufficient for the time being.
> 
> > How could these not being challenged during technical discussion? Or there
> > is not technical discussion?
> As far as I know, there was a discussion, and few changes to the first UX
> proposal in bug 1039386. I didn't participate on it, but it is my
> understanding that an agreement was reached before I took care of the bug.
> > 
> > IMO we should not just do this. Or, do it in gecko, implement some strict
> > API for it like
> > mozApps.listAllHandler(activityType)
> > mozApp.setDefaultHandler(activityType)
> I agree with you that Gecko should be much more efficient on this, as doing
> it on Gaia is just a hijack of the process (Gecko sends a list of possible
> apps, we take over and choose the app directly after matching it with the
> content of mozSettings, and tell Gecko to open the app. Instead Gecko can do
> the matching and just open the selected app).
> > 
> > And let gecko do the trick for us about the three questions above. Although
> > I don't know how to achieve that.. and gecko has trouble to answer the
> > questions, let's not do this.
> I understand that Gecko would be better for this, but if it can't be done at
> the moment, maybe we should try with Gaia until we find a better/faster way
> with gecko.
> 

I think we need to do the correct thing instead of a rushing feature just because "android has it", "UX loves it". Also, this feature is not part of v3 spec, so we have much time to do it more accurately.

The biggest problem of doing those in gaia is the asymmetric information about mozApps between gecko and gaia.
Also, can someone describe what was actually implemented here. Due to the way filters work, every time a "pick" activity is launched, a different set of app selections might be shown. So the first time the "pick" activity is launched apps A and B might be shown and the user chooses to use app B as default. But next time apps A, B and C might be valid options, in which case the user might want to choose app C. Will B still be used without giving the user a choice?
Flags: needinfo?(jonas)
Jonas, one question, when you say that "next time apps A, B and C might be valid options" is it because C app has been installed afterwards?
If that's the case, the the default app is reset and the menu selection will be presented again to the user.
Flags: needinfo?(jonas)
No, it could be because the app which is invoking the MozActivity does so with slightly different parameters. But still using the same activity-name.
Flags: needinfo?(jonas)
As far as I know current UX spec is simple enough to avoid such corner cases
(In reply to Alive Kuo [:alive][NEEDINFO!] from comment #11)
> 
> I think we need to do the correct thing instead of a rushing feature just
> because "android has it", "UX loves it". Also, this feature is not part of
> v3 spec, so we have much time to do it more accurately.
I agree with you on doing the correct thing, and do it accurately.
But I'd like to say that this hasn't been added nor rushed just because 'android has it' or because 'UX loves it'. It was a requirement from a partner, so it was proposed, discussed and agreed. Or at least that was my understanding.

Also, at the end, the most important thing is "to do it".


(In reply to Jonas Sicking (:sicking) from comment #12)
> Also, can someone describe what was actually implemented here. Due to the
> way filters work, every time a "pick" activity is launched, a different set
> of app selections might be shown. So the first time the "pick" activity is
> launched apps A and B might be shown and the user chooses to use app B as
> default. But next time apps A, B and C might be valid options, in which case
> the user might want to choose app C. Will B still be used without giving the
> user a choice?
Short answer: yes, it might happen that B would still be the app launched.
Longer with more context: it would depend on specifics for that activity and type, as setting a default app is based on filters too (not just for a 'pick' activity, but for a 'pick this type').
Following the specs (https://bug1039386.bugzilla.mozilla.org/attachment.cgi?id=8522777 page 10), the filters has been grouped to try to shorten the otherwise huge list of possibilities. 

So using your example, imagine that apps A and B are able to pick both 'jpg' and 'png', app C is able to pick only 'png' (and 'jpg' and 'png' are both in the filters supported by the default launch [https://github.com/mozilla-b2g/gaia/blob/master/shared/js/default_activity_helper.js#L66-L70 / example with 'open' as we don't support 'pick']).
We first call an activity with 'pick/jpg', so A and B are shown in the list. We choose B, and mark it as 'default'.
When an activity for 'pick/png' is called, even though the list would show now A,B and C, app B would be open directly, as it was set before as default for all the filters on default_activity_helper.
This problem was raised already, you can see it on bug 1039386 comment 18.

Another problem I see is that for setting an app as default, it'd only need to be capable of handling the specific type/filter requested at the moment of the call (specs 6 - set the app as the default app to complete this particular action and bypass the selection step). 
But when we want to change that on settings, the app list presented would include only the apps that handles ALL filter options (specs 10 - To reduce complexity, [...], ONLY show the apps that support all the content types)
Hello all,

I agree with Marcelino in comment 10 and have updated the spec on p.7 to cover cases 1 & 2 mentioned in comment 4. Thanks!
Flags: needinfo?(jelee)
But comment 10 is incorrect. A different set of apps appearing in the picker menu does *not* only happen when a new app is installed. So making UX drafts based on that assumption seems bad no?
Now UX specs includes also following cases that require clearing the default app setting:
* If an app which is the default handler no longer supports the activity tied to that default setting
* If an app which is the default handler of certain activity is uninstalled
Do you miss anyone else?
Flags: needinfo?(jonas)
Flags: needinfo?(jonas)
Assignee: fernando.campo → nobody
Status: ASSIGNED → NEW
Firefox OS is not being worked on
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: