[webkit-reviews] review denied: [Bug 33217] Web Inspector: there should be a way to "deactivate" or "skip" all breakpoints while debugging. : [Attachment 49005] [PATCH] Proposed solution with English.lproj

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Feb 18 07:17:22 PST 2010


Pavel Feldman <pfeldman at chromium.org> has denied Alexander Pavlov (apavlov)
<apavlov at chromium.org>'s request for review:
Bug 33217: Web Inspector: there should be a way to "deactivate" or "skip" all
breakpoints while debugging.
https://bugs.webkit.org/show_bug.cgi?id=33217

Attachment 49005: [PATCH] Proposed solution with English.lproj
https://bugs.webkit.org/attachment.cgi?id=49005&action=review

------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
> +void JavaScriptDebugServer::setBreakpointsActivated(bool activated)

I'd use (de)activateBreakpoints pair of methods here. Or setBreakpointsActive.
I personally don't like Activated, leaving to Timothy.
> +	   if (WebInspector.panels.scripts)
> +	      
WebInspector.panels.scripts.addEventListener("breakpointsActivationChanged",
this._breakpointsActivationChanged, this);

ScriptsPanel has a pointer to the active view, so you don't need listener-based
indirection here. r- is for this.

> +    // Breakpoints should be activated by default, so emulate a click to
toggle on.
> +    this._deactivateClicked();

This sounds confusing. By default should be activated, so let us call
"deactivate".


More information about the webkit-reviews mailing list