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

VERIFIED FIXED

Status

defect
P1
major
VERIFIED FIXED
7 years ago
7 years ago

People

(Reporter: tchung, Assigned: vingtetun)

Tracking

unspecified
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:+)

Details

Attachments

(3 attachments, 1 obsolete attachment)

Reporter

Description

7 years ago
Posted image 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
Reporter

Updated

7 years ago
Summary: System App crashes and restarts homescreen, leaving → System App crashes and restarts homescreen, showing a dual status bar
Reporter

Comment 1

7 years ago
Posted file logcat
logcat during crash reproduction.
Actually this is a gaia bug: we show the icon for the system app, and here we try to launch it again OOP.
Component: General → Gaia
Hardware: ARM → All
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
There are at least two bugs here.  Removing the icon should fix this specific problem.
Summary: System App crashes and restarts homescreen, showing a dual status bar → System App launches OOP when an icon that shouldn't be there is tapped, showing a dual status bar
Severity: normal → major
blocking-basecamp: ? → +
Reporter

Updated

7 years ago
Blocks: 804086
Posted patch patch (obsolete) — Splinter Review
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.
Assignee: nobody → fabrice
Attachment #675347 - Flags: review?(21)
(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 :(
Priority: -- → P1
Summary: System App launches OOP when an icon that shouldn't be there is tapped, showing a dual status bar → Why do we have a system app? Let's remove it.
dup of bug 804313.... but I'll reverse dup since this is already nom'ed.
Posted patch PatchSplinter Review
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.
Attachment #675347 - Attachment is obsolete: true
Attachment #675347 - Flags: review?(21)
Attachment #676989 - Flags: review?
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
Attachment #676989 - Flags: review?(fabrice)
Summary: Why do we have a system app? Let's remove it. → Why do we have a system app icon? Let's remove it.
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.
Attachment #676989 - Flags: review?(fabrice)
Reporter

Updated

7 years ago
Duplicate of this bug: 807733
Reporter

Comment 13

7 years ago
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.
(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 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
Attachment #676989 - Flags: review?(fabrice) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Reporter

Comment 17

7 years ago
Verified fixed on the 11-20 nightly build
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.