Closed Bug 768842 Opened 11 years ago Closed 11 years ago

[BrowserAPI] Inform embedder when we show a Gecko error page


(Core :: DOM: Core & HTML, defect)

Not set





(Reporter: justin.lebar+bug, Unassigned)




(1 file)

The embedder should be informed when we show an error page (http error, certificate error, etc.).

This is particularly important for apps.  If the system app tries to load an app and gets a 404, we should do something more meaningful than display a 404 page to the user.

Things like cert errors are kind of complex; I don't think we need to handle the full flow for allowing cert exceptions.  It should be good enough for now if we just say "cert error, can't load page".
Blocks: browser-api
Ha, we actually *should* have been sending locationchange for "about:certerror" and friends, except there's a bug (?) in docshell where we don't send a locationchange for an iframe's initial load, even if the initial load is an error page.

Boris, do you think it would break anything if we fired a locationchange for error pages, in an iframe's initial load?  Otherwise we can restrict this change to browser frames only.
Oh, I see; I should be looking at the statechange event for the error code.  It's still weird that we don't send a locationchange for the first load, if it's an error (per comment 1), but that's not important.
The "don't send locationchange for initial load" thing is restricted to subframes because it confused the UI when we did that.

But browser frames maybe shouldn't test true for the "is a subframe" check!
Chris, I think we drastically underestimated the complexity of informing the embedder of exactly what error occurred.  See [1] for a list of all the errors which can occur.

Informing the embedder /that/ an error occurred is simple, and I'll spin that up.

Depends on: 769200
Attached patch Patch v1Splinter Review
Attachment #637442 - Flags: review?(mounir)
Blocks: 766437
Comment on attachment 637442 [details] [diff] [review]
Patch v1

Review of attachment 637442 [details] [diff] [review]:

::: dom/browser-element/BrowserElementChild.js
@@ +415,5 @@
> +        if (status == Cr.NS_OK) {
> +          return;
> +        }
> +
> +        // XXX See nsDocShell::DisplayLoadError for a list of all the errors we

XXX comments shouldn't be added. This should be a NOTE or a TODO with a follow-up bug filed and the number in the comment.

@@ +417,5 @@
> +        }
> +
> +        // XXX See nsDocShell::DisplayLoadError for a list of all the errors we
> +        // should handle here.
> +        sendAsyncMsg('error', {type: 'other'});

Are you going to give more details about the error in a follow-up?

::: dom/browser-element/mochitest/browserElement_Error.js
@@ +35,5 @@
> +    });
> +
> +  });
> +
> +  iframe.src = "";

Why not:

Otherwise, I believe this is really going to try to access the domain while will be redirected locally.
Attachment #637442 - Flags: review?(mounir) → review+
I'm happy to s/XXX/TODO, but I don't like filing follow-ups for issues we haven't encountered yet.  The follow-up would be vague "support more error codes", but then I'd have to file /another/ bug for "support /this specific/ error code" when we actually needed it.  It's really not a net gain.

> Are you going to give more details about the error in a follow-up?

As needed, yes.

> Why not:

That doesn't work because mochitest returns a 404 page, and we only do the gecko error page when the server doesn't send an error page itself.

Since we already have one test for this (invalid cert) and there are 25 different ways to generate a Gecko error page, I'm happy simply deleting this test wholesale.
Assignee: justin.lebar+bug → nobody
Component: General → DOM: Mozilla Extensions
OS: Mac OS X → All
Product: Boot2Gecko → Core
QA Contact: general → general
Hardware: x86 → All
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla16
Do we have a bug like this for certs changes?
(In reply to mgalli from comment #10)
> Do we have a bug like this for certs changes?

I don't know what you mean by "certs changes", but you will get this error event if you visit a page with an invalid SSL certificate, if that answers your question.
Justin, can you document this API somewhere so we know what information it gives us exactly? This would really help with a discussion we're having about what the UI should do when an app encounters an error.
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.