769842 - Improve the error message reported in a web app when there is no internet connection with no app cache loaded

Status: RESOLVED WONTFIX

(Firefox Graveyard :: Webapp Runtime, defect, P2)

Description

[Jason Smith [:jsmith]]

Right now, the error message reported when there is no internet connection in a web app without app cache loaded states that Firefox cannot connect to X server at host origin. We should provide a better notification method that the app requires an internet connection in order to be used (i.e. reword error message in terms of an app).

Comment 1

[Myk Melez [:myk] [@mykmelez]]

B2G does a better job at this now; perhaps we can reuse their approach and/or code.

Comment 2

[Marco Castelluccio [:marco]]

Could you post a screenshot of the B2G error message? I think we can modify the error page by overriding netError.dtd and/or netError.xhtml.

Comment 3

[Marco Castelluccio [:marco]]

In the "Secure connection failed" case (about:neterror?e=nssBadCert&u=url&d=desc), should we allow the user to add a temporary/permament exception?
I think we shouldn't for security reasons, but at the same time I wouldn't like to limit the users' freedom.
What do you think?

Comment 4

[Myk Melez [:myk] [@mykmelez]]

(In reply to Marco Castelluccio [:marco] from comment #3)
> In the "Secure connection failed" case
> (about:neterror?e=nssBadCert&u=url&d=desc), should we allow the user to add
> a temporary/permament exception?
> I think we shouldn't for security reasons, but at the same time I wouldn't
> like to limit the users' freedom.
> What do you think?

We walk a tightrope in the browser, providing a way to add an exception but making it just-the-right-amount of burdensome to dissuade casual use.

But the browser has to contend with a lot of legacy content that the runtime doesn't necessarily have to handle.  So I would default to not allowing an exception; then we can revisit if we find that this policy is too constraining.

Comment 5

[Marco Castelluccio [:marco]]

Attachment: Patch

The patch:
 - removes the UI code that allowed to add cert exceptions.
 - overrides the error strings.
 - on about: pages, sets the title as the application name.

I could use some help in re-writing the strings for the webapprt case.
In this patch, I've used:
 - for errors imputable to app developers, the string "Please contact us to report this problem" (hoping users will understand that "us" is not Mozilla but the app developers);
 - for recoverable errors, the same strings that we use in Firefox (but removed some references to technical stuff).

Comment 6

[Donovan Preston [:fzzzy]]

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

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

My first bit of feedback is that I think it would be a good idea to split this into two patches, one that moves text that is repeated into entities, and another which changes the wording of the text. I think this will make both patches easier to read and give feedback on.

Overall, I don't really like the changes which now say "us". I know this allows the text of the message to refer to either the website author or the app author, in each context, but it sounds too generic to me.

Maybe it would be better to leave the strings as they currently are for Firefox, but override them in the web runtime to say "The app author" or something similar.

Finally, I really think we need to solicit the input of UX or someone who has experience writing language which users can understand.

Comment 7

[Valentin Tsatskin [:vt]]

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

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

::: webapprt/content/pages/netError.xhtml
@@ -346,5 @@
> -             error types.  -->
> -        <div id="securityOverrideDiv">
> -          <a id="securityOverrideLink" href="javascript:showSecuritySection();" >&securityOverride.linkText;</a>
> -          <div id="securityOverrideContent" style="display: none;">&securityOverride.warningContent;</div>
> -        </div>

Why were the security related sections removed? Was this old code that we no longer use?

::: webapprt/locales/en-US/webapprt/overrides/netError.dtd
@@ +95,5 @@
>  <!ENTITY nssFailure2.title "Secure Connection Failed">
>  <!ENTITY nssFailure2.longDesc "
>  <ul>
>    <li>The page you are trying to view cannot be shown because the authenticity of the received data could not be verified.</li>
> +  <li>Please contact us to report this problem.</li>

I'm assuming you removed the "Alternatively, use the command found in the help menu to report this broken site." portion of the help text because the command is no where to be found in the help menu anymore.

As a side note: Do we know when and why this command was removed? Perhaps we can bring it back and directly put it in the error page for more website reporting.

@@ +119,5 @@
>      that &brandShortName; is permitted to access the Web.</li>
>  </ul>
>  ">
>  
> +<!ENTITY contactDeveloperDesc "<p>Please contact us to report this problem.</p>">

I really think this should be expanded more to tell the user how to actually contact "us". I don't think we should be trying to speak for the developer in our error messages, and instead we should give a recommendation to the user such as "Please contact the website owner to report this problem.".

However, this recommendation still does not do anything to help the user establish the contact. Imagine if crash reports just told the user to send over the crash log manually without any given email address or website to do so.

@@ -161,4 @@
>  <!ENTITY malwareBlocked.title "Suspected Attack Site!">
>  <!ENTITY malwareBlocked.longDesc "
>  <p>Attack sites try to install programs that steal private information, use your computer to attack others, or damage your system.</p>
> -<p>Website owners who believe their site has been reported as an attack site in error may <a href='http://www.stopbadware.org/home/reviewinfo' >request a review</a>.</p>

How come this was removed?

@@ +134,4 @@
>  ">
>  
>  <!ENTITY cspFrameAncestorBlocked.title "Blocked by Content Security Policy">
> +<!ENTITY cspFrameAncestorBlocked.longDesc "&contactDeveloperDesc;">

Do you know under what circumstances the cspFrameAncestorBlocked error occurs? I'm wondering if it ever occurs in normal user use and maybe it's only seen by developers? If so, we should probably keep the technical description if no normal user will see it.

Comment 8

[Marco Castelluccio [:marco]]

Thank you for the quick feedback!
Note that these strings are for the webapp runtime, where apps run in their own chromeless window. I'm not modifying any Firefox string.
My general idea was to provide a "native-like" experience.

Comment 9

[Myk Melez [:myk] [@mykmelez]]

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

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

I'm having trouble testing this patch.  If I disable my network connection, delete the cache for the Mykzilla app, and start the app, I get a modal alert dialog that says "Alert, www.mykzilla.org could not be found. Please check the name and try again." on top of a blank window.

Do you have any tips on how to ensure that the page appears?

::: webapprt/content/webapp.js
@@ +54,5 @@
>      // of the page being loaded if it's from a different origin than the app
>      // (per security bug 741955, which specifies that other-origin pages loaded
>      // in runtime windows must be identified in chrome).
>      let title = WebappRT.config.app.manifest.name;
> +    if (!isSameOrigin(location.spec) && !location.spec.startsWith("about:")) {

Nit: it should be possible for the second conditional to be simply:

  location.protocol != "about:"

::: webapprt/locales/en-US/webapprt/overrides/netError.dtd
@@ +119,5 @@
>      that &brandShortName; is permitted to access the Web.</li>
>  </ul>
>  ">
>  
> +<!ENTITY contactDeveloperDesc "<p>Please contact us to report this problem.</p>">

I agree with both fzzzy and vt that "us" is too vague.  We should tell the user to contact the "app developer" when the problem is something that should be reported to the developer.  And if the manifest includes a URL for the developer, then we should linkify that call to action so the user can click the link to go to the developer's website.

@@ +129,5 @@
>  
>  <!ENTITY phishingBlocked.title "Suspected Web Forgery!">
>  <!ENTITY phishingBlocked.longDesc "
>  <p>Entering any personal information on this page may result in identity theft or other fraud.</p>
> +<p>These types of forgeries are used in scams known as phishing attacks, in which fraudulent web pages and emails are used to imitate sources you may trust.</p>

Nit: removing the first occurrence of "web" here doesn't make much difference.  It occurs later in the text; and it's in the title of the dialog.  It's also fairly reasonable to continue calling this a "web forgery", even if that leaks the abstraction.  We can't entirely plug those leaks anyway, especially in this dialog, which is all about them.

::: webapprt/locales/jar.mn
@@ +9,5 @@
>      locale/webapprt/webapp.properties              (%webapprt/webapp.properties)
>      locale/webapprt/getUserMediaDialog.dtd         (%webapprt/getUserMediaDialog.dtd)
> +    locale/webapprt/appstrings.properties          (%webapprt/overrides/appstrings.properties)
> +    locale/webapprt/dom.properties                 (%webapprt/overrides/dom.properties)
> +    locale/webapprt/netError.dtd                   (%webapprt/overrides/netError.dtd)

Thanks for fixing the indentation here!

Comment 10

[Marco Castelluccio [:marco]]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #9)
> I'm having trouble testing this patch.  If I disable my network connection,
> delete the cache for the Mykzilla app, and start the app, I get a modal
> alert dialog that says "Alert, www.mykzilla.org could not be found. Please
> check the name and try again." on top of a blank window.
> 
> Do you have any tips on how to ensure that the page appears?

I was directly modifying Startup.jsm to load "about:neterror?...", because it was the fastest way to test.

> ::: webapprt/locales/en-US/webapprt/overrides/netError.dtd
> @@ +119,5 @@
> >      that &brandShortName; is permitted to access the Web.</li>
> >  </ul>
> >  ">
> >  
> > +<!ENTITY contactDeveloperDesc "<p>Please contact us to report this problem.</p>">
> 
> I agree with both fzzzy and vt that "us" is too vague.  We should tell the
> user to contact the "app developer" when the problem is something that
> should be reported to the developer.  And if the manifest includes a URL for
> the developer, then we should linkify that call to action so the user can
> click the link to go to the developer's website.

I really like the link idea.
About "Please contact the app developer", I'm not entirely sure because it doesn't feel like a native app, where developers usually refer to themselves.
What about:
- If manifest.developer is set: "Please contact " + manifest.developer.name + " at " + manifest.developer.url
- If manifest.developer isn't set: "Please contact the app developer".

Comment 11

[Donovan Preston [:fzzzy]]

(In reply to Marco Castelluccio [:marco] from comment #10)
> What about:
> - If manifest.developer is set: "Please contact " + manifest.developer.name
> + " at " + manifest.developer.url
> - If manifest.developer isn't set: "Please contact the app developer".

I think this is a great idea.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

Per bug 1238079, we're going to disable the desktop web runtime and remove it from the codebase, so we won't fix these bugs in it.