[webkit-reviews] review denied: [Bug 17229] Inspector should show properties of all JS objects in Console : [Attachment 26310] better patch which doesn't include the bogus select field

[bugzilla-daemon at webkit.org]

Timothy Hatcher <timothy at hatcher.name> has denied Konstantin Käfer
<kkaefer at gmail.com>'s request for review:
Bug 17229: Inspector should show properties of all JS objects in Console
https://bugs.webkit.org/show_bug.cgi?id=17229

Attachment 26310: better patch which doesn't include the bogus select field
https://bugs.webkit.org/attachment.cgi?id=26310&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
Patch looks great! Some minor comments. r- for ow for some clean up.

> -		  
formattedResult.appendChild(formatForConsole(parameters[i]));
> +		  
formattedResult.appendChild(WebInspector.console._format(parameters[i]));

Why this change? The formatForConsole function just calls
WebInspector.console._format.

> +	   if (x instanceof Node) {
> +	       while (this.titleElement.firstChild) {
> +		   this.titleElement.removeChild(this.titleElement.firstChild);

> +	       }

You should use this.titleElement.removeChildren() instead of the while loop (a
prototype extension we have in utilities.js.)

> +	   }
> +	   else {

The "else {" should be on the previous line, so it is "} else {".

> +.console-formatted-object .section, .console-formatted-node .section {
> +  position: static;
> +}
> +

There should be 4 spaces here for the indent, not 2.