Bug 115578 – [GTK] [WebKit2] enable displaying console.log messages to system console

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[arno.]

Hi,
WebCore preferences have a logsPageMessagesToSystemConsoleEnabled method which allows to display console (page) message to system console (shell console).
Maybe we could expose that to WebKit2 Gtk API.

[arno.]

Created attachment 200506 [details]
patch proposal

[arno.]

Created attachment 200530 [details]
updated patch against tot

[WebKit Commit Bot]

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

[WebKit Commit Bot]

Attachment 200530 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp', u'Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h', u'Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp']" exit_code: 1
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1113:  When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1115:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1116:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1117:  Weird number of spaces at line-start.  Are you using a 4-space indent?  [whitespace/indent] [3]
Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1118:  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/WebKitSettings.h"
Total errors found: 5 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Martin Robinson]

Comment on attachment 200530 [details]
updated patch against tot

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

Looks good to me, with just a few nits about naming and style and such. This needs to be approved by another GTK+ reviewer as well.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1108
> +     * WebKitSettings:enable-messages-to-system-console:

I'm not sure if the "system console" is a Gnomey concept. Perhaps this would be better named something like enable-page-messages-sent-to-stdout, though that seems a bit clunky too.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1110
> +     * Enable or disable displaying of page messages to system console.

You should probably explain a bit that this applies to messages that normally go to the web inspector console.

>> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1113
>> +                                    PROP_ENABLE_MESSAGES_TO_SYSTEM_CONSOLE,
> 
> When wrapping a line, only indent 4 spaces.  [whitespace/indent] [3]

Please fix these style issues.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h:395
> +                                                                gboolean        enabled);

Looks like the indentation is off here.

[arno.]

Created attachment 200547 [details]
updated patch

I checked and those messages are only written in case of a message console. So I think it would make sense to specify console in the name of the setting. In the new patch, the setting is called enable-write-console-messages-to-stdout; Other nits are fixed

[Sam Weinig]

Looks fine to me.

[arno.]

Created attachment 200596 [details]
updated patch: fixes gtkdoc errors

[arno.]

Created attachment 200719 [details]
updated patch: fixes gtkdoc errors

oups, I uploaded the wrong patch

[Martin Robinson]

Comment on attachment 200719 [details]
updated patch: fixes gtkdoc errors

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

Looks good to me. This needs another GTK+ reviewer to approve the new API, but I like the name now. :)

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1110
> +     * Enable or disable writing console messages to stdout.

You probably want to expand this documentation slightly to say that this includes console.log and friends.

[Martin Robinson]

Gustavo, Xan, or Carlos, could you take a look at this new WK2 API and r+ the patch if it appears reasonable to you as well?

[Gustavo Noronha (kov)]

Comment on attachment 200719 [details]
updated patch: fixes gtkdoc errors

Sure!

[arno.]

Created attachment 201863 [details]
updated patch: same patch; documentation updated to adress martin's comment

[WebKit Commit Bot]

Comment on attachment 201863 [details]
updated patch: same patch; documentation updated to adress martin's comment

Clearing flags on attachment: 201863

Committed r150158: <http://trac.webkit.org/changeset/150158>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.