WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
69255
[GTK] Implement default error pages in WebKit2 GTK+ API
https://bugs.webkit.org/show_bug.cgi?id=69255
Summary
[GTK] Implement default error pages in WebKit2 GTK+ API
Carlos Garcia Campos
Reported
2011-10-03 05:37:12 PDT
When a page fails to load and client doesn't handle load errors, we should load a default error page with the error.
Attachments
Patch
(11.76 KB, patch)
2011-10-03 05:45 PDT
,
Carlos Garcia Campos
no flags
Details
Formatted Diff
Diff
Updated patch
(6.54 KB, patch)
2011-10-04 01:47 PDT
,
Carlos Garcia Campos
mrobinson
: review+
gustavo
: commit-queue-
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Carlos Garcia Campos
Comment 1
2011-10-03 05:45:48 PDT
Created
attachment 109475
[details]
Patch
Martin Robinson
Comment 2
2011-10-03 09:36:31 PDT
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.
Carlos Garcia Campos
Comment 3
2011-10-03 09:42:01 PDT
(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.
Martin Robinson
Comment 4
2011-10-03 09:54:27 PDT
(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.
Martin Robinson
Comment 5
2011-10-03 09:55:06 PDT
(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. :)
Carlos Garcia Campos
Comment 6
2011-10-03 23:28:50 PDT
(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.
Carlos Garcia Campos
Comment 7
2011-10-04 01:47:23 PDT
Created
attachment 109594
[details]
Updated patch Patch updated to emit load signals even when loading error page.
WebKit Review Bot
Comment 8
2011-10-04 01:50:10 PDT
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.
Gustavo Noronha (kov)
Comment 9
2011-10-04 03:57:00 PDT
Comment on
attachment 109594
[details]
Updated patch
Attachment 109594
[details]
did not pass gtk-ews (gtk): Output:
http://queues.webkit.org/results/9935500
Carlos Garcia Campos
Comment 10
2011-10-04 11:13:31 PDT
Committed
r96616
: <
http://trac.webkit.org/changeset/96616
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug