[webkit-reviews] review granted: [Bug 17228] console.{log, warn,
info, error} should support format strings,
variable arguments : [Attachment 19935] Step 2: Route all
Console-related activities through Console
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Apr 2 02:41:58 PDT 2008
Darin Adler <darin at apple.com> has granted Adam Roben (aroben)
<aroben at apple.com>'s request for review:
Bug 17228: console.{log,warn,info,error} should support format strings,
variable arguments
http://bugs.webkit.org/show_bug.cgi?id=17228
Attachment 19935: Step 2: Route all Console-related activities through Console
http://bugs.webkit.org/attachment.cgi?id=19935&action=edit
------- Additional Comments from Darin Adler <darin at apple.com>
Why does the console() hang off domWindow()? Why not directly off Frame?
I think we should move the "shouldPrintExceptions" and printf stuff into the
console object too.
+void FrameLoader::reportLocalLoadFailed(const Frame* frame, const String& url)
The use of const Frame* here is strange. Just Frame* will do.
- void addMessageToConsole(MessageSource, MessageLevel, const String&
message, unsigned lineNumber, const String& sourceID);
+ // This method should only be called by Console
+ void addMessageToConsole(const String& message, unsigned lineNumber,
const String& sourceID);
I think Console should talk directly to the ChromeClient. I don't think we need
a Chrome function in between. Then you wouldn't need that comment. Also, please
don't call C++ member functions "methods".
It's extremely unconventional to use the name "state" for an ExecState*
parameter. We use "exec" almost everywhere. I think "state" might be better if
we did it everywhere, but I think it's not a great idea to start being original
about this in the console code.
r=me
More information about the webkit-reviews
mailing list