Bug 79918 - [GTK] Add WebKitWebPage::console-message-sent signal to Web Extensions API
Summary: [GTK] Add WebKitWebPage::console-message-sent signal to Web Extensions API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Joone Hur
URL:
Keywords: Gtk
Depends on: 79482
Blocks: 75543
  Show dependency treegraph
 
Reported: 2012-02-29 08:51 PST by Carlos Garcia Campos
Modified: 2015-12-18 11:47 PST (History)
13 users (show)

See Also:


Attachments
Patch (22.98 KB, patch)
2012-02-29 08:55 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
New patch (28.19 KB, patch)
2012-03-29 08:20 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
rebased patch (28.65 KB, patch)
2012-11-13 16:33 PST, Joone Hur
no flags Details | Formatted Diff | Diff
Updated patch (27.53 KB, patch)
2012-11-14 03:59 PST, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Patch (41.22 KB, patch)
2015-09-04 00:41 PDT, Carlos Garcia Campos
no flags Details | Formatted Diff | Diff
Updated patch (41.29 KB, patch)
2015-10-14 23:24 PDT, Carlos Garcia Campos
gns: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2012-02-29 08:51:02 PST
It will be useful to get information about javascript exceptions an other console messages. See also bug #75543.
Comment 1 Carlos Garcia Campos 2012-02-29 08:55:06 PST
Created attachment 129460 [details]
Patch
Comment 2 Carlos Garcia Campos 2012-03-29 08:20:10 PDT
Created attachment 134591 [details]
New patch

I've updated the patch to move the WebKitConsoleMessage boxed type implementation to its own file, following the same approach than WebKitJavaScriptResult and WebKitScriptDialog boxed types.
Comment 3 WebKit Review Bot 2012-03-29 08:23:40 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Philippe Normand 2012-03-29 08:24:34 PDT
Comment on attachment 134591 [details]
New patch

Attachment 134591 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12227001
Comment 5 Carlos Garcia Campos 2012-03-29 08:26:34 PDT
Patch doesn't build because it depends on bug #79482
Comment 6 Joone Hur 2012-11-13 16:33:32 PST
Created attachment 174024 [details]
rebased patch
Comment 7 Carlos Garcia Campos 2012-11-14 03:59:18 PST
Created attachment 174123 [details]
Updated patch

We don't use the C API internally anymore.
Comment 8 WebKit Review Bot 2012-11-14 04:03:30 PST
Attachment 174123 [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/WebKitWebView.cpp:1262:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1263:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1264:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1265:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1266:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1267:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1268:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitWebView.cpp:1269:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitWebView.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitConsoleMessage.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/webkit2.h"
Total errors found: 8 in 13 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 kov's GTK+ EWS bot 2012-11-14 07:14:12 PST
Comment on attachment 174123 [details]
Updated patch

Attachment 174123 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/14832365
Comment 10 Gustavo Noronha (kov) 2012-12-08 08:31:02 PST
Comment on attachment 174123 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=174123&action=review

> Source/WebKit2/UIProcess/API/gtk/WebKitConsoleMessage.h:39
> + * @WEBKIT_CONSOLE_MESSAGE_SOURCE_NETWORK: Message produced by WebSockets.

Wondering if it would make sense to use WEBSOCKETS instead of NETWORK here, but I guess using NETWORK makes this more future-proof, as in, it could be used for other features that deal with network connections, even though that would mean a slight change in behaviour, meaning it would no longer mean websockets.

> Source/WebKit2/UIProcess/API/gtk/WebKitUIClient.cpp:161
> +static void addMessageToConsole(WKPageRef page, WKMessageSourceType source, WKMessageLevelType level, WKStringRef message, unsigned lineNumber, WKStringRef sourceID, const void* clientInfo)

The build fails, apparently we should use WebCore::MessageSource, WebCore::MessageType, and WebCore::MessageLevel here.
Comment 11 Carlos Garcia Campos 2012-12-08 08:39:47 PST
Build fails because this depends on another patch, that unfortunately was rejected and the bug closed as wont fix. We'll have to use injected bundle to expose this api.
Comment 12 Carlos Garcia Campos 2015-09-04 00:23:49 PDT
I'm giving this another try, this time moving the API to the web extensions. This way we avoid the IPC traffic, users can handle the messages in their web extension or send them to the UI process. I think it would be better to use console-message-sent, instead of console-message, since this signal is just a notification, not an action that the user can handle or prevent.
Comment 13 Carlos Garcia Campos 2015-09-04 00:41:15 PDT
Created attachment 260576 [details]
Patch
Comment 14 Csaba Osztrogonác 2015-09-14 11:10:26 PDT
Comment on attachment 174123 [details]
Updated patch

Cleared Gustavo Noronha Silva's review+ from obsolete attachment 174123 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment 15 Carlos Garcia Campos 2015-10-14 23:24:20 PDT
Created attachment 263140 [details]
Updated patch

Fixed the ( used in HTTP error message, I noticed I was using ) ) instead of ( )
Comment 16 Carlos Garcia Campos 2015-10-28 02:09:16 PDT
ping reviewers, Gustavo?
Comment 17 Gustavo Noronha (kov) 2015-12-07 02:24:05 PST
Comment on attachment 263140 [details]
Updated patch

View in context: https://bugs.webkit.org/attachment.cgi?id=263140&action=review

> Source/WebKit2/ChangeLog:34
> +        (didReceiveResponseForResource): Generate a console message in
> +        case of HTTP failure >= 400 for consistency with the inspector.

Nice touch =)
Comment 18 Carlos Garcia Campos 2015-12-07 03:56:51 PST
Committed r193620: <http://trac.webkit.org/changeset/193620>
Comment 19 Michael Catanzaro 2015-12-18 11:29:22 PST
It broke distcheck:

/home/mcatanzaro/src/WebKit/WebKitBuild/GNOME/Documentation/webkit2gtk-4.0/webkit2gtk-docs.sgml:65: element include: XInclude error : could not load /home/mcatanzaro/src/WebKit/WebKitBuild/GNOME/Documentation/webkit2gtk-4.0/xml/WebKitConsoleMessage, and no fallback was found
Comment 20 Michael Catanzaro 2015-12-18 11:47:14 PST
Committed r194280: <http://trac.webkit.org/changeset/194280>