[Bug 805738] Launching any app as 'first-running-app'

[borjasalguero]

This code is needed in order to launch any app as 'first-running-app'. We need it in order to launch FTU ensuring Homescreen. In this approach 'Homescreen' is launched as an App instead of an activity, so probably we could https://bugzilla.mozilla.org/show_bug.cgi?id=801995 as fixed with this code.

[borjasalguero]

@vingtetun @arcturus r?

[vingtetun]

You really don't want to use indexOf as a way to validate the application. Otherwise people will create application with such a name in the origin and weird things may happened.

[borjasalguero]

@vingtetun Agree. Any idea? Could we add a special permission as 'homescreen' in order to identify univocally which app is the homescreen?

[borjasalguero]

@vingtetun Fixed until having a better solution.Now I'm using origin, so there is no possibility that other app use this app name.

[vingtetun]

And this code will also happily broke the homescreen app since it needs to be informed of the 'Home' key for UI/UX purpose.

[borjasalguero]

@vingtetun 'Home' issue fixed. Thanks for the tip.

[borjasalguero]

@vingtetun @arcturus @ferjm Any ideas to identify univocally the homescreen app? I think that applying it to the current code of this PR fixes the bug and also unblock the FTU App, so it's so importart to solve it asap! ;) Any suggestions? Thanks for your support!

[arcturus]

just one thing, @vingtetun @borjasalguero, should we have those constants (the origins from the apps we want to detect) in the profile some how?

It's hurting my eyes a bit to see the string hardcoded, imagine that other operator decides to create their own app for ftu, perhaps they could setup this via the build process.

I'll send a PR to this showcasing if you agree.

[borjasalguero]

@arcturus Great! We should have an univocally way of identifying which apps are 'homescreen' and 'ftu', in order to manage it properly in window_manager.js and bootstrap.js. Thanks!

[ferjm]

I have to agree with @arcturus that having this strings hardcoded is kind of awful.

It sounds like having this constants (homescreen, FTU app ...) as a preference exposed via settings might be a good solution.

Apart from that, I've taken a quick look to the PR code and I don't really like the idea of having two different calls to navigator.mozApps.mgmt.getAll() (bootstrap.js and applications.js) and storing the homescreen details (homescreenURL, homescreenManifestURL, etc.) in the WindowManager. IMHO this should probably be handled only by Applications instead.

[borjasalguero]

@ferjm +1. Why dont we create some get methods in applications.js in order to get a common way of request homescreen or ftu app (i.e. Applications.getHomescreen())? We could use it in window_manager.js and bootstrap.js.

[ferjm]

That sounds good to me.

I'm kind of worried about the lack of unit tests for the system app though, specially for the WindowManager and Applications. This changes should probably be followed by proper unit tests :)

[andreasgal]

I would like to second what @ferjm said. We need some unit tests here.

[borjasalguero]

After talking with @vingtetun we are going to do this in several steps. This code only allow us to launch any app as 'first-running-app', but doesn't include the management of applications that we were talking previously. I will add it in FTU PR and also we are going to open a 'followup' in order to use the same mechanism for 'homescreen'. This code now it's rebased and working properly! (I've tested launching any app as 'first-running-app' -for example SMS App-).
@vingtetun @arcturus r?

[vingtetun]

Any reasons for moving this line?

[borjasalguero]

It was some lines below. Now I've refactored the conditions so it's easy to see the new case that we need.

[borjasalguero]

After reviewing the whole code I found the following issues that has been solved with latest changes:

• homecreen var was updated in WindowManager.launchHomescreen, so we are forcing our bootstrap.js to use WindowManager.launchHomescreen() for updating this values properly.
• Due to the first issue, (currentApp == homescreen && newApp)in setDisplayedApp it's true due to currentApp and homescreen are both null if we dont update homescreen properly (so we cant launch any app as first running app because we are not going to show it properly).
• ensureHomescreen() has an 'app' param that has now sense (nobody is using it!)

All this issues have been fixed. @arcturus @vingtetun @ferjm Do we need to open the followup for moving the method retrieveHomescreen()to applications.js or could we include it in the scope of this PR? Thanks!