When a page fails to load and client doesn't handle load errors, we should load a default error page with the error.
Created attachment 109475 [details] Patch
Comment on attachment 109475 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109475&action=review Looks fine to me, but I'm not sure if it's the right thing to do to "hide" the loading of the error page from the API. > Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:72 > + WebKitWebLoaderClient* client = WEBKIT_WEB_LOADER_CLIENT(clientInfo); > + if (client->priv->loadingErrorPage) > + return; > + > gboolean returnValue; > - g_signal_emit(WEBKIT_WEB_LOADER_CLIENT(clientInfo), signals[PROVISIONAL_LOAD_STARTED], 0, &returnValue); > + g_signal_emit(client, signals[PROVISIONAL_LOAD_STARTED], 0, &returnValue); Are you sure it's good to hide inner workings of the error page load from the API? > Source/WebKit2/UIProcess/API/gtk/WebKitWebLoaderClient.cpp:84 > + WebKitWebLoaderClient* client = WEBKIT_WEB_LOADER_CLIENT(clientInfo); > + if (client->priv->loadingErrorPage) > + return; > + > gboolean returnValue; Ditto.
(In reply to comment #2) > (From update of attachment 109475 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109475&action=review > > Looks fine to me, but I'm not sure if it's the right thing to do to "hide" the loading of the error page from the API. > It's not hidden, provisional-load-failed and load-failed signals documentation say that a custom error page will be loaded if the user doesn't handle the signal. The behaviour is exactly the same than webkit1 API.
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 109475 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=109475&action=review > > > > Looks fine to me, but I'm not sure if it's the right thing to do to "hide" the loading of the error page from the API. > > > > It's not hidden, provisional-load-failed and load-failed signals documentation say that a custom error page will be loaded if the user doesn't handle the signal. The behaviour is exactly the same than webkit1 API. I was referring to the loading of the custom error page. I was referring to the fact that you cannot observe the loading signals as the custom error page loads. Note that this does not happen on the Mac port. I think we should follow the approach of firing the signals. At least the conservative decision is to fire them now and let the UA track if it is loading an error page. I'm guessing the original decision was made to not break Epiphany. Since we are rewriting this code in Epiphany anyway, we can make whatever decision we want. My tendency is to be more similar to other ports.
(In reply to comment #4) > I was referring to the loading of the custom error page Sorry. I was *not* referring to the loading of the custom error page. :)
(In reply to comment #4) > I was referring to the loading of the custom error page. I was referring to the fact that you cannot observe the loading signals as the custom error page loads. Note that this does not happen on the Mac port. I think we should follow the approach of firing the signals. At least the conservative decision is to fire them now and let the UA track if it is loading an error page. > > I'm guessing the original decision was made to not break Epiphany. Since we are rewriting this code in Epiphany anyway, we can make whatever decision we want. My tendency is to be more similar to other ports. For me, the fact that the error message is a web page is an implementation detail, that's why I kept the wk1 behaviour of not exposing the load process of error pages to the user. But I agree, we can just emit the signals for now and take a final decision later, instead of the opposite.
Created attachment 109594 [details] Updated patch Patch updated to emit load signals even when loading error page.
Attachment 109594 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/UIProcess/API/gtk/WebKitError.cpp:27: webkit_network_error_quark is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebKit2/UIProcess/API/gtk/WebKitError.cpp:38: webkit_policy_error_quark is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebKit2/UIProcess/API/gtk/WebKitError.cpp:49: webkit_plugin_error_quark is incorrectly named. Don't use underscores in your identifier names. [readability/naming] [4] Source/WebKit2/UIProcess/API/gtk/WebKitError.h:66: Extra space before ( in function call [whitespace/parens] [4] Source/WebKit2/UIProcess/API/gtk/WebKitError.h:69: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 5 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 109594 [details] Updated patch Attachment 109594 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/9935500
Committed r96616: <http://trac.webkit.org/changeset/96616>