277658 - Make error pages URL a pref

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

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

currently, the error page url is hardcoded in nsDocShell.cpp:
2847     errorPageUrl.AssignLiteral("chrome://global/content/netError.xhtml?e=");

it would be nice if embeddors could specify a different URL - I was specifically
asked by the epiphany people to offer such a way.

I suggest using a pref for this. this would also allow overriding them by users,
for better or worse. other possibilities would include a category entry, or a
per-docshell setting (nsIDocShell::setErrorPageURL), but I think I like the pref
best.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Pref sounds good to me.

Comment 2

[Ian Tommins ('thelem')]

Be careful about making this too easy - we don't want to allow malware to
forward people to an advertising-filled page.

Also, the error page must be local, otherwise it could become an endless loop.

Comment 3

[Vidar Haarr (not reading bugmail)]

Attachment: version 1.0

First version.

I use CharPref, because it was (not clearly) decided in #devs that a
ComplexPref was not needed, as long as embeddors can encode any intl chars
they'd want in there ..

I don't think we need to take any special considerations on this as opposed to
other prefs that have URIs in them, regarding security. If someone has access
to the prefs, I think we'd have bigger problems in store than people getting
sent on roundabouts or to ad sites.

biesi: Can you review this?

Comment 4

[Vidar Haarr (not reading bugmail)]

Comment on attachment 173019 [details] [diff] [review]
version 1.0

timeless kindly let me know that if (!prefBranch), I'm dereferencing random
memory in
+    errorPageUrl.AssignASCII(errorPagePrefLocation);
And suggested I use nsXPIDLCString instead of char for errorPagePrefLocation.

He also asked if I could fixup the PR_FREEIF's to us nsMemory::Free, since
that's what allocs them.

I'll patch it tonight.

Comment 5

[Vidar Haarr (not reading bugmail)]

Attachment: version 1.2

Updated as per comment #4

Comment 6

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

Comment on attachment 173119 [details] [diff] [review]
version 1.2

thanks.

Comment 7

[Boris Zbarsky [:bzbarsky]]

So... GetCharPref() only does ascii, no?  How exactly are non-ascii URIs
supposed to be done?

Comment 8

[Vidar Haarr (not reading bugmail)]

Yes, it only does ASCII .. We figured that it wasn't that important, and that
people could encode intl chars if they needed them (although I'm not entirely
sure what scheme you would use?).

bz: If you're more comfortable with using GetCompexPref with an
nsISupportsString, for example, I could do that ?

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 173119 [details] [diff] [review]
version 1.2

Hmm... Right.  This is a URI, so they can URI-escape the UTF8 (and should).

Comment 10

[Vidar Haarr (not reading bugmail)]

Checked in by silver%warwickcompsoc.co.uk.
Thanks! Marking FIXED.