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.

Above post says it all.

Comment on attachment 172660 [details] v1.0 Sorry, I'm a bad choice for UI changes like this.

Dup of bug 170216?

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

Probably shouldn't use pixels for accessibility reasons. div.et_visible {font-size: medium;}

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

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

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

yeah, it changed today, 5 hours ago ;)

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 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 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.

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.

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.

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.

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.

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.

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

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.

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.

(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?

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.

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.

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.

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

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.

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.

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

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. :)

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.

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.)

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)

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.

(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.

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

Steven: Opera 8 error page.

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.

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

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

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

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?

(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?

(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.

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.

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?

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.

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.

(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.

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.

(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. :-)

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

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

(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.

(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.

(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.

(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

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.

(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. :)

>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 &quot; 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? :-)

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.

(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

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.

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.

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.

(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.

(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?

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

(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.

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?

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

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

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?

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

(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.

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.

Requesting blocking 1.8b4 due to localisation impact.

(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?

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

(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.

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.

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

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

(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..

(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.

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.

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

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

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?

(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.

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

This patch contains the changes for toolkit.

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?

(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

Henrik, is this ready for review?

(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

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)

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.

(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!

(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.

(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 ?

(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.

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.

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.

(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.

(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.

We can take a cue from the about:plugins css body { background-color: -moz-Field; color: -moz-FieldText; font: message-box; }

"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).

(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.

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.

(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).

(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.

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/

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

(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)?

(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)

(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.

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

Theme update for classic and modern (without icons).

Theme updates for toolkit (without icons).

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 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 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 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 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.

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.

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?

(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.

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

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).

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/

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.

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.

(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

(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.

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

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") ^^^

Sorry, last patch was the wrong file.

(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.

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'?

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

Comment on attachment 188965 [details] [diff] [review] Patch v12 (Firefox) Mconnor should do UI reviews, not me.

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

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

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?

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

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

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 on attachment 188916 [details] [diff] [review] Patch v12 I assume this review request is obsolete now.

(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.

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

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..

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.

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

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.

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

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 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

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

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

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?

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

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 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.

(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

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?

(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.

(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)

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.

Filed bug 302309 for the remaining brand issue.

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