824672 - "Close" button on app error page doesn't close

Status: VERIFIED FIXED

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

Description

[Mark Hammond [:markh] [:mhammond]]

Most everything.me apps are not working for me (which is a different bug).  In the error page I see, eg,:

"""
on Twitter is having problems
on Twitter is not loading properly
[Close] [Try again]
"""

If you click the "close" button, nothing happens.  logcat shows:

E/GeckoConsole(11686): [JavaScript Warning: "Scripts may not close windows that were not opened by script." {file: "app://system.gaiamobile.org/js/error.js" line: 3}]

and repeats every time you hit "close"

Comment 1

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

TRIAGE: function broken, blocking IMO.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

Triage: QAWANTED to confirm. Everything.me works for people in the triage.

Comment 3

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

(In reply to Joe Cheng [:jcheng] from comment #2)
> Triage: QAWANTED to confirm. Everything.me works for people in the triage.

Long press power button to invoke airplane mode, and then you could simulate a 'network error' environment.

Comment 4

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

Can't we simply remove the 'Close' button. Now that e.me 'applications' runs inside a different process this makes it hard to allow window.close for them. The simpler solution would be to let the user close them the same way an application is close with the Card View.

Comment 5

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

(In reply to Vivien Nicolas (:vingtetun) from comment #4)
> Can't we simply remove the 'Close' button. Now that e.me 'applications' runs
> inside a different process this makes it hard to allow window.close for
> them. The simpler solution would be to let the user close them the same way
> an application is close with the Card View.

Agree, for short term v1 release.

Comment 6

[Mark Hammond [:markh] [:mhammond]]

(In reply to Vivien Nicolas (:vingtetun) from comment #4)
> Can't we simply remove the 'Close' button. Now that e.me 'applications' runs
> inside a different process this makes it hard to allow window.close for
> them. The simpler solution would be to let the user close them the same way
> an application is close with the Card View.

I'm quite new to this device, so I'm probably missing some context, but I'm not sure what "the same way an application is close with the Card View" means.  When that error screen is displayed there is no other obvious way to close the window (eg, no "back" or other elements for this purpose).  Pressing "home" obviously works though.

Comment 7

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

Josh, need your input. We may have to remove the close button in custom error page.
2 options: 
a) remove the close button only in wrapper error page.
b) remove the close button in all app error page.

Comment 8

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

(In reply to Alive Kuo [:alive] from comment #7)
> Josh, need your input. We may have to remove the close button in custom
> error page.
> 2 options: 
> a) remove the close button only in wrapper error page.
> b) remove the close button in all app error page.

Or new idea...but remember the technical issue in comment 4 makes it hard to have a close button in wrapper-type application(e.me).

Comment 9

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

(In reply to Mark Hammond (:markh) from comment #6)
> (In reply to Vivien Nicolas (:vingtetun) from comment #4)
> > Can't we simply remove the 'Close' button. Now that e.me 'applications' runs
> > inside a different process this makes it hard to allow window.close for
> > them. The simpler solution would be to let the user close them the same way
> > an application is close with the Card View.
> 
> I'm quite new to this device, so I'm probably missing some context, but I'm
> not sure what "the same way an application is close with the Card View"
> means.  When that error screen is displayed there is no other obvious way to
> close the window (eg, no "back" or other elements for this purpose). 
> Pressing "home" obviously works though.

Pressing Home is a good enough way of going out an application. Basically you can close other applications by long pressing on the Home button and swiping the application to the top.

Comment 10

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/7213

https://github.com/mozilla-b2g/gaia/pull/7213
Patch v1:
Remove the close button in custom error page. It's much simpler for now.

Comment 11

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

Comment on attachment 696011 [details]
https://github.com/mozilla-b2g/gaia/pull/7213

Sounds good enough for v1.

Comment 12

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

https://github.com/mozilla-b2g/gaia/commit/b1b8376b476cf5146b5df546afe36d71095f8348

Comment 13

[Josh Carpenter [:jcarpenter]]

(In reply to Vivien Nicolas (:vingtetun) from comment #9)
> Pressing Home is a good enough way of going out an application. Basically
> you can close other applications by long pressing on the Home button and
> swiping the application to the top.

Hold up. That's a UX call. And I respectfully disagree:

* Your proposed solution fails to provide users with a clear method for clearing and exiting an error dialogue. It relies on them eventually hitting the Home button (no doubt out of confusion and frustration). That is very poor usability. The error should be rarely encountered, and easily cleared. A built in "Close" method provides that mechanism.

* Relying on the Home button would be doubly poor for usability if it leaves the app open in the background. If the user goes back to the app ("I want to play my game!"), they presumably run into the error screen again. We should prevent that. 

Ping me if you guys need faster responses, but please don't bum rush changes like these without at least waiting a few hours for myself or someone else from UX to respond to our needsinfos. That's poor form.

Let me know what we can do here. Why can we not retain the current, spec'd design? Because it's hard?

Comment 14

[Mark Hammond [:markh] [:mhammond]]

(In reply to Josh Carpenter [:jcarpenter] from comment #13)
> * Relying on the Home button would be doubly poor for usability if it leaves
> the app open in the background. If the user goes back to the app ("I want to
> play my game!"), they presumably run into the error screen again. We should
> prevent that.

Ah, yes, I think this is exactly what has been happening for me, effectively turning a transient error into a permanent one (given that pressing "home" was the only way of leaving the error page)

Comment 15

[Josh Carpenter [:jcarpenter]]

I spoke w/ Alive on IRC, and we agreed that:

* For now, we should remove the Close button from OOP app error prompts, but keep it for non-OOP apps.
* I will then open a second about trying to put a working Close button,
* I will open another bug for related problems with E.me content opening after an error has been encountered, and failing to load properly even when the cause of the initial error has since been resolved.

Comment 16

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

Attachment: https://github.com/mozilla-b2g/gaia/pull/7224

Patch v2: Remove the close button only in wrapper.

App couldn't be tested now because of 824677.

Comment 17

[Josh Carpenter [:jcarpenter]]

Related:

Errors break E.me content, even after error states have been resolved.
https://bugzilla.mozilla.org/show_bug.cgi?id=822329

App UI should never be visible when an overlay (prompt, value selector, etc) is open
https://bugzilla.mozilla.org/show_bug.cgi?id=825144

Comment 18

[Mark Hammond [:markh] [:mhammond]]

FTR, arranging for privileged code to call nsIDOMWindowUtils.allowScriptsToClose() on the window would allow the script's window.close() to succeed - but I'm not sure if that is actually feasible in this context.

Comment 19

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

(In reply to Mark Hammond (:markh) from comment #18)
> FTR, arranging for privileged code to call
> nsIDOMWindowUtils.allowScriptsToClose() on the window would allow the
> script's window.close() to succeed - but I'm not sure if that is actually
> feasible in this context.

Obviously this is not. This is pure web content.

Comment 20

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

(In reply to Josh Carpenter [:jcarpenter] from comment #15)
> I spoke w/ Alive on IRC, and we agreed that:
> 
> * For now, we should remove the Close button from OOP app error prompts, but
> keep it for non-OOP apps.

What is a non-OOP app?  

> * I will then open a second about trying to put a working Close button,

Sounds good.

> * I will open another bug for related problems with E.me content opening
> after an error has been encountered, and failing to load properly even when
> the cause of the initial error has since been resolved.

Could be a regression, not related to this bug.


(In reply to Josh Carpenter [:jcarpenter] from comment #13)
> (In reply to Vivien Nicolas (:vingtetun) from comment #9)
> > Pressing Home is a good enough way of going out an application. Basically
> > you can close other applications by long pressing on the Home button and
> > swiping the application to the top.
> 
> Hold up. That's a UX call.

UX call? Right. This is also a dev call when it is technically complicated to do what you expect and when the deadline is coming so fast. The issue does not exist anymore since there is no more close button. I'm not saying the UX is better but broken functionnalities seems worse.

If this button is really important for you let's do what you suggest in the other comment: open a new bug to add back this button with the blocking-basecamp? flag. If it is important enough to stop the product from shipping then it will be done.

> And I respectfully disagree:
> 

Sorry if I look rude too. Just rushing in order to finish the product in time with what we have handy.

> * Your proposed solution fails to provide users with a clear method for
> clearing and exiting an error dialogue. It relies on them eventually hitting
> the Home button (no doubt out of confusion and frustration). That is very
> poor usability. The error should be rarely encountered, and easily cleared.
> A built in "Close" method provides that mechanism.
> 
> * Relying on the Home button would be doubly poor for usability if it leaves
> the app open in the background. If the user goes back to the app ("I want to
> play my game!"), they presumably run into the error screen again. We should
> prevent that. 
> 
> Ping me if you guys need faster responses, but please don't bum rush changes
> like these without at least waiting a few hours for myself or someone else
> from UX to respond to our needsinfos. That's poor form.


True. And I can only agree that a built-in close button is better. But should we hold v1 for such a button? I don't think so. And If I'm wrong a new bug opened to add it back with the blocking-basecamp? flag would make a decision for us.

> 
> Let me know what we can do here. Why can we not retain the current, spec'd
> design? Because it's hard?

Because it is not doable the way the error page is implemented. The error page is regular web content not opened by a script and regular web content can not decide to close a window/tab. The close button needs to leave outside the content in order to let us close the window. 


I will consider this bug has closed since I believe the blocking-basecamp+ flag has been added because of a broken functionality and feel free to open new bugs to change the current behavior with blocking-basecamp? flag set.

I'm sorry about that since I don't really like the new behavior too...

Comment 21

[Mark Hammond [:markh] [:mhammond]]

(In reply to Vivien Nicolas (:vingtetun) from comment #19)
> Obviously this is not. This is pure web content.

Sorry - it wasn't obvious to me.  I was thinking that maybe the app launch code could call this (eg, shell.js or similar).

Comment 22

[Josh Carpenter [:jcarpenter]]

(In reply to Vivien Nicolas (:vingtetun) from comment #20)
> UX call? Right. This is also a dev call when it is technically complicated
> to do what you expect and when the deadline is coming so fast. The issue
> does not exist anymore since there is no more close button. I'm not saying
> the UX is better but broken functionnalities seems worse.
> 
> If this button is really important for you let's do what you suggest in the
> other comment: open a new bug to add back this button with the
> blocking-basecamp? flag. If it is important enough to stop the product from
> shipping then it will be done.

I hear ya, we know how crazy slammed you guys are, so if you think there's a usability impact—like in this instance—just needsinfo myself or the relevant designer and we can work together to figure out what we'd like to do, and what's feasible within the precious time remaining :)

Comment 23

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

Comment on attachment 696248 [details]
https://github.com/mozilla-b2g/gaia/pull/7224

Cancel the review flag since the revert is reverted.

Comment 24

[Chris Kreinbring [:CKreinbring]]

Verified removal of the Close button in Unagi build 20121231070201.

Comment 25

[Josh Carpenter [:jcarpenter]]

I will connect w/ VIvien and Alive this week to figure out what we can do here.