804883 - Why do we have a system app icon? Let's remove it.

Status: VERIFIED FIXED

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

Description

[Tony Chung [:tchung]]

Attachment: screenshot

Clicking the rocket app, will crash, restart, and leave a duplicate status bar at the top of the screen

See screenshot and logcat.

Repro:
1) install 10-23-2012 unagi stable build:
2) tap the "System Rocket" app
3) verify app crash, and homescreen restarts
4) when homescreen starts up again, verify the status bar is shown twice.

Expected:
- system app doesnt crash, no dual status bar app

Actual:
- crash and dual bar

Comment 1

[Tony Chung [:tchung]]

Attachment: logcat

logcat during crash reproduction.

Comment 2

[[:fabrice] Fabrice Desré]

Actually this is a gaia bug: we show the icon for the system app, and here we try to launch it again OOP.

Comment 3

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

There's some part of the system app that's launched OOP, but I don't know how it relates to this.  I'm going to file another bug for that.  See https://bugzilla.mozilla.org/show_bug.cgi?id=804884#c1

Comment 4

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

There are at least two bugs here.  Removing the icon should fix this specific problem.

Comment 5

[[:fabrice] Fabrice Desré]

Attachment: patch

I think it's ok to check for the 'System' literal since we already are in a isCoreApps() case.
Another solution would be to have a "hide from homesescreen" flag.

Comment 6

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Fabrice Desré [:fabrice] from comment #2)
> Actually this is a gaia bug: we show the icon for the system app, and here
> we try to launch it again OOP.

The icon is here because we add back a launch_path for the system app in order to fix some system messages duplications :(

Comment 7

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

dup of bug 804313.... but I'll reverse dup since this is already nom'ed.

Comment 9

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

It seems like we need to add more and more launch_path to application because of the way system message handle them, so more application will pop up on the homescreen.

So let's add the list of hidden applications to application-data.js.

Comment 10

[[:fabrice] Fabrice Desré]

Comment on attachment 676989 [details] [diff] [review]
Patch

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

Clearing because I'm not sure if using the manifestURL is doable since using the origin looks very common...

::: apps/homescreen/js/grid.js
@@ +341,5 @@
>            }
>            HomeState.saveShortcuts(init.dock);
> +
> +          for (var i = apps.length - 1; i >= 0; i--) {
> +            if (init.hiddens.indexOf(apps[i]['origin']) != -1) {

origin? Please use manifestURL in new code to be future proof.

::: apps/homescreen/js/state.js
@@ +5,4 @@
>    const DB_NAME = 'HomeScreen';
>    const GRID_STORE_NAME = 'Grid';
>    const DOCK_STORE_NAME = 'Dock';
> +  const HIDDENS_STORE_NAME = 'Hiddens';

Nit: HIDDEN_STORE_NAME

::: build/applications-data.js
@@ +36,5 @@
>      makeURL('communications', 'contacts'),
>      makeURL('browser'),
>      makeURL('feedback')
> +  ],
> +  hiddens: [

Nit: s/hiddens/hidden

Comment 11

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Comment on attachment 676989 [details] [diff] [review]
Patch

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

I have a local branch with s/HIDDENS/HIDDEN/gi. I have not changed the origin/manifestURL since it is used all over the place in the homescreen and using manifestURL belongs to a different bug imo.

Comment 13

[Tony Chung [:tchung]]

Bug 807733 is likely a dupe of this, but that bug does call out there are now 2 more extra apps shown (homescreen and bluetooth icon).   We need to remove those also.

Comment 14

[[:fabrice] Fabrice Desré]

(In reply to Vivien Nicolas (:vingtetun) from comment #11)
> Comment on attachment 676989 [details] [diff] [review]
>  I have not changed the
> origin/manifestURL since it is used all over the place in the homescreen and
> using manifestURL belongs to a different bug imo.

I had to try ;)

Comment 15

[[:fabrice] Fabrice Desré]

Comment on attachment 676989 [details] [diff] [review]
Patch

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

Clearing because I'm not sure if using the manifestURL is doable since using the origin looks very common...

::: apps/homescreen/js/grid.js
@@ +341,5 @@
>            }
>            HomeState.saveShortcuts(init.dock);
> +
> +          for (var i = apps.length - 1; i >= 0; i--) {
> +            if (init.hiddens.indexOf(apps[i]['origin']) != -1) {

origin? Please use manifestURL in new code to be future proof.

::: apps/homescreen/js/state.js
@@ +5,4 @@
>    const DB_NAME = 'HomeScreen';
>    const GRID_STORE_NAME = 'Grid';
>    const DOCK_STORE_NAME = 'Dock';
> +  const HIDDENS_STORE_NAME = 'Hiddens';

Nit: HIDDEN_STORE_NAME

::: build/applications-data.js
@@ +36,5 @@
>      makeURL('communications', 'contacts'),
>      makeURL('browser'),
>      makeURL('feedback')
> +  ],
> +  hiddens: [

Nit: s/hiddens/hidden

Comment 16

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/18f0b4761487476d8da4e04b35ebb51e10b12344

Comment 17

[Tony Chung [:tchung]]

Verified fixed on the 11-20 nightly build