RESOLVED WONTFIX Bug 79482
[WK2] Implement WebChromeClient::addMessageToConsole()
https://bugs.webkit.org/show_bug.cgi?id=79482
Summary [WK2] Implement WebChromeClient::addMessageToConsole()
Carlos Garcia Campos
Reported 2012-02-24 05:42:42 PST
It's currently unimplemented. It will be useful to get information about exceptions and any other messages sent to the javascript console.
Attachments
Patch (23.99 KB, patch)
2012-02-29 08:14 PST, Carlos Garcia Campos
no flags
Updated patch to add new files to xcode too (30.44 KB, patch)
2012-03-22 07:38 PDT, Carlos Garcia Campos
buildbot: commit-queue-
Another attempt to fix mac build (32.08 KB, patch)
2012-03-29 05:20 PDT, Carlos Garcia Campos
sam: review-
buildbot: commit-queue-
Patch including unit tests (42.12 KB, patch)
2012-04-23 09:48 PDT, Carlos Garcia Campos
buildbot: commit-queue-
Rebased patch (38.76 KB, patch)
2012-11-12 20:39 PST, Joone Hur
buildbot: commit-queue-
Updated patch (35.93 KB, patch)
2012-11-14 02:45 PST, Carlos Garcia Campos
buildbot: commit-queue-
Updated patch (36.67 KB, patch)
2012-11-21 03:18 PST, Carlos Garcia Campos
no flags
Alexey Proskuryakov
Comment 1 2012-02-24 14:03:43 PST
This seems to work on Mac. Are only some platform specific bits unimplemented?
Carlos Garcia Campos
Comment 2 2012-02-25 01:59:32 PST
What do you mean by "work"? I guess mac uses injected bundle API then. Web process doesn't notify the ui process about messages sent to the javascript console, so there's no callback for this in the UIClient. See http://trac.webkit.org/browser/trunk/Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp#L263 So, what I'm requesting is a DidAddMessageToConsole message sent to the UI process instead of the notImplemented(); And a callback in the UIClient to be able to handle messages from the UI process. I'll write the patch if someone confirms this is something that we want.
Carlos Garcia Campos
Comment 3 2012-02-29 08:14:21 PST
Created attachment 129454 [details] Patch I haven't updated the project.pbxproj because I have no idea how to generate the fileRef of new files.
Carlos Garcia Campos
Comment 4 2012-03-21 09:37:08 PDT
Could anybody tell me how to add new files to xcode project files, so that I can fix this patch to work on mac too?
Adam Barth
Comment 5 2012-03-21 10:11:10 PDT
(In reply to comment #4) > Could anybody tell me how to add new files to xcode project files, so that I can fix this patch to work on mac too? Unfortunately, you need to use Xcode itself. If you don't have a Mac, there are some Python scripts you can find that might be able to edit the file. I think there's one called xcode-bodge that used to be used by the Chromium project.
Carlos Garcia Campos
Comment 6 2012-03-22 07:38:02 PDT
Created attachment 133259 [details] Updated patch to add new files to xcode too Rebased to current git master and added new files to xcode too
Carlos Garcia Campos
Comment 7 2012-03-22 07:41:29 PDT
(In reply to comment #5) > (In reply to comment #4) > > Could anybody tell me how to add new files to xcode project files, so that I can fix this patch to work on mac too? > > Unfortunately, you need to use Xcode itself. If you don't have a Mac, there are some Python scripts you can find that might be able to edit the file. I think there's one called xcode-bodge that used to be used by the Chromium project. Thanks!, I used xcodebodge in the end, I hope it works.
Build Bot
Comment 8 2012-03-22 08:05:47 PDT
Comment on attachment 133259 [details] Updated patch to add new files to xcode too Attachment 133259 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12114043
Build Bot
Comment 9 2012-03-22 08:06:50 PDT
Comment on attachment 133259 [details] Updated patch to add new files to xcode too Attachment 133259 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12117042
Carlos Garcia Campos
Comment 10 2012-03-22 08:11:01 PDT
It seems it didn't work :-(, any idea what's wrong?
Carlos Garcia Campos
Comment 11 2012-03-29 05:20:14 PDT
Created attachment 134553 [details] Another attempt to fix mac build This time I added the files to WebKit2 and WebProcess targets, I don't know if it's enough or even correct.
Build Bot
Comment 12 2012-03-29 05:38:06 PDT
Comment on attachment 134553 [details] Another attempt to fix mac build Attachment 134553 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12193281
Build Bot
Comment 13 2012-03-29 05:44:46 PDT
Comment on attachment 134553 [details] Another attempt to fix mac build Attachment 134553 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12192259
Gustavo Noronha (kov)
Comment 14 2012-03-30 12:48:34 PDT
Looking at the xcode project file and comparing to WKBase, it looks like the header is missing from the 'in Headers' lists: BCDDB317124EBD130048D13C /* WKBase.h in Headers */, BCDDB317124EBD130048D13C /* WKBase.h in Headers */ = {isa = PBXBuildFile; fileRef = BCDDB316124EBD130048D13C /* WKBase.h */; settings = {ATTRIBUTES = (Public, ); }; }; Seems like this is what's missing for the new header.
Sam Weinig
Comment 15 2012-04-01 19:33:53 PDT
Is this actually used by people? I would rather not add additional client APIs if no one wants this.
Carlos Garcia Campos
Comment 16 2012-04-01 23:30:56 PDT
(In reply to comment #15) > Is this actually used by people? I would rather not add additional client APIs if no one wants this. Yes, at least people who use the GTK+ port, I don't know other ports users though.
Sam Weinig
Comment 17 2012-04-02 22:26:40 PDT
Comment on attachment 134553 [details] Another attempt to fix mac build If you are going to add new API, please add API tests to TestWebKitAPI as well.
Carlos Garcia Campos
Comment 18 2012-04-03 10:13:33 PDT
(In reply to comment #17) > (From update of attachment 134553 [details]) > If you are going to add new API, please add API tests to TestWebKitAPI as well. Fair enough. Patch attached to bug #79918 includes unit tests for the GTK+ port API that wraps this one, fwiw. In any case, I'll add a test to TestWebKitAPI too, but we don't use/build it, so it won't be trivial thing.
Carlos Garcia Campos
Comment 19 2012-04-23 09:48:17 PDT
Created attachment 138366 [details] Patch including unit tests I still haven't figured out how to add new files to xcodeproj files, and I don't have a mac, so it will still fail to build for mac and win.
Build Bot
Comment 20 2012-04-23 10:10:44 PDT
Comment on attachment 138366 [details] Patch including unit tests Attachment 138366 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12494022
Build Bot
Comment 21 2012-04-23 10:27:05 PDT
Comment on attachment 138366 [details] Patch including unit tests Attachment 138366 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12511241
Philippe Normand
Comment 22 2012-11-12 01:47:00 PST
The Qt port might be interested by that feature as well. I heard Joone recently rebased this patch, any news about it?
Joone Hur
Comment 23 2012-11-12 20:39:41 PST
Created attachment 173804 [details] Rebased patch
WebKit Review Bot
Comment 24 2012-11-12 20:43:33 PST
Attachment 173804 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/CMakeLists.txt', u'Source/W..." exit_code: 1 Source/WebKit2/WebProcess/WebCoreSupport/WebChromeClient.cpp:267: Omit int when using unsigned [runtime/unsigned] [1] Total errors found: 1 in 22 files If any of these errors are false positives, please file a bug against check-webkit-style.
Build Bot
Comment 25 2012-11-12 21:06:08 PST
Comment on attachment 173804 [details] Rebased patch Attachment 173804 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14818425
Carlos Garcia Campos
Comment 26 2012-11-12 23:42:34 PST
Comment on attachment 173804 [details] Rebased patch The patch adds a new callback to the UI client, so we need to add a new version an update the API traits. I'll submit a new patch.
Build Bot
Comment 27 2012-11-13 03:11:29 PST
Comment on attachment 173804 [details] Rebased patch Attachment 173804 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/14821521
Carlos Garcia Campos
Comment 28 2012-11-14 02:45:54 PST
Created attachment 174119 [details] Updated patch Added new interface version to UI client and update API traits. Also added constructors to ConsoleMessage to simplify the code. I haven't included the xcode changes because they don't work anyway.
Build Bot
Comment 29 2012-11-14 03:04:54 PST
Comment on attachment 174119 [details] Updated patch Attachment 174119 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14844023
Joone Hur
Comment 30 2012-11-20 06:00:17 PST
There is a build break in the Mac port as follows: Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/WebPageProxyMessages.h:85:5: error: class 'ConsoleMessage' was previously declared as a struct [-Werror,-Wmismatched-tags] class ConsoleMessage; ^ /Volumes/Data/EWS/WebKit/Source/WebKit2/Shared/ConsoleMessage.h:39:8: note: previous use is here struct ConsoleMessage { ^ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/DerivedSources/WebKit2/WebPageProxyMessages.h:85:5: note: did you mean struct here? class ConsoleMessage; ^~~~~ struct 1 error generated. Why is ConsoleMessage added to WebPageProxyMessages.h as forward declaration of a class?
Carlos Garcia Campos
Comment 31 2012-11-20 08:46:32 PST
We need to add WebKit::ConsoleMessage to the list of structs in struct_or_class function (Source/WebKit2/Scripts/webkit2/messages.py)
Carlos Garcia Campos
Comment 32 2012-11-21 03:18:43 PST
Created attachment 175402 [details] Updated patch Added WebKit::ConsoleMessage to the list of structs
Build Bot
Comment 33 2012-11-21 03:40:47 PST
Comment on attachment 175402 [details] Updated patch Attachment 175402 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/14911982
Sam Weinig
Comment 34 2012-11-23 16:10:26 PST
We purposely left this out, the WebInspector has superseded the need for this callback.
Joone Hur
Comment 35 2012-11-25 07:37:29 PST
(In reply to comment #34) > We purposely left this out, the WebInspector has superseded the need for this callback. However, there are webview clients that don't use the WebInspector. In this case, this callback is very useful.
Sam Weinig
Comment 36 2012-11-25 10:09:33 PST
(In reply to comment #35) > (In reply to comment #34) > > We purposely left this out, the WebInspector has superseded the need for this callback. > > However, there are webview clients that don't use the WebInspector. In this case, this callback is very useful. For those rare cases, the bundle provides a WKBundlePageWillAddMessageToConsole callback.
Eric Seidel (no email)
Comment 37 2013-01-04 00:52:51 PST
Comment on attachment 175402 [details] Updated patch Cleared review? from attachment 175402 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).
Note You need to log in before you can comment on or make changes to this bug.