[webkit-reviews] review denied: [Bug 29336] WebInspector.log() function not protected if console not yet created : [Attachment 39848] proposed patch - 2009/09/21

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Patrick Mueller
<pmuellr at yahoo.com>'s request for review:
Bug 29336: WebInspector.log() function not protected if console not yet created
https://bugs.webkit.org/show_bug.cgi?id=29336

Attachment 39848: proposed patch - 2009/09/21
https://bugs.webkit.org/attachment.cgi?id=39848&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>

> +	       var func = function() {WebInspector.log(null)};
> +	       WebInspector.log.interval = setInterval(func, 1000);

> +    // ignore null messages, specifically the one sent by queued timer
> +    if (!message)
> +	   return;

I am not sure passing "null" is the best way to check. What if I want to log
somthing and it happens to be undefined/null, I would like you know that. Just
make the check function do the "WebInspector.ConsoleMessage && this.console"
test and then replay the messages if that passes.

Also just pass the function inline to setInterval or give the variable a better
name than func. Put spaces inside the curly braces too.


> +	   WebInspector.log.interval = null;

A better way to write this is:

delete WebInspector.log.interval;


> +	   for (var i = 0; i < queued.length; ++i) {
> +	       WebInspector.log(queued[i]);
> +	   }

No need for the braces.