netError.xhtml should (if technically possible) display platform specific favicons: XP: http://mxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/icons/warning-16.png Vista: http://mxr.mozilla.org/seamonkey/source/toolkit/themes/winstripe/global/icons/warning-16-aero.png OS X: http://mxr.mozilla.org/seamonkey/source/toolkit/themes/pinstripe/global/icons/warning-16.png Linux: http://mxr.mozilla.org/seamonkey/source/toolkit/themes/gnomestripe/global/icons/notfound.png

I'm not sure this is easy to do... I ran into this in with the favicon for the about:robots page -- which is basically a copy of netError.xhtml. It seems the way our security policy works prevents just linking to packaged icon url (eg, chrome://whatever). This was also noted as a problem in bug 229737, which added favicons to the error page. Dunno if a clever workaround of some sort might be possible now. See also, bug 314416.

Am I missing something here? This patch seems to work beautifully (on Vista at least).

Does it work even when the file is packaged in chrome?

Yes. I tested this by entering a non-existing domain name in the location bar (like sgyadjfyskduykduvcgvyf.com) in order to trigger the address not found error.

Not sure if Mano is the right person, maybe you'll need a docshell peer

Comment on attachment 316487 [details] [diff] [review] Patch (v1) [Checkin: Comment 11] Not sure if this needs sr...

Comment on attachment 316487 [details] [diff] [review] Patch (v1) [Checkin: Comment 11] Does this need UI review? Probably not, r+sr=jst

If that patch really works we should be able to undo the patch from bug 301119 right? Would be nice to understand what's changed... Did the fix for bug 342485 make the fix for bug 301119 unnecessary?

Comment on attachment 316487 [details] [diff] [review] Patch (v1) [Checkin: Comment 11] Super-low risk patch to change a favicon. Shouldn't have any negative side effects

Comment on attachment 316487 [details] [diff] [review] Patch (v1) [Checkin: Comment 11] a1.9=beltzner

mozilla/docshell/resources/content/netError.xhtml 1.27 Ehsan, can you file a followup to investigate comment 8?

(In reply to comment #11) > Ehsan, can you file a followup to investigate comment 8? Done. -> bug 430433.

It would bave been nice to at least let us know that you busted us...

This could use some tests. Then they could also be used to check whether bug 430433 makes sense. The tests should test both error pages and normal pages linking to chrome:// favicons.

I think that, unless the regression (bug 430792) is definitely going to be fixed by RC1, this should be backed out [1]. If the choice is releasing with a platform specific icon that could appear unexpectedly in history and bookmarks - sometimes permanently [2] - or a generic icon that knows its place, we should go with the latter. [1] It can always be reapplied when regression is fixed - ideally made part of same patch. [2] If a location provides a site icon it should be reset on next visit but if not, and for some non-default values for site_icons and favicons preferences, the error icon will stick. I suspect, but haven't checked, that redirects and RSS feeds could also be affected.

I have a patch ready for review on that bug, but it's been sitting there for a while. I'm going to see if I can ask someone else for a more quick review. In the mean time, it would be good if you can include this comment in that bug as well.