280190 - netError.xhtml should look a bit better.

Status: RESOLVED FIXED

(Core :: DOM: Navigation, enhancement)

Description

[Jasper]

The netError.xhtml file hasn't been visually updated in a while. Since the
Camino project might use this page instead of the (annoying) error sheets I
thought it was time to give it a little update.

Changes:
Error Title: 16px black
Short description: left alligned, no inset
Long Description: left alligned and text has padding so the blue back doesn't
look cramped
Retry: I made this a button istead of textual link, I believe users will now
actually see and use that option.

Comment 1

[Jasper]

Attachment: v1.0

Above post says it all.

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 172660 [details]
v1.0

Sorry, I'm a bad choice for UI changes like this.

Comment 3

[Jesse Ruderman]

Dup of bug 170216?

Comment 4

[Jesse Ruderman]

*** Bug 170216 has been marked as a duplicate of this bug. ***

Comment 5

[Greg Campbell]

Probably shouldn't use pixels for accessibility reasons. 

div.et_visible {font-size: medium;}

Comment 6

[Jasper]

Attachment: title size is now large

I changed the title font size to be large instead of 16px as commented on in
comment #5.

Comment 7

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

Comment on attachment 172957 [details]
title size is now large

note that this file has changed a bit on trunk now, this file will need the
same changes

Comment 8

[Jasper]

Hm I used the file that was in the embed.jar within Camino trunk builds.

Comment 9

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

yeah, it changed today, 5 hours ago ;)

Comment 10

[Vidar Haarr (not reading bugmail)]

Attachment: adds a warning icon

How does this look?
There could be a problem with the icon - I think toolkit and seamonkey use
different alert icon identifiers? How can we solve that, if that's the case?

Comment 11

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

Comment on attachment 173482 [details] [diff] [review]
adds a warning icon

I don't think I'm the best person for such reviews, changing request to neil.

but shouldn't the entire CSS be in a /skin/ directory?

Comment 12

[[e-mail]]

Comment on attachment 173482 [details] [diff] [review]
adds a warning icon

>+      background: url('chrome://global/skin/icons/Warning.png') no-repeat;
At the very least this line belongs in a skin .CSS file.
And you probably only meant to set tha background image.
And I haven't bothered to look for other errors.

Comment 13

[nrlz]

For the short descriptions it would be nice if the URL (or whatever caused the
error) was bolded so that people don't have to read the entire sentence.
e.g.,
==
<h1>Address Not Found Error</h1>
<b>www.fdgdfgdfgdfg.com</b> could not be found. Please check the name and try again.
==
<h1>Protocol Not Known Error</h1>
<b>dfgdg</b> is not a registered protocol.

Comment 14

[William Price]

Attachment: Patch for netError.xhtml -- adjusts CSS for a nicer look & feel

Not sure where this bug stands w/ relation to the /skin/ directory...
And, honestly, this is my first patch submitted to the Mozilla Project.

This patch simply adjusts the inline style of netError.xhtml to look more
visually appealing.  NOTE: There are a couple units specified in pixels; I
tried to avoid these where possible, but I used it for spacing from the top of
the window so it shouldn't affect accessibility.  It seemed to respond well
when I tested at a wide range of font sizes and window dimensions.

Comments & review welcomed/requested.

Comment 15

[William Price]

Attachment: Improved look & feel for netError.xhtml CSS (v1.1)

Fixed a mistake where error page wouldn't present scroll bars if the page was
zoomed larger than the area available in the client window.  Also improved
contrast while I was at it.  (I can post a screenshot if desired.)

Still requesting comments / review.
Truely sorry for the bugspam.

Comment 16

[William Price]

Attachment: Patch for existing netError-related files (xhtml, dtd, js)

Ok, I heard rumor that new releases of Firefox were (or are going to) be
shipping with error pages turned on by default.  If that's indeed the case, I
think the error page needs some updating so I did a larger overhaul than my
previously ignored patch.

FEATURES:

- Better splash of color

- Clean look & flow makes it easier to read

- All the text isn't in one huge block

- Increased the "try again" button by quite a bit to
  make it stand out and easier to target with a mouse

- Added a graphic to liven the page up a bit (user
  experience); it's amazing what one image can do.

- Redesigned the markup and added a separate CSS file
  (containing a lot more style info than the previous
  document)

- The "try again" button is now visible at ANY reasonable
  screen resolution WITHOUT the need to scroll.

- Text explanations will scroll when the browser window
  is small

- Text displayed to the user is updated and, IMHO, a bit
  more helpful; added a "suggestions" section in addition
  to the detailed explanation.


POSSIBLE ISSUES:

- If accepted, this change **WILL AFFECT LOCALIZATION**.
  The netError.dtd file defines additional entities now.

- There are *new* files to be added to the tree:
  two PNG graphics and one CSS file; CSS could probably
  be inlined into the XHTML head, but there's a pretty
  decent amount of styling going on.

- I'm not sure exactly where to put the three new files.
  (2 PNGs, 1 CSS)  For now, they're in the toolkit global
  chrome; someone needs to check the patch against the
  correct tree locations because I'm not 100% certain
  I know where everything's supposed to be.

Comment 17

[William Price]

Attachment: Zip file of each individual file as changed, plus the 3 new files.

This file is related to the previous patch file and includes:

readme.txt
bug280190-wrp_v2.diff
netError.xhtml
netError.dtd
netError.js
netError.css *
netErrorExclamation.png *
netErrorGradient.png *

The three files marked with (*) are new, not revisions.

Comment 18

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

could you attach a screenshot of the new look of the error pages?

Comment 19

[William Price]

Attachment: 1024x768 screenshot

Comment 20

[William Price]

Attachment: 640x480 screenshot showing scrolling

These screenshots fix one typo and one small cosmetic change that I made since
posting the patches; I'll re-generate the patch(es) with these changes if it
looks like this is going to go forward.

Comment 21

[Jasper]

OMG I'd rather not see this land, looks like a bad idea. And most importantly to
me it looks even worse in combination with Mac OSX.

Comment 22

[William Price]

(In reply to comment #21)
> OMG I'd rather not see this land, looks like a bad idea. And most importantly to
> me it looks even worse in combination with Mac OSX.

Could you elaborate a little more?  And since I don't have a MacOS machine
nearby at the moment, perhaps you could attach (or just e-mail me) a screenshot?

Comment 23

[William Price]

Attachment: Alternate "bubble" graphic for consideration (Firefox-specific)

While waiting for people's opinions on whether it's good to move forward on
this patch I've been updating my local copy per my personal preferences.  This
alternate bubble graphic is what I prefer for Firefox.	(It has a bit more
character... literally.)

While I'm on the subject, it seems like Jasper is very concerned about MacOS X
and Camino appearance on this and other bugs.  I just wanted to mention that
since the CSS and images are completely external to the main XHTML that
different projects can substitute their own files for their own look & feel. 
For instance, the Firefox-specific bubble would obviously be for Firefox only.

This raises another topic from above: themes/skins.  I'm not versed on how to
make this (i.e. colors & image) such that it can be changed by a theme.  If
someone wants to help walk me through that, drop an e-mail.  But I (personally)
think this is leaps and bounds over the current alternative; ability to skin or
not.

Continuing...

What I've got so far is more of a "remove and replace" than an actual patch --
as the diffs indicate.	There's very few lines I haven't touched.  There *are*
some sections of code that seem to be dead and removable, but I'd like input
from the module owner and/or author (which appears to be Adam).

Since I really, REALLY want the error page to look nice, I went through the
meta bug to see where else I can help out.  Bug 188795 includes a few patches
to add additional error cases and text.  It's patches would become incompatible
with this change, so I took the spirit of that bug and included the 4 new cases
here, along with user-friendly text explanations.

A possible blocker is bug 290219 where error pages break if Javascript is
disabled in the prefs (probably the difference between document.documentURI and
document.location.href factoring into whether Javascript is enabled).  I can't
fix that, but I repurposed the (previously unused?) "generic" entity and
altered the XHTML and JS to display a truly generic error title and description
if the provided error type is missing or unknown.  This works regardless of
javascript, but until 290219 is fixed the page won't display the real error
condition **nor will the Retry button work**.  This is probably a blocker.

Other changes since my last attachment:
- Minor cleanup/changes to CSS for asthetics and better 
  small window support.

- Removed excess padding from the blue title area that
  resulted from a change and I didn't notice.

- Adjusted the gradient PNG to better aid text/bkgrnd contrast

If it looks like this is going forward, I'll repackage and post a new patch for
review.  In the meantime, I'm looking for feedback/opinions and *detailed*
criticsm.

Comment 24

[William Price]

Attachment: Patch for review (part 1 of 2 for this bug; existing files)

Patch of existing XHTML, JS, and DTD files including all of the changes
mentioned above as well as some cleaned up JS with the help of biesi.

NOTE: Orphaned code has been removed and it operates properly; Adam has not
reviewed these changes.

Comment 25

[William Price]

Attachment: Zip of new files for *themes* (css, 2 png's) to go in chrome://global/skin/

Contains the new files to be added to themes:

1) CSS styling for the error page
2) PNG gradient used by the stylesheet
3) PNG graphic used by the stylesheet

Intended to be available at chrome://global/skin/  in any/all default theme
packages.  Optionally, the Firefox themes can use the alternate bubble graphic
attached to the bug.

Comment 26

[William Price]

Attachment: Retry: patch of existing XHTML, JS, DTD files for review

Sorry for this and any resultant bugspam.  The first patch did not upload
completely; hopefully it will this time.

Comment 27

[William Price]

Attachment: 3rd try: Patch of existing XHTML, JS, DTD files for review

They say 3rd time's a charm... very sorry for the spam, my eyes were playing
tricks on me.  Just a couple more e-mails as I clear the botched review flags.

Comment 28

[Ben Goodger (use ben at mozilla dot org for email)]

Hi William, 

Thanks for the great work reviving this bug... I've contacted Steve
Garrity/Kevin Gerich who may want to work with you on the visuals here. I'm
cc'ing them. 

Comment 29

[Steven Garrity [:sgarrity]]

William: We would like to help out with the visual style of this page. We'll
have a go at it as soon as we can.

For reference, here is Safari's new similar error page:
http://media.arstechnica.com/images/tiger/safari-friendly-error-big.jpg

Comment 30

[William Price]

Attachment: Static version of the page to make styling easier

Sounds great; the important thing is that it looks good.  :)

I've attached a static version of the XHTML page that doesn't require the
localization DTD entities or the javascript code.  Hopefully this makes it
easier to style as if it were really just a regular page.  It currently links
to netError.css in the same directory as the document.	You can start from
scratch or grab the netError.css out of the most recent zip file attachment for
a starting point.

Note that, for technical reasons, all the explanation/hint text is included in
the document and portions are simply hidden/revealed by their CSS classes by
the JS code.  Simply FYI in case you wonder why you get screenfulls of
headlines and paragraphs of text.

After I looked at the Safari example you gave I wanted to clarify something: I
intended the error page to be simple and brief in terms of the main error title
and the quick one or two sentences inserted by the browser.  But my complaint
with other browsers' error pages (Safari's included) is that they don't explain
enough or offer much information to help a user who's willing to change
something and try again.  (This would also cut back on a few support questions
over on the MozillaZine forums.)  So while restyling/designing, I hope to keep
the text portions rather intact (minus a bit of phrasing here and there).

Thanks for the help and attention, guys. :)

Comment 31

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Keep in mind that error pages can be shown inside of iframes - for example, on
www.dilbert.com, I have ad.doubleclick.net resolving to 0.0.0.0.  Screenshot at
http://ctho.ath.cx/tmp/moz/dilberterror.png - new error pages should not look
obnoxious in situations like this.

Comment 32

[William Price]

Good point, and one I had overlooked initially.

A possible solution would use the Javascript to detect the frame size and turn
off the graphics and "extended help" for frames smaller than... 500x400 or so? 
Just have to mark some elements as display:none or change their CSS class so
they render an alternate, smaller style.

(Really need to get the Javascript issue fixed in bug 290219.)

Comment 33

[Steven Garrity [:sgarrity]]

I've done some experimenting with the visuals for this page. I'm trying to go
for a simpler look (something we won't get tired of looking at over the next 5
years).

This includes the Warning.png that Kevin Gerich did for the alert dialogs in
Firefox:

http://actsofvolition.com/include/netError/0.2/netError.xhtml

It seems to me to be much too wordy. Perhaps we could trim it down, or hide
everything but the basic message in a hidden DIV we show when you click on a
"More Info..." or "Help" link? Or maybe we just link to the actual help? (that's
what Safari does)

Comment 34

[William Price]

I could see going with linking to the built-in help or a method of
showing/hiding the extra details.  The important thing (to me) is that the extra
assistance be easy to understand and easily accessible -- in addition to the
pleasing visual style you've achieved.

The built-in help option has the advantage that one would expect this help to be
there, instead of randomly in the tree as it is now -- though to add new errors
with the current implementation, one would have to modify netError.xhtml anyway
so perhaps it's best to keep it in one place for now?  The show/hide option is
easy enough by moving the short description out of the longContent div and we
can expand/collapse that div as necessary.

My only real issue be the lack of the "try again" link/button.  It's true that
people could press refresh and get the same effect, but they might simply press
enter in the URL location and it's not clear that there will be a difference. 
(From what I understand, the former will repost form data, the latter will not.)
 I think having the button near the error message the user is reading will make
it more likely that they'll get what they expect.  So is there a
visually-appealing way to get that button/link back on the page, preferably in a
way where it doesn't scroll "below the fold" on smaller window/frame sizes?

Thanks for your help with this.

Comment 35

[Pavel Cvrcek [:JasnaPaka]]

(In reply to comment #33)
> http://actsofvolition.com/include/netError/0.2/netError.xhtml

Looks nice but i think that this preview isn't "clear". What something like
Opera error page? White background, highlisted topic, description of error, link
to help (support) and maybe something like "Try again" button but like link.
Only simple CSS, no graphics - maybe only one error pic.

Comment 36

[Steven Garrity [:sgarrity]]

Pavel: do you have a screenshot of the Opera error page?

Comment 37

[Pavel Cvrcek [:JasnaPaka]]

Attachment: Screenshot of Opera 8 error page

Steven: Opera 8 error page.

Comment 38

[Jasper]

Ok just to bump the discussion. 

I think the current style of error page we have isn't that bad. It's simple and
it says what needs to be said. Adding images and elaborate styles won't really
help people to understand what's going on.

As I showed with my first attachemnt, I think simply polishing up the current
page with some items that make everything easier for the eye and esier to
read/understand is good enough.

Comment 39

[Steven Garrity [:sgarrity]]

Jasper: Do you have the current style or your first attachment in a form that's
easily viewable with content in it? Thanks.

Comment 40

[Bruce Davidson]

Undoing fields unknowingly changed when cc'ing myself. Apologies for the spam.

Comment 41

[Chris Lawson (gone)]

Just a very brief comment: the error pages currently displayed by Camino are
fine *except* for one huge mistake.

They never say *what* is throwing that error up there. I thought for the longest
time it was an HTTP proxy doing it until I remembered some discussion I saw on
Bugzilla a while back.

I really like the way Safari makes it clear that the error is being throw by
Safari. Something really simple, like adding the browser icon to the page, would
go a long way toward fixing this.

cl

Comment 42

[Steven Garrity [:sgarrity]]

A few more design variations:
http://actsofvolition.com/include/netError/0.3/netError.xhtml
http://actsofvolition.com/include/netError/0.4/netError.xhtml

Would an image on this page be accessible on a theme basis?

Comment 43

[William Price]

(In reply to comment #42)
> Would an image on this page be accessible on a theme basis?

The last CSS I posted (from my design) is intended to go into themes, as well as
any images it points to in chrome, so YES.  This probably wasn't too clear, so
I'll clarify now:

The actual look, as debated here, will probably apply to *default* themes but
allow theme authors to restyle the error page to better suit their respective
styles (including spacing and colors, etc).  By removing the inline CSS from
netError.xhtml and referencing an external CSS file in the chrome://global/skin
space, we automatically make the error pages skinnable.  Likewise, any images
the CSS references should also be in the global/skin area.

(And I like the image (or an image) there on 0.4, but it seems too narrow for a
standard browser window; making it a percentage of the frame width might suit
iframes and full windows better?  I thought the exclamation triangle looked good.)

One thing I'm not clear on is when to call this "done" in terms of this bug and
seek to land it; I assume, Steven, that you can shoot mail over to Ben for
approval when you think it's at a final-enough stage?

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #42)
> A few more design variations:
> http://actsofvolition.com/include/netError/0.3/netError.xhtml
> http://actsofvolition.com/include/netError/0.4/netError.xhtml

Steven, it looks nice but I doesn't like the tag soup. So I modified your CSS
and accordingly to the tags I'm using. Now the code is much cleaner. My version
can display all currently available error descriptions when it's called with the
needed parameters. 

For example:
http://hskupin.info/tmp/netError/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again.

Like you already did in your version, we should support a clickable url within
the short description if neccessary. This can be done by updating the file
http://lxr.mozilla.org/aviary101branch/source/toolkit/locales/en-US/chrome/global/appstrings.properties

Actually I don't know if there is still a bug which handles the displaying of
the real URL inside the location bar instead of the chrome URI? The chrome URI
is still too complex and long which will confuse the normal user.

Comment 45

[Jasper]

This is defenitly going in the right direction.

03 is perhaps to wide, but the general look and feel is very good.
04 the use of an icon is good to gove the user a better idea that it's
important. But red might be to scrayy, so maybe go for also a smaller yellow
triangle icon. The red one looks like a stop sign which is not what we want
people to do. And please make this one wider, somwhere inbtween wood probably
look a lot better.

Comment 46

[Jasper]

Note that I agree with comment 41, which states that using the app icon in the
error page is probably even a better idea. Steven and I have talked about that,
maybe it is something we should look at seriously. How would we tackle this
technically so that also Camino can do this?

Comment 47

[William Price]

Ok.. lots to respond to:

1) Again, the goal is to not only come up with a nice-looking error page, but
one that is skinnable by themes as well.  The netError.xhtml file itself is NOT
a part of the theme, so it needs to be pretty flexible so a theme's CSS can
style it decently and not be locked into what we decide here for the "default."

2) To that effect, the "tag soup" mentioned in comment #44 is somewhat required.
 It's not really necessary for Steven's design *but* the extra DIV tags allow
theme authors to group or modify some of the longer content that would otherwise
not be possible with standalone (ungrouped) DIVs as your reduced case uses.  For
an example, see my original concept page where certain parts (the longer content
contained in multiple DIVs) would scroll together as one if the frame was too
small.  Though I do admit we can clean up some of the tag attributes.

3) Regarding the hyperlink in the short description... Modifying
appstrings.properties is NOT an option IF we intend to keep the dialog boxes
around as even a hidden option.  Remember, that text is used in those dialog
boxes too AFAIK and putting a hyperlink in the dialog (if even possible) is not
good.  If we're trashing the option to use dialogs instead of error pages
completely, then it's no big deal.  I like the URL hotlink, but it might have to
wait for another time.

4) Just as a reminder, in the trunk builds, the failed URL *is* in the location
bar -- there's no more visible chrome:// url.

5) Finally, to Jasper: If the application's icon is (or can be put) in the
global/skin chrome space (as a part of the theme), it can be referenced by the
CSS.  Camino's default theme's CSS could point to that PNG and all is good.

6) The favicon, however, is a slightly different issue.  Since it must be
<link>'d in netError.xhtml (again, which is not a part of the theme) we'd be
hard-coding a chrome URL to a new icon (e.g.
chrome://global/skin/netErrorIcon.png) so the icon itself can be defined by a
theme by changing the file itself rather than a variable location as we can do
with CSS.

Thanks for all the input, everyone.

Comment 48

[Karsten Düsterloh]

Attachment: Safari 2.0 error page

Comment 49

[Samuel Sidler (old account; do not CC)]

William: Replying to your commments a bit.

I'm unsure what this bug is and maybe it's description should be changed. I was
under the impression it *was* deciding on a default error page. If that's not
true and it's simply deciding on the basic structure of one, to be changed, can
you please update the description and file a new bug for the default design/CSS?

Secondly, regarding the favicon, if there isn't a way (or semi-doable way) to
allow them to be changed, I would suggest having *no* favicon. This would
eliminate any visual "weirdness" between platforms.

Comment 50

[Jasper]

(In reply to comment #47)
> Ok.. lots to respond to:
> 
> 1) Again, the goal is to not only come up with a nice-looking error page, but
> one that is skinnable by themes as well.  The netError.xhtml file itself is NOT
> a part of the theme, so it needs to be pretty flexible so a theme's CSS can
> style it decently and not be locked into what we decide here for the "default."

Please file a separate bug for this request. The reason this bug was filed was
to make the page all apps now use look generally better.


> 5) Finally, to Jasper: If the application's icon is (or can be put) in the
> global/skin chrome space (as a part of the theme), it can be referenced by the
> CSS.  Camino's default theme's CSS could point to that PNG and all is good.

Camino does not use themes, we use OS X interface API for our look. So it would
need another solution. And same goes here also. Please file a separate bug for
the implementation of styles on the userError page.

Comment 51

[William Price]

Seems to be a bit of a communication barrier, and I'm trying to remedy that as
best as possible.  So #1, regarding the purpose of this bug, to make
"netError.xhtml ... look a bit better":  that's a very vague specification and
what it takes to do that can be viewed and interpreted differently by each
individual (and has been so far).  I'd love to change the description to
something more specific, but I'm sure others would have issues with what I'd put
regardless; I'll wait on that for now unless there's a consensus.

And yes, the goal *IS* to come up with the default error page.  At the same
time, one of the eventual goals of the error page was to make it skinnable.  (I
get this from past discussions on other bugs involving Adam and others back when
the original netError.xhtml was created.) So...

While creating the "default" error page for Firefox, Seamonkey(?), and Camino we
should also *think ahead*.  If there's a strong argument for not letting themes
skin the error page, so be it.  Otherwise, we need to make consessions.  Having
a few more DIV tags in the XHTML allows for that (using their 'id' tags in
conjunction with custom CSS).

Steven's current designs don't *require* those extra DIVs (we're talking just a
few, really).  But having them present in the XHTML file doesn't hurt this
"default" design but makes it possible to have more complicated or "fancy"
designs at a theme developer's whim.

My comments regarding the "tag soup" were to this effect.  Having them doesn't
hurt the current focus (other than a few more bytes in the file), but it has
some pretty good benefits to eventual theme developers.  That's all.

---------

Next, regarding the favicon: it wasn't my intent to suggest that the favicon
wouldn't be flexible -- it's simply flexible in another way.  The CSS allows us
to place a graphic inside the existing elements as background images.  That
allows theme developers who change the CSS to have one place where they change
those URIs.

Normal graphic elements of the page can be specified in CSS.
The favicon, as referenced by netError.xhtml itself, cannot.

The resolution to keep in mind, then, is that to change the favicon with a
theme, the favicon URI must be "hard coded" to a URI in the *skin* area of the
global chrome.  So instead of changing a URI in CSS you actually have a
*different* file in each theme (each named the same so it resolves correctly)
and you get the same effect.  It'd need to be documented, that's all.

--------------
In summary... the goals of this bug (in my mind) are:

- Design a netError.xhtml/CSS/JS combination that is sufficient for our needs
  as an error page but also has a bit more flexibility for more "complicated"
  designs that a theme author might want.

- Within that design, come up with a default style for the error page to be
  included with the primary Mozilla distributions -- customized for each 
  application as necessary.

- Aside from those technical aspects, ensure that the error page is easy
  to read by the end user and provides enough information that s/he isn't
  left scratching their head for the most part.  Read: user-friendly.

-----------
So far we've seen wording changes, JS re-working, and CSS style proposals.
If those participating or lurking object to anything, PLEASE say so.
We need a consensus on what is to be done or this will *never* land.

Thanks.

Comment 52

[William Price]

(In reply to comment #50)

Bah.  Two posts, same time.

> Please file a separate bug for this request. The reason this bug was filed was
> to make the page all apps now use look generally better.

Yes, but to do that separately will then alter whatever gets decided here
because of that change.  If we can anticipate that here, and take care of it
HERE, then it'll be easier for everyone to get what they want.  I'm trying to
minimize collisions, not create more as a new bug would do.  (See below...)



> Camino does not use themes, we use OS X interface API for our look. So it would
> need another solution. And same goes here also. Please file a separate bug for
> the implementation of styles on the userError page.

But this is easy to fix; a version of netError.xhtml that references CSS and
other images in whatever chrome area you want instead of the global/skin -- and
whoila!  The same page designed here *for* theme support can be used in Camino
with very little modification.  Everyone wins.

I'm trying to satisfy everyone, which is usually an impossible result, but I
*REALLY* see this as possible and want to work to make sure everyone gets what
they need and want.  If you can't see it "working" as I've got it in my head,
send me e-mail and I'll try to better explain without bugspamming everyone.

We're so close to a solution... we just need to all get on the same page.  :-)

Comment 53

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, I re-added the styling divs to get a skinable error page. The div for the
button I left off because it shouldn't be needed IMHO. I changed the red icon
against the Warning.png from the classic skin. Now the favicon contains the
firefox icon to show that's something happened within Firefox itself. With all
changes described above and the underlaying firefox logo the error page looks
much better than the last version.

The updated version can be found here:
http://hskupin.info/tmp/netError/v0.2/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again

Comment 54

[Hixie (not reading bugmail)]

Drive-by commenting: For accessibility reasons, the error title should be an
<h1>, not a <p>. Hope that helps.

Comment 55

[William Price]

(In reply to comment #53)
> Ok, I re-added the styling divs to get a skinable error page. The div for the
> button I left off because it shouldn't be needed IMHO. I changed the red icon
> against the Warning.png from the classic skin. Now the favicon contains the
> firefox icon to show that's something happened within Firefox itself. With all
> changes described above and the underlaying firefox logo the error page looks
> much better than the last version.

Thanks. :-) Though imho the "errorIcon" object should remain a DIV (not an IMG
as you have here).  Reason being that an empty DIV can be sized/placed and have
its *background* image set all in CSS rather than putting anything in the xhtml
file itself and hard-setting a URI -- something that we're stuck with for the
favicon but should avoid elsewhere.

Comment 56

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #54)
> Drive-by commenting: For accessibility reasons, the error title should be an
> <h1>, not a <p>. Hope that helps.

You are right, that should be changed. Currently I see one problem when
replacing the <p> with <h1>. We will have 10 <h1> tags within the <div>
"errorTitle". For accessibility reasons it's a bad idea. So I tried to use only
one <h1> tag and set the content with javascript. This fails because the entity
doesn't get evaluated. I always get e.g. "&malformedUri.title;". How can I do
that? Writing to the <h1> innerHTML attribute doesn't work. Is there any
function which evaluates the entity?

(In reply to comment #55)
> Thanks. :-) Though imho the "errorIcon" object should remain a DIV (not an IMG
> as you have here).  Reason being that an empty DIV can be sized/placed and
> have its *background* image set all in CSS rather than putting anything in the
> xhtml file itself and hard-setting a URI -- something that we're stuck with
> for the favicon but should avoid elsewhere. 

I used an img due to accessibility reasons. Icons which show warnings or errors
shouldn't be placed as background images into a div. By using images and the alt
tag directly everyone can see/read that's something erroneous happend. Images
can also be placed/sized everywhere with the help of CSS. Yes, the URI is
hardcoded, but with a chrome URI we could use an image provided by skins. Same
applies for the favicon.

Comment 57

[William Price]

(In reply to comment #56)
> You are right, that should be changed. Currently I see one problem when
> replacing the <p> with <h1>. We will have 10 <h1> tags within the <div>
> "errorTitle". For accessibility reasons it's a bad idea. So I tried to use only
> one <h1> tag and set the content with javascript. This fails because the entity
> doesn't get evaluated. I always get e.g. "&malformedUri.title;". How can I do
> that? Writing to the <h1> innerHTML attribute doesn't work. Is there any
> function which evaluates the entity?

Yeah, I ran into that issue too, and I know Adam had tried to find a solution. 
Barring some JS function that will parse the element I think I've found a hack
for this using some modified JS and new XHTML attributes defined in the
netError.dtd file.  All the text still must be parsed (and present) in the XHTML
file, with individual <h1>s (or <p>s for the other content), but using JS we can
copy the parsed attribute value of the proper tag into the its innerHTML such
that only one ever has text content for a screen reader to read.  Would that be
acceptible for accessibility?  I assume the screen readers ignore empty H1s.  ???

** This also has the nice side-effect of not selecting ALL error messages when a
user might copy/paste from the error page as it currently does.  :-)



> I used an img due to accessibility reasons.
Fair enough; can't argue that.

Comment 58

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #57)
> Yeah, I ran into that issue too, and I know Adam had tried to find a solution. 
> Barring some JS function that will parse the element I think I've found a hack
> for this using some modified JS and new XHTML attributes defined in the
> netError.dtd file.  All the text still must be parsed (and present) in the XHTML
> file, with individual <h1>s (or <p>s for the other content), but using JS we 

I not happy with this solution, so I tried another way. It's almost hackish but
it works and I don't know another way at this moment. I'm using an JS hash which
holds all entities which will be parsed. Yeaah! ;) So I only have to copy the
correct value to the destination tag and we have a clean (X)HTML file.

http://hskupin.info/tmp/netError/v0.3/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again

Comment 59

[William Price]

If it works as stated, I like this a LOT better, thanks.

I'm getting some JS parsing errors that result in an empty error page,
but I think that's probably because I'm using an altered copy of the DTD
and have some single ticks in the definition, conflicting with the ticks
surrounding the entities in the XHTML.

So with this progress, what are the remaining roadblocks in people's eyes?

- We still need a final _default_ style.

- Jasper: are you satisfied with the Camino work-around or is it
  still unacceptable?  If it's more complicated that I'm seeming
  to think, again, please e-mail me so we can find a solution.

- Anything else?
  We've covered accessibility improvements; cleaner XHTML; some XHTML and
  JS alterations (minimal) that I'll include to cover the case when JS is
  disabled (which would prevent the fill-in code from running, currently);
  theme support (with what I think is an easy compromise for Camino); I've
  got some extra DTD definitions to include a few errors for which there
  are no current entries.  I think that's about it.

Comment 60

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #59)
> If it works as stated, I like this a LOT better, thanks.
> 
> I'm getting some JS parsing errors that result in an empty error page,
> but I think that's probably because I'm using an altered copy of the DTD
> and have some single ticks in the definition, conflicting with the ticks
> surrounding the entities in the XHTML.

Yes, this is the problem. And that's why it's a hack for me. I don't like that
as much but I havn't found another way to parse the entities. Swapping out the
entities into an external js file doesn't work cause it's not parsed. Building
the string dynamically within the xhtml file also doesn't work, cause the line
("&"+err+".title;") results in an parser error. Is anyone else here, who can
show us a better solution to get the entities parsed?

> - We still need a final _default_ style.

Which remaining requirements aren't done within the actual proposal? 

> - Jasper: are you satisfied with the Camino work-around or is it
>   still unacceptable?  If it's more complicated that I'm seeming
>   to think, again, please e-mail me so we can find a solution.

Which workaround do you mean? Perhaps I missed a comment.

>   We've covered accessibility improvements; cleaner XHTML; some XHTML and
>   JS alterations (minimal) that I'll include to cover the case when JS is
>   disabled (which would prevent the fill-in code from running, currently);

If you disable JS within the browser it shouldn't affect the execution of JS for
pages called over a chrome-URI. The current error page works fine with
deactivated JS. :)

Comment 61

[William Price]

>Yes, this is the problem. And that's why it's a hack for me. I don't like that
>as much but I havn't found another way to parse the entities. Swapping out the
>entities into an external js file doesn't work cause it's not parsed. Building
>the string dynamically within the xhtml file also doesn't work, cause the line
>("&"+err+".title;") results in an parser error. Is anyone else here, who can
>show us a better solution to get the entities parsed?

So what I did was change the single ticks surrounding the entity names with full
quotes, and that helped a lot since any " in the DTD would have to be escaped as
" anyway -- no collisions that way when it's parsed.

I also wrote an additional JS function to parse "fake" tags  (i.e. [ul] instead
of <ul>) so that tags could be present in the entity text, still.  This was
required because of where we're parsing.  It would let the JS hash contain all
of the string up until the first <...> tag at which it would truncate.  Moving
tags to the [tag]...[/tag] format and swapping characters at runtime in JS lets
us do both.

Again, it's a hack, but it works until someone can find something more elegant.


> Which remaining requirements aren't done within the actual proposal? 
Requirements?  None except for a visual style everyone's comfortable with and
the drivers (or at least Ben in terms of Firefox) are willing to include with
the official build.  I was kinda leaving that to Steven (and you?) to work out.


> Which workaround do you mean? Perhaps I missed a comment.

Jasper's primarily concerned with Camino.  When I worked theme support into the
error page, his concern was about interoperability with Camino which doesn't use
themes at all.  The "workaround" is changing the chrome URIs in netError.xhtml
(and any CSS) to point to wherever netError.xhtml is currently found in Camino's
chrome path *rather than* chrome://global/skin/... where they'll point for skin
support on Firefox.  Basically, Firefox gets netError with one chrome path,
Camino with another.  With that change, it should all work on both.


> If you disable JS within the browser it shouldn't affect the execution of JS 
> for pages called over a chrome-URI. The current error page works fine with
> deactivated JS. :)

I agree that it *shouldn't* but on my not-too-old build (Deer Park 20050523) if
I disable JS then the fillIn script never runs, leaving the error page *blank*
except for a retry button that *doesn't* work.  Chrome path or not, it's an
issue.  I referenced the bug way up there ^^^ somewhere.  I took your changes
and CSS and modified them for parsing tags in the entities and have the
"general" entity visible by default that gets hidden by the script when it's
run... so at least there's *something* on the page in the case where JS is
disabled.  The retry button is also hidden by default because it too depends on
JS.  The refresh toolbar button will work fine, however.

Anything else?  :-)

Comment 62

[Steven Garrity [:sgarrity]]

There is a 16x16 pixel image in classic.jar:
skin/classic/global/icons/notfound.png that is well suited to be the favicon for
this page.

Here's a copy of it for easy viewing:
http://actsofvolition.com/images/screenshots/notfound.png

This works especially well when you have not-found errors piling up in
background tabs.

Comment 63

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #62)
> There is a 16x16 pixel image in classic.jar:
> skin/classic/global/icons/notfound.png that is well suited to be the favicon
> for this page.

Steven, I updated my proposal to use that icon now and the rest of your v0.4
CSS. I also quickly exchanged the yellow exclamation mark agains a red one. Just
a hack for demonstration. I don't know if my proposal is satisfying some of your
guidelines?

http://hskupin.info/tmp/netError/v0.4/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again

Comment 64

[Greg Campbell]

Visually, I think the body could use some left and right padding so the box
doesn't move all the way to the edges when the viewport is narrow.

Comment 65

[Steven Garrity [:sgarrity]]

We're pretty close (visually, I mean). A few suggested changes to Henrik's
latest version (comment #63):

1. In the CSS file, add "padding: 0 1em;" to the "body" element. This takes care
of the good point in comment #64.

2. In the CSS file, Change the border line in #errorPageContainer to "border:
1px solid #CCC;". This lighter border color makes rough aliasing on the round
corners less apparent (and looks better, I think).

3. Drop the Firefox logo watermark in the background. This just cuts down on
readability - this pages doesn't have to look beautiful, it has to be clear,
simple and straightforward. After all, do we really want to be "branding" a page
that will have an audience made up of people who are frustrated that they can't
load a page? This would also have to be swapped out of "non-official branding"
builds. Looks fine without it.

4. So, we need a good graphic for the main icon on this page. There are two
images already in Classic.jar that might be appropriate, the "i" stop sign
(http://actsofvolition.com/include/netError/0.4b/Error.png) and the yellow "!"
triangle (http://actsofvolition.com/include/netError/0.2/warning.png). However,
both of these are 32px by 32px. I think a 48px by 48px image would be more
appropriate - I'll put out a call to Kevin Gerich for this... KEVVVVIIINNN!!!

I may still tinker with some ideas to make this page look/feel a bit less like a
normal web page and more like an application/system notice. I think what we have
here, though (with the changes I've listed here) is strong and we should go
ahead with it.

Comment 66

[Simon Fraser [no longer active]]

Comments on
http://hskupin.info/tmp/netError/v0.4/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again

Icon:
  As mentioned in other comments, this needs to be improved. Either use a yellow
  warning icon (with exclamation point) or a red error icon, but not a mixture of
  both. The icon should have antialiased edges.

Title:
  "Address Not Found Error" is too geeky. It should just be "Address not found",
  for example.

Subheading:
  The site name should be in quotes, or emboldened to make it distinct from the
  rest of the text.

Blurb:
  This paragraph is just too long. No-one will read it. It's probably better
  to either just come up with shorter paragraph, or break the possible reasons
  for the failure into a list.

  What is "directory name service"? DNS stands for Domain Name Server (or
  Service or System). If you spell it out, put DNS in brackets so that people
  recognize the acronym.

Comment 67

[William Price]

(In reply to comment #66)

I have a new DTD file that addresses all of the textual concerns you mentioned
with the exception of bolding/quoting the site in question.  That phrase is from
the same appstrings source that the dialog box uses, so changes to that should
be a different bug since it affects more than the error pages.

Comment 68

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #65)
> 1. In the CSS file, add "padding: 0 1em;" to the "body" element. This takes
> care of the good point in comment #64.

Definitely a better solution. I've to agree.

> 2. In the CSS file, Change the border line in #errorPageContainer to "border:
> 1px solid #CCC;". This lighter border color makes rough aliasing on the round
> corners less apparent (and looks better, I think).

Done.

> 3. Drop the Firefox logo watermark in the background. This just cuts down on
> readability - this pages doesn't have to look beautiful, it has to be clear,

Done.

Due to the icon... We shouldn't use two different images for the favicon and the
main image. This is irritating in some way.

Further I had to make some changes to the JS file. The entry for ShortDesc was
added as a textnode to the outer div. Now it's a children of the paragraph. What
I've currently not done is cleaning up the JS file, which I'll do this evening.

http://hskupin.info/tmp/netError/v0.5/netError.xhtml?e=dnsNotFound&u=http%3A//www.dnsnotfound.com/&d=www.dnsnotfound.com%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again

(In reply to comment #67)
> I have a new DTD file that addresses all of the textual concerns you mentioned
> with the exception of bolding/quoting the site in question.  That phrase is from
> the same appstrings source that the dialog box uses, so changes to that should
> be a different bug since it affects more than the error pages.

William, can you attach your new DTD here?

Comment 69

[William Price]

Attachment: Proposed new DTD

This new DTD file has rephrasing/rewording throughout.
The goal was to make it more helpful and more concise.
Suggestions welcome.

Comment 70

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #69)
> This new DTD file has rephrasing/rewording throughout.
> The goal was to make it more helpful and more concise.
> Suggestions welcome.

Thank you William. I updated all my files accordingly your changes. I also had
to update the DTD cause with the current xhtml we run in trouble due to <ul>
can't be a child of <p>. After updating all files I merged them into my local
tree. The error page looks really fine for gnomestripe now except the missing
images.

Comment 71

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v4 with all changes since the last week

This is my first proposed patch. It inlcudes all changes for XHTML, JS, DTD and
CSS.

Currently we are using hard-coded colors for text colors and backgrounds. IMHO
we should also update the CSS files for all themes to use the users system
colors?

Comment 72

[Karsten Düsterloh]

Henrik, please use -pu8 (or something similar with -u) for patches.

Comment 73

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch with pu8

Sorry, my script produced the wrong diff. New try with pu8.

Comment 74

[Henrik Skupin [:whimboo][⌚️UTC+1]]

After playing around with the patch I noticed that I currently use mixed colors
(system / hardcoded). Which one should we use for netError.xhml? Kevin, whats
your decision? Any news from your side due to the missing icon?

Comment 75

[Ben Basson]

It would probably make sense to use whatever colours are picked in Tools >
Options > Content > Colours.

Comment 76

[William Price]

(In reply to comment #73)
> Created an attachment (id=185403)
> Patch with pu8

FYI, Henrik, the [p]...[/p] markup you inserted into the generic.longDesc entity
will NOT get parsed in the case that JS is disabled; it will still work in the
case where JS is enabled but the page receives an unknown error name string.

Steven, where does this stand w.r.t. the new icons, if any?
Barring further objections, this looks like it's ready to land and be tested.

Comment 77

[Adam Guthrie]

Since the checking for bug 216466 will make XUL error pages enabled my default,
I think this should be blocking aviary1.1 too. There is no sense in making XUL
error pages default if they're still going to look like crap.

Comment 78

[Jaime Mitchell (use [e-mail] for email)]

Requesting blocking 1.8b4 due to localisation impact.

Comment 79

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #76)
> FYI, Henrik, the [p]...[/p] markup you inserted into the generic.longDesc entity
> will NOT get parsed in the case that JS is disabled; it will still work in the
> case where JS is enabled but the page receives an unknown error name string.

William, I disabled javascript but I can't confirm this behavior. The entities
are parsed and inserted correctly. I can't understand why it's working only for
some people. So we have to wait for the mentioned fix of bug 292624 by bsmedberg
in bug 290219.

If javascript is enabled there is no error displayed for me. But I had the same
behavior before two days. I refreshed my whole tree and had to apply my patch
again. I only run the make command in the needed directories, which result in
the same parsing error. Try to run a build_all, perhaps this will remove it.
 
> Steven, where does this stand w.r.t. the new icons, if any?
> Barring further objections, this looks like it's ready to land and be tested.

So whats with the colors? Should I hardcode them for this time? I would update
the files and create a new patch. What shall I do?

Comment 80

[Henrik Skupin [:whimboo][⌚️UTC+1]]

William, we still need the missing chrome URIs for Camino. Can you or any other
person here offer them for me? Thanks.

Comment 81

[William Price]

(In reply to comment #80)
> William, we still need the missing chrome URIs for Camino. Can you or any other
> person here offer them for me? Thanks.

I'm not a Camino user/dev, Jasper is your best bet there regarding this bug. 
Assuming that the chrome://global/content/ path for netError.xhtml is valid in
Camino, I'd say stick it in there instead; of course, the Camino people should
be the ones to provide the real answer.

Re: javascript.  Ok, I was on a slightly older build and the new ones do work w/
JS disabled.  So my previous comment was completely wrong.  The only concern at
this point is for an unknown error code reported in the URI.  This shouldn't
happen, but if it were to occur we'd still have a blemish:

Scenario:
1) An unknown error type is passed to netError.xhtml
2) JS executes initPage()
3) aErrorDesc[err] is undefined (err isn't in the hash)
4) the parseFakeTags() routine never runs
5) existing &generic.longDesc; entity remains in place
6) [p]...[/p] tags in the entity aren't parsed & replaced with <p>...</p>
7) [p]...[/p] remains in visible text to user.

Suggestion:
Replace "&generic.longDesc;" with "<p>&generic.longDesc;</p>" and remove the
[p]...[/p] tags from the entity definition.  The entire contents of the
"errLongDesc" div (incl. <p> tags) will be replaced by the JS in one of the
other cases, which will have parsed [p] tags.

Comment 82

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v5

Yet no update for camino URIs. I'll wait for more input. But I changed the
netError.js. Now non-existent error codes are mapped to the generic one. I do
that because we can't replace "&generic.longDesc;" with
"<p>&generic.longDesc;</p>". Long descriptions with block level elements (e.g.
<ul>) inside the entity can't exist inside an inline element, like <p>. Anyway
it should work now. Further I also updated the colors to hardcoded values.

Comment 83

[Benjamin Smedberg]

Please do not worry about the chrome URIs for Camino, the security bug will
change the shipped location.

Comment 84

[Kevin Gerich]

Attachment: Screenshot of Winstripe warning icon and favicon

Images are located here:
http://kmgerich.com/archive/images/warning.png
http://kmgerich.com/archive/images/warning-small.png

Comment 85

[Caleb]

(In reply to comment #84)
> Created an attachment (id=187466) [edit]
> Screenshot of Winstripe warning icon and favicon
> 
> Images are located here:
> http://kmgerich.com/archive/images/warning.png
> http://kmgerich.com/archive/images/warning-small.png

Looking GREAT so far!

But what about a Try Again, Retry or a Refresh button? I think it could be
useful when people submit forms and such..

Comment 86

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #84)
> Images are located here:
> http://kmgerich.com/archive/images/warning.png
> http://kmgerich.com/archive/images/warning-small.png

Sweet. Thank you Kevin. How is the checkin handled for this two images? Should I
attach it to my patch? I don't think so. I need the chrome URI where the images
will be placed to update the xhtml and css.

(In reply to comment #85)
> But what about a Try Again, Retry or a Refresh button? I think it could be
> useful when people submit forms and such..

There is already a try again button, which will exactly do that. If users submit
e.g. a form the data isn't lost. With "location.reload();" the data will be send
again.

Comment 87

[Kevin Gerich]

The chrome URL should probably be chrome://global/skin/icons/Warning-large.png
and chrome://global/skin/icons/Warning-small.png for the favicon. The images
should be checked in when the patch goes in.

Comment 88

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v6 with addressed chrome locations

Ok, I've updated the patch to use the mentioned chrome locations. Seems that we
could start testing after 1.1a2?

Comment 89

[Max]

*** Bug 299558 has been marked as a duplicate of this bug. ***

Comment 90

[Max]

Can I also add that in italics at the bottom of the page, there should be
something like 'Mozilla Firefox', to indicate that this page is being served by
the browser and not by the server of the URL being requested?

Comment 91

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #90)
> Can I also add that in italics at the bottom of the page, there should be
> something like 'Mozilla Firefox', to indicate that this page is being served by
> the browser and not by the server of the URL being requested?

Not my decision. William, what do you think about?

Anyway, after a discussion on IRC I added the needed CSS files for SeaMonkey.
Because the bigger part of the patch is located inside core, I splitted the
patch into two parts.

Comment 92

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Seamonkey part of patch v7

This patch contains all changes for docshell, dom and themes.

Comment 93

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Firefox part of patch v7

This patch contains the changes for toolkit.

Comment 94

[Kadir Topal [:atopal]]

One of the most often asked questions in the german help forums is about the
netError-pop-up, and in almost all cases that's due to an update of Firefox
which caused a problem with the software-firewall. So could you please add
"firewall" to the list?

Comment 95

[Jaime Mitchell (use [e-mail] for email)]

(In reply to comment #94)
>almost all cases that's due to an update of Firefox
> which caused a problem with the software-firewall. So could you please add
> "firewall" to the list?

Although its impossible to actually detect for a specific firewall problem could
firewall not be mentioned in the following:

<!ENTITY dnsNotFound.longDesc 

Problems with your firewall, local network proxy, or DNS 

Comment 96

[Jim Ginn]

Henrik, is this ready for review?

Comment 97

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #96)
> Henrik, is this ready for review?

Perhaps if we don't want a browser name at the bottom of the page. Otherwise
I've to wait for an answer of Steven, if it's ok for him.

Here my current proposal:
http://hskupin.info/tmp/netError/v7/netError.jpg

Comment 98

[Steven Garrity [:sgarrity]]

Henrik: I'm fine with the browser name as you have it in this screenshot
(http://hskupin.info/tmp/netError/v7/netError.jpg). I can't comment on the
HTML/CSS used, since it's just a screenshot, but it looks fine there (nice and
subtle)

Comment 99

[Caleb]

Is the page's title hardcoded to "Page Load Error"? If so, then IMO the page's
title should change according to the error!

So if we have an "Address Not Found" error, the title should change accordingly.

Also when you get an Address Not Found, the first bullet suggests to check the
spelling AND the capitalization... does capitalization REALLY matter for this
type of error?

It should matter for a Page Not Found, but AFAIK DNS addresses are not case
sensetive.

Comment 100

[William Price]

(In reply to comment #99)
> Is the page's title hardcoded to "Page Load Error"? If so, then IMO the page's
> title should change according to the error!
> 
> So if we have an "Address Not Found" error, the title should change accordingly.

This could be done with some additional javascript; I'd argue against it for
now.  Consider multiple background tabs.  As-is, when one of these error pages
appears, the favicon will appear as a warning triangle and the page/tab title
will be "Page Load Error".  This will be consistent regardless of the cause; the
page itself will provide more detail.


> Also when you get an Address Not Found, the first bullet suggests to check the
> spelling AND the capitalization... does capitalization REALLY matter for this
> type of error?
> 
> It should matter for a Page Not Found, but AFAIK DNS addresses are not case
> sensetive.

Whoops.  That's a cut/paste hangover.  I don't mind gently reminding the user
that URLs are case sensitive (the path part, anyway)... but as it's not a
possible cause of this error, it might be best to remove the phrase.

As for everything else, it looks wonderful.  Let's get these last nitpicks
resolved and get it out for review.  Thanks for everyone's help!

Comment 101

[Jeff Walden (gone starting June 8)]

(In reply to comment #97)
> Here my current proposal:
> http://hskupin.info/tmp/netError/v7/netError.jpg

It was mentioned on IRC that the phrasing of the bullets is wonky; the problem
is that the bullet texts don't structurally parallel each other very well.  My
attempts at working with the text and leaving each bullet in statement form
failed, so my suggestion converts each bullet to a question.  I actually think
this may be more readable and usable, as I expect the reader will actually
convert the suggestions to questions internally in order to answer them. 
Anyway, the bullets:

-Is the site address spelled correctly?
-Does the site still exist?
-Is your Internet connection working properly?

This also eliminates some of the opaque terminology used in the screenshot text.
 The "working Internet connection" text serves exactly the same purpose as the
proposed text, and it's more understandable.  If the user's having problems with
any one of the listed problem spots, he'll either already know to check these
spots or will just be confused by the list.

I'm also not sure why you need " (URL)" in the text.  More people will be
familiar with "address" than with "URL", and the number of people who know the
latter but not the former will be pretty small.

Comment 102

[Caleb]

(In reply to comment #100)
> (In reply to comment #99)
> > Is the page's title hardcoded to "Page Load Error"? If so, then IMO the page's
> > title should change according to the error!
> > 
> > So if we have an "Address Not Found" error, the title should change accordingly.
> 
> This could be done with some additional javascript; I'd argue against it for
> now.  Consider multiple background tabs.  As-is, when one of these error pages
> appears, the favicon will appear as a warning triangle and the page/tab title
> will be "Page Load Error".  This will be consistent regardless of the cause; the
> page itself will provide more detail.

This probably depends on the taste, although I'd like to hear other peoples'
opinions about having the title reflect the error.
As you said, the favicon will be there to tell you that there is an error, and
IMO the title should say what error.

But don't take my word for it, I'd like other people to weigh in.

But overall, this is very nice :)

Although I think that netError.dtd needs some polishing. Maybe Jeff Walden or
others can proof-read it and post comments ?

Comment 103

[Jo Hermans]

(In reply to comment #101)
> I'm also not sure why you need " (URL)" in the text.  More people will be
> familiar with "address" than with "URL", and the number of people who know the
> latter but not the former will be pretty small.

And it's not even an URL, it's just a dns-name :-) Maybe we should just remove
it before some PITA files a bug for it.

Comment 104

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, following updates were made:

* new DTD from William
* using entity &brandFullName; (won't work for SM until bug 299891 is fixed)
* updated themes for image locations (small and large warning icon)

Until we aren't ready for review I still won't add here more and more
attachments. You can find all neccessary files (patches and screenshots) here:

http://hskupin.info/tmp/netError/v8/

Please proof-read especially the DTD, so we can land it soon. A patch for bug
299891 I will create later today.

Comment 105

[Caleb]

Does attachment 187466 [details] use the same font as the error pages currently do (in v8) ?

The screenshot http://hskupin.info/tmp/netError/v8/netError-fx.jpg kinda looks
worse font-wise.

Comment 106

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #105)
> Does attachment 187466 [details] [edit] use the same font as the error pages currently
do (in v8) ?

Not currently. :/ But we should set the system font selected by the user. I
change the font-family to message-box later. Another question we already had,
but no answer was given, is which background and text colors to use. Do we want
to have a simple webpage or should it be perfectly integrated into the browser
(with any system color style)?

> 
> The screenshot http://hskupin.info/tmp/netError/v8/netError-fx.jpg kinda looks
> worse font-wise.

Comment 107

[William Price]

(In reply to comment #106)
> Another question we already had, but no answer was given, is which
> which background and text colors to use. Do we want to have a simple
> webpage or should it be perfectly integrated into the browser
> (with any system color style)?

I'll answer with a question. :)  How do the default themes do it?

The page should look consistent with the default themes shipped by Mozilla.  For
example, if Gnomestripe changes significantly when system color settings are
altered, the error page should still look like it "fits" with Gnomestripe. 
That's all we're really worried about because all the colors can be overridden
by individual theme authors if/when they choose.

Comment 108

[Kevin Gerich]

We can take a cue from the about:plugins css

body {
  background-color: -moz-Field;
  color: -moz-FieldText;
  font: message-box;
}

Comment 109

[Caleb]

"www.not.existant could not be found. Please check the name and try again"

Could this actually motivate/confuse some newbies into editing the URL in the
addressbar and then hitting the Try Again button in the error page? (which
basically reloads the broken page all over again).

Comment 110

[William Price]

(In reply to comment #109)
> "www.not.existant could not be found. Please check the name and try again"
> 
> Could this actually motivate/confuse some newbies into editing the URL in the
> addressbar and then hitting the Try Again button in the error page? (which
> basically reloads the broken page all over again).

Three options:

1) Change the wording of the "Try Again" button.
   I dislike this option because button verbage should be short and to the
   point.  At the same time, this button is doing exactly that: trying the
   same thing again.

2) Change the wording of the inserted message.
   This would be preferable, I think, and would involve changing the
   wording in appstrings.properties as a part of this bug.

3) Consider it a nitpick outside the scope of this bug and let there be
   separate discussion about it.  Users have come to expect the need to
   press enter or click the "Go" button after making changes to the
   address bar.  No browser experience would lead them to expect otherwise.

I agree the current wording in appstrings, combined with the error pages,
might give that impression to a novice -- but IMHO file it as a separate
bug.  That has a scope outside these pages (affects dialog boxes, too).
And this bug is covering a lot of changes as it is.

Comment 111

[Ben Basson]

Why not simply remove "and try again" from the string? 

It's safe to assume that users will recognise that further action is needed
before their modified address will be loaded which makes the instruction to do
so redundant.

Comment 112

[Caleb]

(In reply to comment #110)
> Three options:
> 
> 1) Change the wording of the "Try Again" button.
>    I dislike this option because button verbage should be short and to the
>    point.  At the same time, this button is doing exactly that: trying the
>    same thing again.

This option does not sound reasonable to me either.

> 2) Change the wording of the inserted message.
>    This would be preferable, I think, and would involve changing the
>    wording in appstrings.properties as a part of this bug.

This option sounds decent to me. But the result has to be something simple, and
not a long explanation.

> 3) Consider it a nitpick outside the scope of this bug and let there be
>    separate discussion about it.  Users have come to expect the need to
>    press enter or click the "Go" button after making changes to the
>    address bar.  No browser experience would lead them to expect otherwise.

I agree, but this page may lead to confusion since it says to "try again" and
there is a "Try Again" button at the bottom.

> I agree the current wording in appstrings, combined with the error pages,
> might give that impression to a novice -- but IMHO file it as a separate
> bug.  That has a scope outside these pages (affects dialog boxes, too).
> And this bug is covering a lot of changes as it is.

Well, I don't think that this particular problem affects dialogs. Even though
that they also say "Try again". The problem on this page is that there is a
button that tries again, yet it might give the wrong impression as to what we
are trying again.

If this isn't covered here then I can open a new bug, but it'll have to block
1.8b4 because that's where the l10n freeze happens, and it will be difficult to
make changes to the strings afterwards.
That's why I think it would be nice to have a final dtd here that won't need to
be touched. In order to do this, we need some more help from people who have
experience in this field (like the ones responsible for updating the general
help system).

Comment 113

[William Price]

(In reply to comment #112)
> Well, I don't think that this particular problem affects dialogs. Even though
> that they also say "Try again". The problem on this page is that there is a
> button that tries again, yet it might give the wrong impression as to what we
> are trying again.

The point was that the string you're complaining about comes from a different
file (not netError.dtd); specifically, it comes from appstrings.properties.  The
strings in this file define what's displayed in the error dialogs, and also
happen to be included on this page through a bit of URI/Javascript magic.

Because, however trivial, it impacts the dialog boxes I say open another bug. 
That file could use some general rewording anyway.

Comment 114

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Updated patch v9:

* Exchanged hardcoded colors with theme colors
* Using user selected font (message-box)
* retryButton is now a xul button
* Modified CSS for all themes (tested with Fx: winstripe, SM: classic, modern)

Patches and screenshots:
http://hskupin.info/tmp/netError/v9/

Comment 115

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

or the try again button could be removed, having users press "Reload" instead...

Comment 116

[Samuel Sidler (old account; do not CC)]

(In reply to comment #115)
> or the try again button could be removed, having users press "Reload" instead...

This is definitely not a good option if you're discussing confusing users.
"Reload" what? Reload the error page? Reload the attempted location? "Reload" is
*way* too generic and not a good idea.

Henrik, version 9 looks great. I think it's time to check in these changes and
file new bugs to refine and enhance the error pages. We've done more than enough
refining thus far and, as was mentioned in comment 112, as many changes as
possible need to be in before the freeze for localization purposes.

Can we *please* come to a consensus on version 9 and file new bugs for enhancing
the page(s)?

Comment 117

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

(In reply to comment #116)
> This is definitely not a good option if you're discussing confusing users.
> "Reload" what? Reload the error page? Reload the attempted location? "Reload" is
> *way* too generic and not a good idea.

I meant the existing reload button. Note that MSIE does not have a special "Try
again" button either (so I assume that this doesn't confuse users too much)

Comment 118

[Samuel Sidler (old account; do not CC)]

(In reply to comment #117)
> I meant the existing reload button. Note that MSIE does not have a special "Try
> again" button either (so I assume that this doesn't confuse users too much)
> 

Ah... heh... I still don't think a "Try Again" button would confuse users. Any
that do make the mistake will probably realize on the first reload (try again)
what the button does and does not do.

Let's get these changes in, file a new bug about removing the "Try Again" button
and see what people feel once the new error pages are being tested. Getting this
out now allows more people to test the pages and file bugs to improve/refine them.

Comment 119

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v9 (core)

Ok, let's start with changes I made for docshell and dom...

Comment 120

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v9 (SeaMonkey)

Theme update for classic and modern (without icons).

Comment 121

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: patch v9 (Firefox)

Theme updates for toolkit (without icons).

Comment 122

[[e-mail]]

Comment on attachment 188639 [details] [diff] [review]
patch v9 (core)

>+    <link rel="icon" href="chrome://global/skin/icons/warning-small.png" type="image/png" />
Does this actually work? Certainly links in normal web pages can't link to
chrome:

>+	netInterrupt: "&netInterrupt.longDesc;",
Spaces, not tabs, please.

>+    <script type="application/x-javascript" src="chrome://global/content/netError.js"></script>
Why not <script .../> ? Or does your editor generate html-compatible xhtml?

>+      <!-- NOTE: Camino will need to change the image URI to an alternate chrome location. -->
>+      <img id="errorIcon" src="chrome://global/skin/icons/warning-large.png" />
Well this simply shouldn't be hardcoded here. Either set the background on an
div (in CSS!) or use a XUL image (and set it in CSS!)

>+      <xul:button xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul" id="errorTryAgain" label="&retry.label;" onclick="retryThis();" />
Wrong, XUL buttons use oncommand. What was wrong with the HTML button?

>+  err  = aErrorDesc[err] ? err : "generic";
if (!(err in aErrorDesc))
  err = "generic";
(also you had two spaces between err and =)

Comment 123

[William Price]

Comment on attachment 181822 [details] [diff] [review]
3rd try: Patch of existing XHTML, JS, DTD files for review

clearing old attachment flags; sorry for bugspam

Comment 124

[[e-mail]]

Comment on attachment 188640 [details] [diff] [review]
patch v9 (SeaMonkey)

>+  border-bottom: 1px solid LightGray;
You should be using a system colour here.

>+#errorTryAgain {
>+  display: block;
Why?

Comment 125

[[e-mail]]

Comment on attachment 188639 [details] [diff] [review]
patch v9 (core)

>+    <link rel="stylesheet" href="chrome://global/skin/netError.css" type="text/css" media="all" />
Shouldn't this use an <?xml-stylesheet?> processing instruction?

>+    <link rel="icon" href="chrome://global/skin/icons/warning-small.png" type="image/png" />
Hmm... so I tested this offline, and it promptly changed my bookmark's icon to
be the warning icon :-(

Comment 126

[[e-mail]]

Comment on attachment 188640 [details] [diff] [review]
patch v9 (SeaMonkey)

Further to comment 124:

>+  font: 100% message-box;
Why the 100%?

>+#errorTryAgain {
>+  display: block;
>+  margin: 2.5em 0 0 0;
>+  font-size: 85%;
>+}
If this is a XUL button then it should use the XUL button font size.

Comment 127

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Updated patch v10 with major changes:

* Replaced netError.dtd with netError.properties to use a StringBundle (this
will remove the js crap of parsing entities)
* Moved content of netError.js into netError.xhtml due to bug 292624
* After IRC talk the favicon is removed for the error page (see comment 125 and
for design reasons)
* The icon is now defined as background-image within the CSS
* Tweaking of CSS (font sizes and colors)
* Modified getErrorCode to return "generic" if code is not given (do we need a
check if netError.properties contains the needed strings?)
* All other addressed changes by Neil

Patches and screenshots:
http://hskupin.info/tmp/netError/v10/

Neil, when the patches are ok for you now, I'll attach them here.

Comment 128

[Dave Townsend [:mossop]]

Should the text: 'e.g. "ww.mozilla.com" instead of "www.mozilla.org"' not be
'e.g. "ww.mozilla.org" instead of "www.mozilla.org"'. Either that or should the
org and com be highlighted as well as potential typing errors?

Comment 129

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #128)
> Should the text: 'e.g. "ww.mozilla.com" instead of "www.mozilla.org"' not be
> 'e.g. "ww.mozilla.org" instead of "www.mozilla.org"'. Either that or should the
> org and com be highlighted as well as potential typing errors?

Sure. I updated patch v10.

Comment 130

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

+      <xul:button
xmlns:xul="http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul"
id="errorTryAgain" oncommand="retryThis();" />

please wrap lines at 80 characters

Comment 131

[Steve England [:stevee]]

From http://hskupin.info/tmp/netError/v10/

Currently the areas covered when a website can't be found are :
* mistake typing
* expired domain
* local network connection settings
* firewall

I question the usefulness of mentioning "Has the domain's registration expired?"
as most users won't know one way or the other, and will not know how to check
such a thing.

I also wonder aloud if there should be some mention of high network traffic
perhaps being responsible (packet loss) or ISP-related problems, and/or some
mention that it might be the fault of the server the user is trying to contact
(high load, not online, experiencing problems, down for maintanance, etc).

Comment 132

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I had to revert the changes I made for StringBundle. Due to netError.xhtml won't
have chrome privileges we can't use XPCOM. For now netError.dtd is used again to
hold the error descriptions:

http://hskupin.info/tmp/netError/v11/

Comment 133

[Steven Garrity [:sgarrity]]

I'm not sure I quite understand what was happening with the favicon. Comment
#125 said that it "changed my bookmark's icon to be the warning icon". Which
bookmark icon was that?

Perhaps that is another bug? Regardless, I do think it is important to have the
favicon indicate an error. This helps clarify that it is a browser message, not
an ordinary website, and it will let you know that pages loading in tabs in the
 background have failed before you view them individually.

Comment 134

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v12

Ok, I added the favicon again. For remaining problems we should file a new bug.
I hope we'll finish it soon to get the l10n train. Neil, can you take a look at
it? If you can't give a review who is responsible for? The other patches for SM
and Fx will follow later today.

Comment 135

[[e-mail]]

(In reply to comment #134)
>If you can't give a review who is responsible for?
I'd guess either Christian Biesinger and/or Boris Zbarsky.
That said, the patches look good to me, except
a) attachment 181740 [details] seems to work well with the Modern theme
b) the Classic theme might work best with the existing alert-exclam image

Comment 136

[Jeff Walden [:Waldo]]

(In reply to comment #135)
> I'd guess either Christian Biesinger and/or Boris Zbarsky.

Note that both (I'm not absolutely sure on the former) disavow reviewing UI
changes, so they may not be the best choices here as that's exactly what's being
changed.

Comment 137

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v12 (Firefox)

Benjamin, if the core part doesn't change for now, this patch should be the
final version provided that it's fine for you.

Comment 138

[Steve England [:stevee]]

Re: http://hskupin.info/tmp/netError/v11/Firefox-winstripe.jpg

Should the
(e.g. "ww.mozilla.com" instead of www.mozilla.org")
be
(e.g. "ww.mozilla.org" instead of www.mozilla.org")
                  ^^^

Comment 139

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v12 (Firefox)

Sorry, last patch was the wrong file.

Comment 140

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #138)
> Re: http://hskupin.info/tmp/netError/v11/Firefox-winstripe.jpg
> 
> Should the
> (e.g. "ww.mozilla.com" instead of www.mozilla.org")
> be
> (e.g. "ww.mozilla.org" instead of www.mozilla.org")
                    ^^^

I already fixed that. You can see it within the DTD. The screenshots are a bit
too old.

Comment 141

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v12 (SeaMonkey)

Neil, I looked at 'alert-exclam.gif' but I'm personally not happy with this
icon. I know that you don't like the favicon but when we are using
'alert-exclam.gif', we'll have two different icons within that page. And this
doesn't look well. If you don't agree, we should get a new SeaMonkey styled
favicon which is similar to 'alert-exclam.gif'?

Comment 142

[Boris Zbarsky [:bzbarsky]]

I'm not going to be able to get to this for a while (weeks).  Maybe try biesi?

Comment 143

[Benjamin Smedberg]

Comment on attachment 188965 [details] [diff] [review]
Patch v12 (Firefox)

Mconnor should do UI reviews, not me.

Comment 144

[Jaime Mitchell (use [e-mail] for email)]

*** Bug 300843 has been marked as a duplicate of this bug. ***

Comment 145

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch v12 (Core) with changes on bug 292624

Cause netError.js was removed with the checkin on bug 292624, I had to move the
code into netError.xhtml.

Comment 146

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Running this updated file shows me a security error for the favicon:

Security Error: Content at
about:neterror?e=protocolNotFound&u=moz-neterror%3Apage&c=&d=moz-neterror%20is%20not%20a%20registered%20protocol.
may not load or link to chrome://global/skin/icons/warning-small.png.

It seems that we have no longer access to that file. I think we should remove it
after all? But why the other icon is loaded without any security error?

Comment 147

[Caleb]

Bug 299480 was fixed, should changes from that bug be ported to this one ?

Comment 148

[Chris Thomas (CTho) [formerly [e-mail] [e-mail]]]

Should I mark bug 229737 as a duplicate of this bug, or have it depend on this bug?

Comment 149

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

Comment on attachment 189391 [details] [diff] [review]
Patch v12 (Core) with changes on bug 292624

sorry for taking so long :-/

+    <link rel="icon" href="chrome://global/skin/icons/warning-small.png"
type="image/png" />

that's not so nice, but I guess it's unavoidable :-/

fwiw, this patch doesn't apply to trunk... which made reviewing this really
painful...

      var aErrorTitle = {
      var aErrorDesc = {

don't use a leading a for non-arguments

+	 proxyConnectFailure: "&proxyConnectFailure.title;"
+      };
+			 

please remove the trailing whitespace here

why use two <script> blocks?


	return instr.replace(/\[/g, '<').replace(/\]/g, '>');

OK, so what is the reason why you can't just use <p> etc in the DTD?


	if (title && aErrorTitle[err])
	  title.innerHTML = aErrorTitle[err];

please don't use nonstandard attributes when you can avoid it. (i.e. use
createTextNode and appendChild instead of innerHTML)

Why the check for aErrorTitle[err]?

	  sd.innerHTML = getDescription();

looks like this, too, needs not use innerHTML.

I'm not sure that I'd check that getElementById actually returns something...
I'd probably use an assertion, but there are none in JS :-/

I guess leaving it in is ok.

	if (desc && aErrorDesc[err])
	  desc.innerHTML = parseFakeTags(aErrorDesc[err]);

why the aErrorDesc[desc] check? You made sure above that there is desc in
aErrorDesc. the optimization not to call this if empty doesn't seem worth this.

 <!-- Link the processing script -->
    <script type="application/x-javascript"
src="chrome://global/content/netError.js" />

there's no such file... plus, the JS code is already in the file.

      <!-- Error Title -->
      <div id="errorTitle">
	<h1 id="errorTitleText" />
      </div>

hmm, is the <div> around the <h1> needed? What does it give you?

same for:
	<div id="errorShortDesc">


I didn't look at the text changes yet, will do that in the next round ;)

Comment 150

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

Comment on attachment 188916 [details] [diff] [review]
Patch v12

I assume this review request is obsolete now.

Comment 151

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

(In reply to comment #149)
> 	return instr.replace(/\[/g, '<').replace(/\]/g, '>');

Oops, that was unfinished. I meant:

If you have to use that [p] replacing, why not:
  instr.replace(/\[([^\]]+)\]/, "<$1>");

hmm... guess that's less readable... although it expresses the idea more clearly


It sucks that you have to use innerHTML.

Comment 152

[Henrik Skupin [:whimboo][⌚️UTC+1]]

*** Bug 229737 has been marked as a duplicate of this bug. ***

Comment 153

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

Comment on attachment 188916 [details] [diff] [review]
Patch v12

ok, review of the dtd changes:

<!ENTITY dnsNotFound.longDesc "[p]The browser could not find the host server

"host server"? shouldn't that be just "host"?

[ul][li]Is the computer connected to an active network?[/li]

I guess. but really, only offline mode triggers that...

[p]The requested site did not respond to a connection request and the browser
has stopped waiting for a reply.

the second half feels somewhat redundant with the first half to me...

so the browser cannot properly connect to the site.

it can't connect improperly either..

Comment 154

[Caleb]

I filed bug 301208 about any possible improvements that could be made to the new
netError.dtd so this bug could stay focused on the bulk part of the error pages.

Comment 155

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch with rtl and corrected parts

Comment 156

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Patches for theme issues will follow tomorrow. Screenshots can already be found
here: http://hskupin.info/tmp/netError/v13/

Comment 157

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch with rtl (toolkit)

Benjamin, all netError.css are identical but we can't put it in
docshell/resources due to SeaMonkey themes uses a different netError.css. I
would still prefer to put it in every single theme. If we have to make changes
for a theme, it would make our life much easier.

Comment 158

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patch with rtl (seamonkey)

Comment 159

[[e-mail]]

(In reply to comment #149)
>>title.innerHTML = aErrorTitle[err];
>use createTextNode and appendChild instead of innerHTML
Better still, use title.textContent

Comment 160

[[e-mail]]

Comment on attachment 189830 [details] [diff] [review]
Patch with rtl (seamonkey)

r=me conditional on the images actually matching the theme (or you stealing
alert-exclam.gif instead).

Comment 161

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

Comment on attachment 189710 [details] [diff] [review]
Patch with rtl and corrected parts

oh... that's why you had two script blocks... urg. I think I prefer having two
of them, then.

with that and neil's comment, r=biesi

Comment 162

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Core patch with mentioned review nits

All mentioned parts are done for the core part. So carrying over r= from
attachment 189826 [details] [diff] [review].

Comment 163

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: SM patch with existing alert-exclam icons

Classic and modern uses the alert-exclaim.gif now. Carrying over r= from
attachment 189830 [details] [diff] [review].

Comment 164

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Loading of the favicon will leave a security error within the javascript console:

Security Error: Content at
about:neterror?e=dnsNotFound&u=http%3A//www.not.existent/&c=UTF-8&d=www.not.existent%20could%20not%20be%20found.%20Please%20check%20the%20name%20and%20try%20again.
may not load or link to chrome://global/skin/icons/warning-small.png.

I think we should remove them until bug 301119 will be fixed?

Comment 165

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

yeah, I forgot to mention that in my review comments, please remove the favicon
until that bug is fixed.

Comment 166

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Core patch without favicon

Comment 167

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: SM patch without favicon

Comment 168

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Toolkit patch without favicon

Comment 169

[Darin Fisher]

Comment on attachment 189995 [details] [diff] [review]
SM patch without favicon

I'm not a CSS wizard, so I did not review the CSS file very thoroughly.  It
looks fine to me, sr=darin

Comment 170

[Peter Van der Beken [:peterv]]

Comment on attachment 189994 [details] [diff] [review]
Core patch without favicon

>Index: docshell/resources/content/netError.xhtml
>===================================================================

>+<!ENTITY redirectLoop.longDesc "[p]The browser has stopped trying to retrieve the requested item. The site is redirecting the request in a way that will never complete.[/p][ul][li]Have you disabled or blocked cookies required by this site?[/li][li][em]NOTE[/em]: If accepting the site's cookies does not resolve the problem, it is likely a server configuration issue and not your computer.[/li][/ul]">

That sounds weird: 'it is a ... issue and not your computer', what is not their
computer?
With that, sr=peterv.

Comment 171

[Caleb]

(In reply to comment #170)
> (From update of attachment 189994 [details] [diff] [review] [edit])
> >Index: docshell/resources/content/netError.xhtml
> >===================================================================
> 
> >+<!ENTITY redirectLoop.longDesc "[p]The browser has stopped trying to
retrieve the requested item. The site is redirecting the request in a way that
will never complete.[/p][ul][li]Have you disabled or blocked cookies required by
this site?[/li][li][em]NOTE[/em]: If accepting the site's cookies does not
resolve the problem, it is likely a server configuration issue and not your
computer.[/li][/ul]">
> 
> That sounds weird: 'it is a ... issue and not your computer', what is not their
> computer?
> With that, sr=peterv.

DTD changes will be handled in bug 301208 once the new error pages land

Comment 172

[Mike Pinkerton (not reading bugmail)]

has anyone tried this in Camino (or any other embedding app) to make sure that
it's not dependent on parts of the chrome that aren't built for embedding apps?

Comment 173

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #172)
> has anyone tried this in Camino (or any other embedding app) to make sure that
> it's not dependent on parts of the chrome that aren't built for embedding apps?

Ok, some tests were done and we have a problem! For Camino &brandFullName; can't
be resolved because the referenced &realBrandDTD; within the
chrome://global/locale/brand.dtd does not exist.

We have two options for now. The simplest way is to remove the whole brand stuff
at this time. We could open a new bug for that issue. Otherwise we could find a
way to add the chrome://branding part for Camino. I would prefer the first
option to get in the patches ASAP.

Comment 174

[Samuel Sidler (old account; do not CC)]

(In reply to comment #173)
> We have two options for now. The simplest way is to remove the whole brand stuff
> at this time. We could open a new bug for that issue. Otherwise we could find a
> way to add the chrome://branding part for Camino. I would prefer the first
> option to get in the patches ASAP.

I discussed this with pink in IRC and he agreed that the best idea is to pull
the branding stuff and add it back later. There will be problems because this
has to be done in embedding since it's not specific to Camino. There'd have to
be makefile changes to copy the file somewhere. So, this is a much bigger issue
and to land it quickly, remove the brand stuff.

a=pink (per IRC)

Comment 175

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Core patch with temporarily removed brand stuff

Since there are good reasons to remove the brand stuff for now, this patch
should hopefully work with each embedded app. I only changed the
netError.xhtml. In that case we don't have to update the CSS files again, when
branding is available.

I will file a new bug for handling the brand.dtd with embedded apps.

Carrying over r= and sr= but I can't set approval1.8b4 due to limited
restrictions. See comment 174 where it's already given.

Comment 176

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Filed bug 302309 for the remaining brand issue.

Comment 177

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

Last 3 patches on this bug checked in; I'm assuming brand name stuff will be
done in another bug, as will favicons.

Comment 178

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Works fine with Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050728
Firefox/1.0+

=> FIXED.

Comment 179

[Ian Macfarlane]

Rather than "Verify the address is correct and contains no typographical
errors", wouldn't it be a bit less tech-glish to say "Verify the address is
correct and has been spelt correctly"?

Comment 180

[Adam Guthrie]

(In reply to comment #179)
> Rather than "Verify the address is correct and contains no typographical
> errors", wouldn't it be a bit less tech-glish to say "Verify the address is
> correct and has been spelt correctly"?

Bug 301208 was made to address issues like this.

Comment 181

[Nickolay_Ponomarev]

Sorry for posting in this bug, but the icon in the new error pages makes them
look quite bad in narrow frames, such as bloglines' left frame with blogs. It
would be better if the image was "float: left" in such cases.

Comment 182

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #181)
> Sorry for posting in this bug, but the icon in the new error pages makes them
> look quite bad in narrow frames, such as bloglines' left frame with blogs. It
> would be better if the image was "float: left" in such cases.

See comment 122. It's included within the CSS as a background image now. So it
can't be formatted with float.

Comment 183

[Bryan]

Adding dependent bug: 304087 per [e-mail] on IRC (DEV)

Comment 184

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 304101 has been marked as a duplicate of this bug. ***

Comment 185

[zug_treno]

Bug 318446 is also about a better looking netError.xhtml, it might be related to this bug?