798722 - Re-style error messages displayed by window manager when non-fatal mozbrowsererror event received

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect, P3)

Description

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

STR
 (1) Swipe to e.me
 (2) Choose Fashion
 (3) Choose Elle
 (4) Tab "Beauty" link in nav bar

Instantly, the scary-looking "This app is having problems; This app is not loading properly" dialog pops up.  My girlfriend literally startled when this happened.

There are a few things wrong here
 - the Elle link is somehow broken?  Or something.  (I can't tell what's wrong from the popup.)
 - the notification is pretty extreme if it's just a broken link
 - the notification is quite jarring in context

Comment 1

[Ben Francis [:benfrancis]]

I don't *think* this is a 404, I think it's the process that e.me mozbrowser iframes use crashing, probably due to low memory. We should follow this up in bug 798747

However, this error message does not look like it's implemented to visual spec.

Patryk can you confirm what the visual design should be for the crashed process error message inside the everything.me wrapper and also what do you expect for 404 page not found messages inside the wrapper?

Comment 2

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

(In reply to Ben Francis [:benfrancis] from comment #1)
> I don't *think* this is a 404, I think it's the process that e.me mozbrowser
> iframes use crashing, probably due to low memory. We should follow this up
> in bug 798747
> 

The homescreen can't launch subprocesses, so I don't think this is the case at all.

Assuming the blocking+ reset was accidental, resetting.

Comment 3

[Ben Francis [:benfrancis]]

Yes I didn't mean to un-set basecamp-blocking+, sorry.

What process do the mozbrowser iframes the everything.me wrapper uses run in? I thought they all shared one content process, but separate from the homescreen process.

Comment 4

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

They run in the homescreen process.

Comment 5

[Ben Francis [:benfrancis]]

Ah OK, so it looks like the error message you're seeing is displayed whenever there's an error which the mozbrowsererror event informatively describes as "other", which is basically any error which *isn't* type "fatal". In other words I couldn't have been more wrong!

Because there are such a broad range of errors that are currently described as "other" it's a bit tricky to provide a more meaningful error message, but maybe we could make it look less scary like you say.

Interestingly, looking through the homescreen and window manager code it appears that everything.me and homescreen bookmark content actually runs in the system app's process, not the homescreen app's process. The bookmark launchers call window.open() which is caught by the system app which then opens the iframe. That seems to indicate that it would be possible to run this content OOP, but it's currently not implemented that way (remote doesn't get set to true for windows opened from the homescreen app without a manifest URL).

I'm curious to know why because presumably that means a content crash would also take the system app down with it.

Comment 6

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

(In reply to Ben Francis [:benfrancis] from comment #5)
> Interestingly, looking through the homescreen and window manager code it
> appears that everything.me and homescreen bookmark content actually runs in
> the system app's process, not the homescreen app's process.

/me gets out defibrillator, tries to restart heart

I loaded the Elle ~app and then kill -9'd the homescreen process, and the Elle app went down.  So I don't believe the content is loaded in the main process.

But if that's true, we would need to fix, statim! :)

Comment 7

[Ben Francis [:benfrancis]]

Yep, wrong again. I asked Vivien and it turns out the iframe object is created by the platform when window.open(_blank) is called. Then the createFrame method in the window manager takes an optional argument to use an existing frame object (from the mozbrowseropenwindow event) instead of creating a new one (which is what I thought it was doing). In this case that means the frame gets created in the homescreen process after all :)

https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/window_manager.js#L766 should have been a giveaway.

This probably explains why the homescreen crashes so much when I open lots of homescreen bookmarks.

I will follow up with UX about re-designing the "other" error message.

Comment 8

[Ben Francis [:benfrancis]]

Josh, this is the other error message issue I mentioned on IRC.

This is slightly different to bug 729898 (which applies to Gecko error screens used in the browser app) and bug 796750 (which is when there's no Internet connection at all).

This applies to apps, everything.me content and homescreen bookmarks opened in "the wrapper" which experience one of a large number of different errors when loading a page.

The Gaia window manager doesn't display gecko error messages for apps & bookmarks like the browser app does, it instead catches an "error" event and acts accordingly.

The API which provides these events currently only sends two types of error, "fatal" and "other". When it receives a "fatal" error the app closes and the transitive message is displayed saying that the app experienced a problem and closed. When it receives an "other" error it currently displays a slightly scary looking (and not properly designed) "This app is having problems; This app is not loading properly" screen with a big sad face.

Unfortunately this error could be displayed in a wide range of circumstances (the kinds of things which would trigger gecko error screens in the browser app), but Chris suggests we could design it to make it look a bit less scary.


Chris: If you want to file a bug about broken links in everything.me "apps" we should probably have a separate bug for that, but I don't know who at everything.me has a Bugzilla account.

Comment 9

[Josh Carpenter [:jcarpenter]]

Thanks for the overview, Ben, much appreciated. 

Dumb question: what happens if we don't show anything for "Other" errors?

Comment 10

[Ben Francis [:benfrancis]]

(In reply to Josh Carpenter [:jcarpenter] from comment #9)
> Dumb question: what happens if we don't show anything for "Other" errors?

I suspect the Gecko errors are displayed (I think the window manager is just overlaying it's own error message over the top!), but I haven't tested this.

Comment 11

[Ben Francis [:benfrancis]]

...which means that Patryk may want to drop the transparency from the design he's proposed in bug 729898

Comment 12

[Patryk Adamczyk [:patryk] UX]

(In reply to Ben Francis [:benfrancis] from comment #11)
> ...which means that Patryk may want to drop the transparency from the design
> he's proposed in bug 729898

Removed the transparency: https://www.dropbox.com/lightbox/home/OWD_Moz_share/_Production/Visual%20Design/System/Loading%20Errors?select=app_connection_error_121010.jpg

Asset for the background here: https://www.dropbox.com/sh/orycxuowe2lsewp/3mL7blxZVN#f:app_connection_error_bk.png

Comment 13

[Staś Małolepszy :stas]

(In reply to Patryk Adamczyk [:patryk] UX from comment #12)
> (In reply to Ben Francis [:benfrancis] from comment #11)
> > ...which means that Patryk may want to drop the transparency from the design
> > he's proposed in bug 729898
> 
> Removed the transparency:
> https://www.dropbox.com/lightbox/home/OWD_Moz_share/_Production/
> Visual%20Design/System/Loading%20Errors?select=app_connection_error_121010.
> jpg

This is also a good change for another reason, as far as I understand it, in context of bug 796157.  Back when the app error overlay was transparent, you could see the text 'Try again. B2G will attempt...' in the error message.  'B2G' is the current value of the 'brandName' entity in the testing builds.

The same string is used in the browser app.  I assumed it would be preferable to say 'Firefox will attempt' in the browser app, but then again, we only had one string to cater to both scenarios (browser error and app error).

Now that the transparency is gone, we don't need to worry about the app error scenario in context of this specific error message.  Yay!

Comment 16

[Josh Carpenter [:jcarpenter]]

On the topic of strings, it's worth mentioning that they have not been reviewed yet for these items (Browser, and system errors). If there is weirdness (eg: mentioning "B2G"), we will catch and fix.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

This is fixed by 796750 's patch, closing.