[webkit-reviews] review denied: [Bug 87644] Web Inspector: Option for selecting/deselecting all breakpoints in breakpoint pane : [Attachment 145710] Patch

[bugzilla-daemon at webkit.org]

Pavel Feldman <pfeldman at chromium.org> has denied sam <dsam2912 at gmail.com>'s
request for review:
Bug 87644: Web Inspector: Option for selecting/deselecting all breakpoints in
breakpoint pane
https://bugs.webkit.org/show_bug.cgi?id=87644

Attachment 145710: Patch
https://bugs.webkit.org/attachment.cgi?id=145710&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=145710&action=review


> Source/WebCore/inspector/front-end/BreakpointManager.js:142
> +    {

I don't quite like that you count and toggle breakpoints using different
collections. You could do it all in sidebar and toggle exactly what you count.

> Source/WebCore/inspector/front-end/BreakpointsSidebarPane.js:195
> +	       if (!enableBreakpointCount)

I don't think this logic is intuitive. You should add both actions at all times
but disable 'enable all' when all are already enabled, you should disable
'disable all' when everything is already disabled