Implement window.console in WebCore

• Component: DOM
• Status: RESOLVED FIXED

[David Kilzer (:ddkilzer)]

The first step to supporting the Firebug Console API (see Bug 14354) is to implement window.console in WebCore.  (Currently, both Safari for Windows and Safari for Mac OS X implement it within the browser.)

Will attach a patch shortly.

[David Kilzer (:ddkilzer)]

Created attachment 15943 [details]
Patch v1

Initial implementation of window.console in WebCore.

[David Kilzer (:ddkilzer)]

Comment on attachment 15943 [details]
Patch v1

Maciej says:

We've used shadow/shadowing to refer specifically to overriding a built-in Window property with a var declaration, I think a better name for this concept would be "Replaceable".

[David Kilzer (:ddkilzer)]

Comment on attachment 15943 [details]
Patch v1

Wrong license applied to source files as well.  Should be BSD-style, not LGPL.

[Sam Weinig]

Created attachment 15944 [details]
Updated patch [WIP]

Adding an updated patch that is not quite ready for review.  This moves Console.{h,cpp,idl} to WebCore/page/ with the rest of the window objects, fixes the licenses, fixes some style issues and changes the extended attribute to Replaceable.

[David Kilzer (:ddkilzer)]

(In reply to comment #4)
> Created an attachment (id=15944) [edit]
> Updated patch [WIP]

WebCore.pro and WebCore.vcproj changes were lost from previous patch.

[Sam Weinig]

Created attachment 15946 [details]
Even more updated [Still WIP]

Adds WebCore.pro and WebCore.vcproj back

[David Kilzer (:ddkilzer)]

<rdar://problem/5539122>

[Timothy Hatcher]

+    // FIXME: this should use LogMessageLevel, but must use ErrorMessageLevel for now to forward message to the console.

etc.

We should pass the right message level to Crome and let Crome's addMessageToConsole do the right thing when calling the client. Passing the wrong message level is gimping the new Web Inspector console.

[Timothy Hatcher]

If you make this change:

Index: page/Chrome.cpp
===================================================================
--- page/Chrome.cpp	(revision 27149)
+++ page/Chrome.cpp	(working copy)
@@ -197,7 +197,7 @@
 
 void Chrome::addMessageToConsole(MessageSource source, MessageLevel level, const String& message, unsigned lineNumber, const String& sourceID)
 {
-    if (source == JSMessageSource && level == ErrorMessageLevel)
+    if (source == JSMessageSource)
         m_client->addMessageToConsole(message, lineNumber, sourceID);
 
     if (InspectorController* inspector = m_page->inspectorController())

Then you can pass all the correct message levels.

[Adam Roben (:aroben)]

(In reply to comment #8)
> +    // FIXME: this should use LogMessageLevel, but must use ErrorMessageLevel
> for now to forward message to the console.
> 
> etc.
> 
> We should pass the right message level to Crome and let Crome's
> addMessageToConsole do the right thing when calling the client. Passing the
> wrong message level is gimping the new Web Inspector console.

When I added the message level/source stuff I was being pretty paranoid about not changing the set of messages passed up to the ChromeClient. I think we can be less paranoid now and do as you suggest.

[Sam Weinig]

Created attachment 16904 [details]
patch

[Adam Roben (:aroben)]

Comment on attachment 16904 [details]
patch

It might be nice to add a test that actually called methods on the Console object.

r=me

[Sam Weinig]

Landed in r27161

[Timothy Hatcher]

*** Bug 14263 has been marked as a duplicate of this bug. ***