Bug 20904

Summary: gracefully handle too many console messages.
Product: WebKit Reporter: Kevin McCullough <kmccullough>
Component: Web Inspector (Deprecated)Assignee: Kevin McCullough <kmccullough>
Severity: Normal Keywords: InRadar
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.5   
URL: http://www.boulder.nist.gov/div853/greenfn/tutorial.html
Description Flags
Patch and manual test
timothy: review-
New Patch with test timothy: review+

Description Kevin McCullough 2008-09-17 14:39:17 PDT
From the radar:

Safari, when opening the page 
goes bezerk with its error generation.
Not only does this drown out all other messages, it takes up a ton of CPU power and disk space, and to add insult to injury, it seems to happen not just one when the page is opened, but constantly while the page is active.

this seems like it would be a situation in which you'd like some kind of message that says "too many errors, can't proceed" kinda like how a compiler would.  changing this to enhancement request because this problem doesn't seem to affect the usability and responsiveness of Safari or the Web Inspector.

9/11/08 10:41 AM Kevin McCullough:
There are a couple of solutions to this.  First, we can group repeated messages and just let the user know how many were duplicated.  Second, we could have a cap on how many total messages we show and let it be changeable by the user.

I have  a potential patch for the first suggestion
Comment 1 Kevin McCullough 2008-09-17 14:40:39 PDT
Comment 2 Kevin McCullough 2008-09-17 14:43:32 PDT
Created attachment 23510 [details]
Patch and manual test
Comment 3 Timothy Hatcher 2008-09-17 15:01:48 PDT
Comment on attachment 23510 [details]
Patch and manual test

+        //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?
Comment 4 Kevin McCullough 2008-09-17 15:15:05 PDT
-.resource-sidebar-tree-item .bubble.warning {
+.resource-sidebar-tree-item .bubble.warning, .bubble.warning {

Removing ".resource-sidebar-tree-item" did not propagate the style, I'm a little fuzzy on CSS but I think this means some other style takes priority?

+.console-error-level .console-message-text, .console-error-level-with-count {
     color: red;

This is the color of the text in an error message.  I added one more style rule because the next rule in this file was for ". console-error-level::before" and added the red X.  I don't want that image so I needed a new rule.

Did you want a better value for "red"?  An RGB?  This was this way before.
Comment 5 Kevin McCullough 2008-09-17 16:25:20 PDT
Created attachment 23513 [details]
New Patch with test
Comment 6 Timothy Hatcher 2008-09-17 16:40:59 PDT
Comment on attachment 23513 [details]
New Patch with test

I would rename lastMessage to previousMessageElement to better match previousMessage, otherwise it is confusing. 


Make sure you set review to ? when posting a patch to Bugzilla so it shows up in the review queue.
Comment 7 Kevin McCullough 2008-09-18 10:33:47 PDT
Committed revision 36615, which groups repeated messages.

That seems to handle the memory issue too.