[webkit-reviews] review denied: [Bug 20904] gracefully handle too many console messages. : [Attachment 23510] Patch and manual test

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Sep 17 15:01:48 PDT 2008


Timothy Hatcher <timothy at hatcher.name> has denied 's request for review:
Bug 20904: gracefully handle too many console messages.
https://bugs.webkit.org/show_bug.cgi?id=20904

Attachment 23510: Patch and manual test
https://bugs.webkit.org/attachment.cgi?id=23510&action=edit

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
+	 //call a function here, badge with incremental

This comment isn't good, and doesn't help me understand the code. Remove it or
make it better.

+	 var repeatedMessage = this.currentGroup.messagesElement.lastChild;
+	 if (!repeatedMessage)
+	     return;

This is risky and prone to being broken. The Console object should store a
reference to the last message element (like previousMessage but
previousMessageElement). Then clearMessages would have to clear that reference
too.

+	     repeatedMessage.style['padding-left'] = "6px";

You should add a new class to the repeated message so the "6px" can be in the
CSS and not in the code. Also we don't use single quotes in Inspector JS.

-.resource-sidebar-tree-item .bubble.warning {
+.resource-sidebar-tree-item .bubble.warning, .bubble.warning {

Just remove .resource-sidebar-tree-item.

+.console-error-level .console-message-text, .console-error-level-with-count {
     color: red;
 }
 
What is this? What is red?


More information about the webkit-reviews mailing list