[webkit-reviews] review denied: [Bug 30324] Console shows no repeat count when repeatedly logging an Event : [Attachment 41509] proposed patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Oct 20 15:22:57 PDT 2009


Pavel Feldman <pfeldman at chromium.org> has denied Keishi Hattori
<casey.hattori at gmail.com>'s request for review:
Bug 30324: Console shows no repeat count when repeatedly logging an Event
https://bugs.webkit.org/show_bug.cgi?id=30324

Attachment 41509: proposed patch
https://bugs.webkit.org/attachment.cgi?id=41509&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
>	       && (this.url == msg.url)
> -	       && (this.message == msg.message);
> +	       && (this.message == msg.message)
> +	       && (this.repeatCount > msg.repeatCount);

This does not look right: messages are equal when their properties are equal,
whereas this check should be made external. We are already handle isEqual in
InspectorController, I don't think we should do it in the frontend as well.
They way I would solve it is following: Split
InspectorFrontend::addMessageToConsole into
InspectorFrontend::addConsoleMessage(msg) and
InspectorFrontend::updateConsoleMessageCount(count). Call latter when you
increment count from inspector controller; Nuke this isEqual you are modifying
as a whole.


More information about the webkit-reviews mailing list