1480095 - Add API for custom error pages

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[James Willcox (:snorp) ([e-mail]) (he/him)]

Bug 1451476 exposes page load error codes to apps. Often, apps will want to then display their own error pages to the user.

Right now DocShell uses an internal flag, LOAD_FLAGS_ERROR_PAGE, to load error pages. If we expose this flag to apps, we should get most of the same special handling that Gecko's internal error pages enjoy.

Comment 1

[James Willcox (:snorp) ([e-mail]) (he/him)]

smaug, bz: I think we could use your opinion on this. Is exposing LOAD_FLAGS_ERROR_PAGE to apps a good way to allow custom error pages? This would tie into the error code stuff from bug 1451476.

In that bug, smaug brought up that iframes are a problem. That's true here as well. Another thought I had in bug 1451476 was to return an error page URL.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I think the biggest worry would be that LOAD_FLAGS_ERROR_PAGE loads are pretty special in terms of their session history handling and right now can only happen at very specific places in the pageload process.  Trying to do them at other times may run into now-unknown bugs.

From that point of view, returning the error page url so docshell can keep doing what it's doing now but just with a different url is a safer change.  But can you return such a url synchronously in geckoview?

Apart from that issue, explicitly adding a loadErrorPage() function does not seem insane to me...

Comment 3

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

Review commit: https://reviewboard.mozilla.org/r/261582/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/261582/

Comment 4

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

Review commit: https://reviewboard.mozilla.org/r/261584/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/261584/

Comment 5

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1480095 - Add example custom error page to GeckoView Example

Review commit: https://reviewboard.mozilla.org/r/261586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/261586/

Comment 6

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

::: docshell/base/nsDocShell.cpp:4319
(Diff revision 1)
> -  errorPage.AssignLiteral("neterror");
> +  bool isFennec = false;
> +#ifdef MOZ_WIDGET_ANDROID
> +  isFennec = mozilla::jni::IsFennec();
> +#endif
>  
> -  if (mLoadURIDelegate) {
> +  if (mLoadURIDelegate && !isFennec) {

I don't think Fennec-specific stuff should be in `nsDocShell.cpp`. What happens if we performed default action if `handleLoadError` returned null?

::: docshell/base/nsDocShell.cpp:4321
(Diff revision 1)
>      rv = mLoadURIDelegate->HandleLoadError(aURI, aError,
>                                             NS_ERROR_GET_MODULE(aError),
> -                                           &loadErrorHandled);
> +                                           errorPageUrl);

Not really this bug, but this is again prone to out-of-order loads similar to bug 1441059

::: xpcom/base/nsILoadURIDelegate.idl:45
(Diff revision 1)
>     * @param aError The error code.
>     * @param aErrorModule The error module code.
>  
> -   * Returns whether the page load error has been successfully handled.
> +   * Returns an error page URL to load, or null if no error page should be shown.
>     */
> -  boolean
> +  AString

Probably want `nsIURI`

Comment 7

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review268560

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt:116
(Diff revision 1)
> +        assertThat("Title should match",
> +                mainSession.evaluateJS("window.document.title") as String,
> +                containsString("Hello"))

I would expect `onTitleChange` to be called?

Comment 8

[Eugen Sawin [:esawin]]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review268728

::: mobile/android/chrome/geckoview/GeckoViewNavigationContent.js:57
(Diff revision 1)
> -    const handled = LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
> +    const errorUrl = LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
> -                                                    aUri, aError, aErrorModule);
> +                                                     aUri, aError, aErrorModule);
> +
> +    if (!errorUrl) {
> +      // We only need to send PageStop here if we aren't loading
> +      // an error page.

Is this the expected behavior? We wouldn't get a PageStop for the site that failed to load.

::: mobile/android/chrome/geckoview/GeckoViewNavigationContent.js:60
(Diff revision 1)
> +    if (!errorUrl) {
> +      // We only need to send PageStop here if we aren't loading
> +      // an error page.
> -    this.eventDispatcher.sendRequest({
> +      this.eventDispatcher.sendRequest({
> -      type: "GeckoView:PageStop",
> +        type: "GeckoView:PageStop",
> -      sucess: false,
> +        sucess: false,

success

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt
(Diff revision 1)
>   * Any copyright is dedicated to the Public Domain.
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  
>  package org.mozilla.geckoview.test
>  
> -import org.mozilla.gecko.util.GeckoBundle

GeckoBundle is used in the telemetry test.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt:79
(Diff revision 1)
>                          GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
>                          GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
>      }
>  
> +    @WithDevToolsAPI
> +    @Test fun loadErrorPage() {

You could modify loadExpectError to take the error page as an argument instead.

::: mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoSession.java:2464
(Diff revision 1)
>           * @param session The GeckoSession that initiated the callback.
>           * @param uri The URI that failed to load.
>           * @param category The error category.
>           * @param error The error type.
>           */
> -        void onLoadError(GeckoSession session, String uri,
> +        GeckoResult<String> onLoadError(GeckoSession session, String uri,

Missing docs.

::: mobile/android/modules/geckoview/GeckoViewProgress.jsm:20
(Diff revision 1)
>  
>  XPCOMUtils.defineLazyServiceGetter(this, "IDNService",
>    "@mozilla.org/network/idn-service;1", "nsIIDNService");
>  
> +// From nsDocShellLoadTypes.h
> +const LOAD_TYPE_ERROR_PAGE = (1 << 16);

Is there a reason not to have this flag in nsIWebNavigation.idl?

Comment 9

[Eugen Sawin [:esawin]]

Comment on attachment 8997898 [details]
Bug 1480095 - Add example custom error page to GeckoView Example

https://reviewboard.mozilla.org/r/261586/#review268736

::: mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java:760
(Diff revision 1)
> +            }
> +
> +            return GeckoViewActivity.this.createErrorPage(categoryToString(category) + " : " + errorToString(error));
> +        }
> +
> +        public GeckoResult<String> onLoadError(final GeckoSession session, final String uri,

@Override

::: mobile/android/geckoview_example/src/main/java/org/mozilla/geckoview_example/GeckoViewActivity.java:766
(Diff revision 1)
> -                                final int category, final int error) {
> +                                               final int category, final int error) {
>              Log.d(LOGTAG, "onLoadError=" + uri +
>                    " error category=" + category +
>                    " error=" + error);
> +
> +            return GeckoResult.fromValue("data:text/html," + createErrorPage(category, error));

It seems like having an error page template file makes the code more cumbersome, we could just return an inline string instead.

Comment 10

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> I don't think Fennec-specific stuff should be in `nsDocShell.cpp`. What happens if we performed default action if `handleLoadError` returned null?

It's not ideal, but I don't think it's too bad. The problem with continuing in `DisplayLoadError()` if the delegate returns `null` is that error pages are essentially broken in GV. We shouldn't show them unless they are fixed, IMO. It's not as easy as just including Fennec's error pages, because many of them talk to Fennec.

Comment 11

[Eugen Sawin [:esawin]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268922

Patch contains empty error.html for the example app.

I also agree with Jim's comments. We can keep Fennec-specific handling in GeckoView code, as long as it's required.

::: docshell/base/nsDocShell.cpp:4320
(Diff revision 1)
> +#ifdef MOZ_WIDGET_ANDROID
> +  isFennec = mozilla::jni::IsFennec();
> +#endif
>  
> -  if (mLoadURIDelegate) {
> -    bool loadErrorHandled = false;
> +  if (mLoadURIDelegate && !isFennec) {
> +    nsAutoString errorPageUrl;

We're mixing URL and URI again, maybe better: errorPageUriString?

::: docshell/base/nsDocShell.cpp
(Diff revision 1)
>      rv = mLoadURIDelegate->HandleLoadError(aURI, aError,
>                                             NS_ERROR_GET_MODULE(aError),
> -                                           &loadErrorHandled);
> -    if (NS_SUCCEEDED(rv) && loadErrorHandled) {
> -      // The request has been handled, nothing to do here.
> +                                           errorPageUrl);
> +    if (NS_FAILED(rv) || errorPageUrl.IsEmpty()) {
> +      // We don't want to load an error page. We're done here.
> -      *aDisplayedErrorPage = false;

You still want to set *aDisplayedErrorPage = false explicitly, in case the code above changes.

Comment 12

[Eugen Sawin [:esawin]]

Comment on attachment 8997898 [details]
Bug 1480095 - Add example custom error page to GeckoView Example

https://reviewboard.mozilla.org/r/261586/#review268924

I think without error.html, the code could be simplified, but otherwise OK.

Comment 13

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review268728

> Is there a reason not to have this flag in nsIWebNavigation.idl?

It's complicated, I think, due to how the load flags are are created in nsDocShellLoadTypes.h. Also, the error load type is private to the docshell. Anyway, the current incarnation of the patch doesn't seem to need it so I've removed this.

Comment 14

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review268836

::: mobile/android/chrome/geckoview/GeckoViewNavigationContent.js:57
(Diff revision 1)
> -    const handled = LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
> +    const errorUrl = LoadURIDelegate.handleLoadError(content, this.eventDispatcher,
> -                                                    aUri, aError, aErrorModule);
> +                                                     aUri, aError, aErrorModule);
> +
> +    if (!errorUrl) {
> +      // We only need to send PageStop here if we aren't loading
> +      // an error page.

We seem to get one according to the tests at least.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt
(Diff revision 1)
>   * Any copyright is dedicated to the Public Domain.
>     http://creativecommons.org/publicdomain/zero/1.0/ */
>  
>  package org.mozilla.geckoview.test
>  
> -import org.mozilla.gecko.util.GeckoBundle

It is, but you don't need to import it since you don't explicitly declare it anywhere.

Comment 15

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review268560

> I would expect `onTitleChange` to be called?

It is, I've wrestled with the tests now.

Comment 16

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997898 [details]
Bug 1480095 - Add example custom error page to GeckoView Example

https://reviewboard.mozilla.org/r/261586/#review268736

> It seems like having an error page template file makes the code more cumbersome, we could just return an inline string instead.

It's just not much fun to edit an inline html file. Apps are definitely going to use an asset or resource, so we should too.

Comment 17

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> It's not ideal, but I don't think it's too bad. The problem with continuing in `DisplayLoadError()` if the delegate returns `null` is that error pages are essentially broken in GV. We shouldn't show them unless they are fixed, IMO. It's not as easy as just including Fennec's error pages, because many of them talk to Fennec.

Then return empty string from `handleLoadError` if we're in GV and `null` if we're in Fennec? That way you don't need to check `isFennec` in `nsDocShell` itself.

Comment 18

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/261582/diff/1-2/

Comment 19

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/261584/diff/1-2/

Comment 20

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997898 [details]
Bug 1480095 - Add example custom error page to GeckoView Example

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/261586/diff/1-2/

Comment 21

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> Then return empty string from `handleLoadError` if we're in GV and `null` if we're in Fennec? That way you don't need to check `isFennec` in `nsDocShell` itself.

Yeah, I can try that with this latest patch.

Comment 22

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> Yeah, I can try that with this latest patch.

Whoops, I don't think we're actually in any better shape there. It's valid to return a null error page with GV. We short-circuit and don't load anything at all. We still need to know if we're in Fennec in DocShell.

Comment 23

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268922

> We're mixing URL and URI again, maybe better: errorPageUriString?

In DocShell, URL is consistently used for strings, and URI for nsIURI.

Comment 24

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> Whoops, I don't think we're actually in any better shape there. It's valid to return a null error page with GV. We short-circuit and don't load anything at all. We still need to know if we're in Fennec in DocShell.

I don't see why it has to happen at the `nsDocShell` level. If consumer returns `null` and we're in GV, we pass back empty string to stop loading. If consumer returns `null` and we're in Fennec, we pass back `null` to perform default loading. A non-null non-empty string in either case performs custom loading. That seems like a doable solution.

Comment 25

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> I don't see why it has to happen at the `nsDocShell` level. If consumer returns `null` and we're in GV, we pass back empty string to stop loading. If consumer returns `null` and we're in Fennec, we pass back `null` to perform default loading. A non-null non-empty string in either case performs custom loading. That seems like a doable solution.

Seems fragile and non-obvious to me. Also, not sure we can put an empty string in a nsIURI. I think the intentions are clear if we have jni::IsFennec() -- never load a custom error page in Fennec. If you feel strongly, I'll give it a shot.

Comment 26

[James Willcox (:snorp) ([e-mail]) (he/him)]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> Seems fragile and non-obvious to me. Also, not sure we can put an empty string in a nsIURI. I think the intentions are clear if we have jni::IsFennec() -- never load a custom error page in Fennec. If you feel strongly, I'll give it a shot.

We also want different behavior in GV vs Fennec when there is no delegate at all in the DocShell. With GV, it should block the error page load. I guess we could always install the delegate in GV, but that seems pretty clumsy to me considering we can avoid it with 3 lines in the DocShell. I don't get the aversion to this at all -- we have MOZ_WIDGET_ANDROID and jni::isFennec() all over the code base.

Comment 27

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate r=esawin,bz,jchen

MozReview-Commit-ID: IhVC9nU60fy

Comment 28

[James Willcox (:snorp) ([e-mail]) (he/him)]

Attachment: Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen

MozReview-Commit-ID: 4pgHD6oh4GY

Comment 29

[Eugen Sawin [:esawin]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review269244

Looks good with some nits.

The file mobile/android/geckoview_example/src/main/assets/error.htm has no place in this patch.

::: docshell/base/nsDocShell.cpp:4321
(Diff revision 2)
> +  isFennec = mozilla::jni::IsFennec();
> +#endif
> +
> +  if (!isFennec) {
> +    if (!mLoadURIDelegate) {
> +      return NS_OK;

Please add a comment that we simply want to block without an error page.

We might want to set *aDisplayedErrorPage = false in case the code above gets changed.

::: docshell/base/nsDocShell.cpp
(Diff revision 2)
>      rv = mLoadURIDelegate->HandleLoadError(aURI, aError,
>                                             NS_ERROR_GET_MODULE(aError),
> -                                           &loadErrorHandled);
> -    if (NS_SUCCEEDED(rv) && loadErrorHandled) {
> +                                           getter_AddRefs(errorPageURI));
> +    if (NS_FAILED(rv) || !errorPageURI) {
> -      // The request has been handled, nothing to do here.
> -      *aDisplayedErrorPage = false;

Please add a comment.
Also, I still think *aDisplayedErrorPage = false should be set.

Comment 30

[Eugen Sawin [:esawin]]

Comment on attachment 8997897 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page

https://reviewboard.mozilla.org/r/261584/#review269246

Looks good with some nits.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt:235
(Diff revision 2)
> -                        GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
> +                GeckoSession.NavigationDelegate.ERROR_CATEGORY_NETWORK,
> -                        GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
> +                GeckoSession.NavigationDelegate.ERROR_PORT_BLOCKED)
>      }
>  
> +    @Test fun loadUntrusted() {
> +        loadExpectError(if (sessionRule.env.isAutomation)

Please set a val for the URI before calling loadExpectError.

::: mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/NavigationDelegateTest.kt:237
(Diff revision 2)
> +        else
> +            "https://expired.badssl.com/",

Redundant ;

::: mobile/android/modules/geckoview/GeckoViewProgress.jsm:236
(Diff revision 2)
> +    const isStop = (aStateFlags & Ci.nsIWebProgressListener.STATE_STOP) != 0;
> +
> +    debug `onStateChange: uri=${uriSpec} isSuccess=${isSuccess}
> +           isStart=${isStart} isStop=${isStop}`;
>  
> -    if (aStateFlags & Ci.nsIWebProgressListener.STATE_START) {
> +    if ((aStateFlags & Ci.nsIWebProgressListener.STATE_START)) {

Why not use isStart here?

::: mobile/android/modules/geckoview/GeckoViewProgress.jsm
(Diff revision 2)
>    onLocationChange(aWebProgress, aRequest, aLocationURI, aFlags) {
>      debug `onLocationChange: location=${aLocationURI.displaySpec},
>                               flags=${aFlags}`;
>  
>      this._hostChanged = true;
> -    if (aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) {

Are we positive that this is no longer an issue?

::: mobile/android/modules/geckoview/LoadURIDelegate.jsm:55
(Diff revision 2)
>        errorClass = nssErrorsService.getErrorClass(aError);
>      } catch (e) {}
>  
>      const msg = {
>        type: "GeckoView:OnLoadError",
> -      uri: aUri.spec,
> +      uri: aUri ? aUri.spec : null,

Why would the URI ever be null?

Comment 31

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8997896 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate

https://reviewboard.mozilla.org/r/261582/#review268556

> We also want different behavior in GV vs Fennec when there is no delegate at all in the DocShell. With GV, it should block the error page load. I guess we could always install the delegate in GV, but that seems pretty clumsy to me considering we can avoid it with 3 lines in the DocShell. I don't get the aversion to this at all -- we have MOZ_WIDGET_ANDROID and jni::isFennec() all over the code base.

It's just that `jni::IsFennec()` was a band-aid for separating GeckoView out from Fennec, so we shouldn't add any more usages of it, at least not in non-Android code. Currently, the only non-Android code that uses it is in the profiler and download-manager code. `nsDocShell` is probably one of the most frequently touched files in Gecko, so anything that's Fennec-specific just adds to the maintainance load for other people that may be touching this code.

It's actually fairly easy to make GV not load an error page by default. We can just add an override for `netError.xhtml` inside "geckoview/jar.mn" and point that to an invalid URI, e.g. `% override chrome://global/content/netError.xhtml chrome://geckoview/nonexistent`. With that, you don't need to detect GV/Fennec here at all.

Comment 32

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8999192 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate r=esawin,bz,jchen

Boris Zbarsky [:bzbarsky, bz on IRC] (vacation Aug 16-27) has approved the revision.

Comment 33

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8999192 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate r=esawin,bz,jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 34

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8999193 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen

Jim Chen [:jchen] [:darchons] has approved the revision.

Comment 35

[Eugen Sawin [:esawin]]

Comment on attachment 8999192 [details]
Bug 1480095 - Allow loading custom error pages via nsILoadURIDelegate r=esawin,bz,jchen

Eugen Sawin [:esawin] on leave from 20 August has approved the revision.

Comment 36

[Eugen Sawin [:esawin]]

Comment on attachment 8999193 [details]
Bug 1480095 - Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen

Eugen Sawin [:esawin] on leave has approved the revision.

Comment 37

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/6cd69a56e853
Allow loading custom error pages via nsILoadURIDelegate r=esawin,bz,jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/d49371d4563a
Allow NavigationDelegate.onLoadError() to return URL for error page r=esawin,jchen
https://hg.mozilla.org/integration/mozilla-inbound/rev/6bb8792d978c
Add example custom error page to GeckoView Example r=esawin

Comment 38

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6cd69a56e853
https://hg.mozilla.org/mozilla-central/rev/d49371d4563a
https://hg.mozilla.org/mozilla-central/rev/6bb8792d978c

Comment 39

[Chris Peterson [:cpeterson]]

status-firefox62=wontfix because we don't need to uplift to GV 62 Beta