[webkit-reviews] review denied: [Bug 28908] Support conditional breakpoints in the frontend : [Attachment 39545] patch (fixed)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 14 09:52:23 PDT 2009


Timothy Hatcher <timothy at hatcher.name> has denied apavlov at chromium.org's
request for review:
Bug 28908: Support conditional breakpoints in the frontend
https://bugs.webkit.org/show_bug.cgi?id=28908

Attachment 39545: patch (fixed)
https://bugs.webkit.org/attachment.cgi?id=39545&action=review

------- Additional Comments from Timothy Hatcher <timothy at hatcher.name>
> +WebInspector.Popup.prototype =
> +{

The brace for prototypes should be next to the equals on one line to match the
other classes.

> +	   var popupDoc = this.element.contentDocument;

We don't abbreviate Document as Doc in WebKit. This should be popupDocument.

> +.breakpoint-list a.breakpoint-has-condition::after {
> +    content: " (?)";
> +}

I now see what this is for. I don't think this is the best way to show
conditional break points. I think we should show the condition somehow in the
Breakpoints pane. (This also isn't localizable in CSS.)

Remove this for now and file a bug about showing them in the Breakpoints pane.


More information about the webkit-reviews mailing list