156997 - Fix up error pages with plain English descriptions and a nice appearance

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect, P1)

Description

[Adam Lock]

The XHTML error pages that appear when a page cannot be loaded or a host is
unreachable need to be fixed up with proper descriptions and a nice appearance.

See bug 28586 for more info.

Comment 1

[Alex Bishop]

Am I right in thinking that the current error messages are those listed in
http://lxr.mozilla.org/mozilla/source/docshell/base/appstrings.properties?

Comment 2

[Adam Lock]

appstrings.properties contains the short error descriptions and is shared with
the the message box errors. BTW bug 156988 will move it from docshell/base into
docshell/resources along with the other error page things.

The long error descriptions are contained here:

http://lxr.mozilla.org/seamonkey/source/docshell/resources/locale/en-US/netError.dtd

At the moment the long descriptions are just placeholders, but hopefully we can
puts some decent descriptions in there.

The XHTML & JS that build the page are here:

http://lxr.mozilla.org/seamonkey/source/docshell/resources/content/

netError.xhtml includes netError.js & netError.dtd. It has an onload handler
that calls a js method called fillIn() which extracts the error id, the short
description and the bad url from the location and insets them. The long
descriptions are held in hidden DIV elements, one of which is shown depending on
the value of the error id. I tried to insert the long desc entities dynamically,
but they don't expand out, hence the reason I'm using hidden DIVs.

The mechanics of the error page aren't set in stone. This is just a first
working cut and is open to modification and improvement.

Comment 3

[Matthew T (active 1999-2002)]

Taking.

Comment 4

[Michael Hendy (Hendikins)]

Attachment: Style sheet for error pages

Attaching for mpt...

Comment 5

[Michael Hendy (Hendikins)]

Attachment: Sample error page: Host not found

Comment 6

[Michael Hendy (Hendikins)]

Comment on attachment 95416 [details]
Sample error page: Host not found

.

Comment 7

[Matthew T (active 1999-2002)]

Comment on attachment 95415 [details]
Style sheet for error pages

This style sheet will need to be made platform-specific, for things like
obeying platform-specific alert element spacing, and not scaling the `caption'
font on OS/2 (where it might be a bitmap font).

Before that happens, three problems should be fixed.
(1) The DD element has extra whitespace at the top and I don't know why.
(2) The whitespace between the DT and the DD should be a margin-right or
similar
    for the DT, not a padding-left for the DD (so that the whitespace would not
    be present in cases when the DT was omitted). When I try and do that,
    however, the DD clears to below the DT instead of remaining beside it.
(3) Various elements add extra whitespace to the bottom, beyond the 1em that
    should be present. This isn't noticable unless you resize the window to
    slightly more than 1em taller than the error message, whereupon you'll get
    an enabled vertical scrollbar when you shouldn't. (To see what I'm talking
    about, add * { border: 1px dotted green; }.)

Comment 8

[Matthew T (active 1999-2002)]

Comment on attachment 95416 [details]
Sample error page: Host not found

This is an example error page. Note that the long message itself contains HTML
in the form of a <code> element: I suppose %1 in the properties file could be
replaced by <code>nameofHost</code>.

Comment 9

[timeless]

uid is being phased out.

Comment 10

[Adam Lock]

What is replacing uid? This bug really needs to go to people who'd like design
some nice error pages. The docshell doesn't care much what the pages look like.

Comment 11

[timeless]

there is no replacement. assign bugs to people and leave them in the component
where the code lives.

Comment 12

[Adam Lock]

Attachment: Some plain English descriptions

I have attached a sample set of plain English descriptions. Thoughts?

We should whack this in (or something similar) and work on the CSS stuff
separately.

Comment 13

[Adam Lock]

Attachment: Patch 2

Second patch clears up text a little and adds some sample styling to the xhtml.

Comment 14

[Adam Lock]

Can someone please review this? The patch is much better than what is there now
and migth stimulate more work on the error page stuff.

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Comment on attachment 105512 [details] [diff] [review]
Patch 2

>Index: mozilla/docshell/resources/content/netError.js
>@@ -47,10 +47,11 @@
>   var i;
> 
>   // Fill in the title
>-  var titleText =  "&" + err + ".title;";
>-  document.title = titleText;
>+  var et = document.getElementById("et_" + err);
>+  if (et != null) {

wouldn't just |if (et) {| work?

>+    et.setAttribute("class", "et_visible");

do et.className = "et_visible" instead.

>   // Long description
>-  var d = document.getElementById(err);
>-  d.setAttribute("style", "display: block;");
>+  var ld = document.getElementById("ld_" + err);
>+  if (ld != null) {
>+    ld.setAttribute("class", "ld_visible");
>+  }

Same here

With that, r=sicking

Comment 16

[Adam Lock]

Attachment: Patch 2 updated

Updated following review suggestions. I've also removed the search
functionality. It can be reinstanted for bug 161276.

Comment 17

[Adam Lock]

Comment on attachment 105512 [details] [diff] [review]
Patch 2

Can I have an sr on this patch to improve the error pages?

Thanks

Comment 18

[Adam Lock]

Comment on attachment 105512 [details] [diff] [review]
Patch 2

Wrong patch, try again

Comment 19

[Hanspeter Niederstrasser]

Suggestions for wording changes in the error messages from Attachment #108911 [details] [diff].

For protocolNotFound.title:

"Not Known" seems clearer than "Not Found" for a bad protocol, eg: "Protocol
htp: Not Known" vs "Protocol htp: Not Found".  This also fits in better with the
text of the error which uses "uknown".

- Protocol Not Found Error
+ Protocol Not Known Error


For protocolNotFound.longDesc:

Adding the incorrect protocol into the error text will make it easier for a user
to know if they mistyped or at least where the error is.  They might not know
that http is a protocol, but will probably recognize that htp is wrong.

- ... starts with a protocol that is not recognized ...
+ ... starts with a protocol (fill in bad protocol here) that is not recognized ...

Comment 20

[Adam Lock]

I'll change the first string, however the second issue requires programmatic
string formatting, so I'd rather leave it until a later checkin. I just want to
put something in now which is significantly better than the existing error page.
It might spur some other people to champion this feature and refine it even further.

Comment 21

[Adam Lock]

Comment on attachment 108911 [details] [diff] [review]
Patch 2 updated

Ben, could you give an sr on this change please?

Comment 22

[Alex Bishop]

Over at MZ's Phoenix forums, some people have been posting concept designs for
the XUL error pages. Some of them are really good.

http://www.mozillazine.org/forums/viewtopic.php?t=3463

Comment 23

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 108911 [details] [diff] [review]
Patch 2 updated

at some point it would be nice to move all the CSS into an external CSS file,
perhaps...

Also, do you really need the <p> tags?	Why not just use the <div>s?  Or style
the <p>s directly?

Comment 24

[Adam Lock]

Attachment: A sample screenshot

Sample screenshot of what it will look like. If you want to improve on it after
the fix goes in, the source is all there...

Comment 25

[Jeremy M. Dolan]

This seems like a step backwards. No search or retry.

Comment 26

[Adam Lock]

Fix is checked in.

Search was zapped because I hardcoded Google which was bad and I didn't see a
way to get a 'favourite' search URL from the prefs. Retry is still there - the
"Try Again" link.

I *hope* this checkin is enough to get people thinking how to improve it further :)

Comment 27

[Adam Lock]

Further to my last comment, bug 161276 deals with making the search option
configurable.

Comment 28

[Adam D. Moss]

How does one turn this on?

Comment 29

[Ian Tommins ('thelem')]

You need to set the hidden pref browser.xul.error.pages.enabled to true

Comment 30

[Jim]

Attachment: pdf crit of screenshot

The attachment is in pdf format. It is a quick crit of Adam's screenshot
(mentioned in comment 24) with a variation. Is there an appropriate newsgroup
to discuss this in?

Comment 31

[Stefan Borggraefe]

This doesn´t work for me. Since this fix landed I get a page only with the "Try
again" link in it. Somehow all the DIVs in netError.xhtml remain invisible as if
fillIn() was never called.

Build ID 2003011208, Windows XP with SP1

Comment 32

[Christian :Biesinger (don't email me, ping me on IRC)]

Stefan: do you maybe have javascript disabled?0.

Comment 33

[Michael Lefevre]

doesn't work for me either on yesterday's nightly on win2k, script is enabled.
hadn't commented because I hadn't had a chance to investigate...

Comment 34

[Stefan Borggraefe]

biesi: JavaScript is enabled.

I just tested this with a fresh CVS build under Debian Linux and it works. Could
this be a Windows-only problem?

Comment 35

[Michael Lefevre]

still doesn't work for me 2003011308/win2k. based on that, Stefan's comment, and
others on IRC, reopening... can someone try this with a windows build and see
what's up?

Comment 36

[Jason Bassford]

WFM with XP (all builds since it was checked in).

My only "nit" is that everything should be left aligned to the same margin, or
at least "Try again" to the error text - but I do see everything.

Comment 37

[Adam D. Moss]

Is the problem theme-related perhaps?  Out of interest, do you have 'use system
colors' or custom colours or similar set?  Because I notice that the error page
has incompletely specified colour usage (e.g. doesn't specify foreground text
colour for the majority of the page, but does specify the background colour of
the main text block).

Comment 38

[Jason Bassford]

To whom was that question directed? <grin>  I do NOT have use system colors set
- I'm using the color and background as specified by the Web site.  The only
other variable here might be that I'm using the Modern theme (under Mozilla) and
a custom Windows theme.

Comment 39

[Michael Lefevre]

tried theme changes and it made no difference. colour settings are all default
(besides, the "try again" appears right at the top of the window), so it's not that.

after some further experimenting, I've discovered that if I remove my prefs.js
file and leave only the line to enable this in user.js, it works. so it's
something lurking in my prefs.js file. will continue to investigate.
re-resolving as fixed for the moment, as it's clearly not a general problem.

Comment 40

[Michael Lefevre]

have reduced my prefs.js down to minimum necessary to cause the problem. with no
user.js and a prefs.js containing just these three lines, it fails as I described:

user_pref("browser.xul.error_pages.enabled", true);
user_pref("capability.principal.certificate.p0.granted", "UniversalXPConnect");
user_pref("capability.principal.certificate.p0.id",
"72:39:8D:54:A7:8C:17:90:37:8B:D3:7D:7D:0C:87:C5:C9:C4:55:E1");

have no idea what script those lines are for, but I don't understand why it
should cause the error pages to fail. suggestions? new bug? not a problem?
Stefan - do you have anything similar in your prefs.js?

Comment 41

[Matthias Versen [:Matti]]

FYI: 
I got a problem with the new error page and a clean (clobbered( cvs build. 
I got only the "try again" link but nothing more.
It worked in a new profile but a fast repair of my old profile didn't fix it.
(delete cache\*.*, xul.mfl, localstore.rdf, chrome\*.* )
After that i deleted all lines in my prefs.js except for Mailnews and it worked
after that.n (also after setting my old prefs back with the GUI)

Comment 42

[Stefan Borggraefe]

Michael: I had the following two lines in my prefs.js:

user_pref("capability.principal.codebase.p0.granted", "UniversalXPConnect");
user_pref("capability.principal.codebase.p0.id", "http://bugzilla.mozilla.org");

After removing them I now see the new error pages.

Comment 43

[kmike]

confirming with nightly build 2003011508 (win98se), after removing following
lines from my prefs.js:
user_pref("capability.principal.certificate.p0.granted", "UniversalXPConnect");
user_pref("capability.principal.certificate.p0.id",
"72:39:8D:54:A7:8C:17:90:37:8B:D3:7D:7D:0C:87:C5:C9:C4:55:E1");

i'm now seeing nice error pages. by the way, that "try again" link didn't even
work with those lines in prefs.js.

Comment 44

[Stefan Borggraefe]

Filed Bug 204068 about the issue described in comment #39 through comment #43.