Summary: | gracefully handle too many console messages. | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Kevin McCullough <kmccullough> | ||||||
Component: | Web Inspector (Deprecated) | Assignee: | Kevin McCullough <kmccullough> | ||||||
Status: | RESOLVED FIXED | ||||||||
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 | ||||||||
Attachments: |
|
Description
Kevin McCullough
2008-09-17 14:39:17 PDT
Created attachment 23510 [details]
Patch and manual test
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?
-.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. Created attachment 23513 [details]
New Patch with test
Comment on attachment 23513 [details]
New Patch with test
I would rename lastMessage to previousMessageElement to better match previousMessage, otherwise it is confusing.
r=me
Make sure you set review to ? when posting a patch to Bugzilla so it shows up in the review queue.
Committed revision 36615, which groups repeated messages. That seems to handle the memory issue too. |