Bug 19131 – Can't remove a breakpoint

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Timothy Hatcher]

You can only disable/enable a breakpoint. Need to be able to remove breakpoints. Drag and poof away?

[Patrick Mueller]

Wouldn't it be easier to just make this a tri-state?  For a statement with no breakpoint, consecutive clicking puts the line into a - break point enabled / break point disabled / break point removed - states

[Patrick Mueller]

The inability to delete breakpoints continues to bug me.  At one point recently, one of my source files was just splattered with blue turds as I kept moving breakpoints around, reloading the file, etc.

Here's a patch to change the enable/disable breakpoint click states to enable/disable/remove.  ie, as you click on the line number in a source view, the breakpoint gets added, then disabled, then removed, then added, ...

This seems fairly natural to me.  Another option would be to only add/remove via the click, and allow enable/disable in the new breakpoints sidebar.  Not clear whether removing breakpoints from the sidebar is required - I think I'd argue that if we want to add that, we may well want more function in the sidebar like "disable all/remove all", etc, and so should look at "remove a breakpoint from sidebar" as part of a larger design effort for that sidebar.

I'm happy to kosher this up as a patch, if folks agree the current patch is useful.

Index: WebCore/inspector/front-end/SourceFrame.js
===================================================================
--- WebCore/inspector/front-end/SourceFrame.js	(revision 47011)
+++ WebCore/inspector/front-end/SourceFrame.js	(working copy)
@@ -289,8 +289,10 @@
             return;
 
         var sourceRow = event.target.enclosingNodeOrSelfWithNodeName("tr");
-        if (sourceRow._breakpointObject)
-            sourceRow._breakpointObject.enabled = !sourceRow._breakpointObject.enabled;
+        if (sourceRow._breakpointObject && sourceRow._breakpointObject.enabled)
+            sourceRow._breakpointObject.enabled = false;
+        else if (sourceRow._breakpointObject) 
+            WebInspector.panels.scripts.removeBreakpoint(sourceRow._breakpointObject);
         else if (this.addBreakpointDelegate)
             this.addBreakpointDelegate(this.lineNumberForSourceRow(sourceRow));
     },

[Timothy Hatcher]

Either option seems fine to me. The ideal design that hasn't been implemented yet, would be to allow dragging a breakpoint to move it and dragging out oif the gutter will "poof" it away, like Xcode.

[Patrick Mueller]

Created attachment 34682 [details]
proposed patch

patch as proposed in comment 2

[Timothy Hatcher]

Comment on attachment 34682 [details]
proposed patch

You will need to remove the "No new tests" line other wise the patch wont commit.

[Patrick Mueller]

Created attachment 34739 [details]
patch with "no test" changelog comment removed

removed the no test line from the ChangeLog entry

[Eric Seidel (no email)]

Comment on attachment 34739 [details]
patch with "no test" changelog comment removed

Clearing flags on attachment: 34739

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/SourceFrame.js
Committed r47202
	M	WebCore/ChangeLog
	M	WebCore/inspector/front-end/SourceFrame.js
r47202 = cc0d301a171511657105756961620615f89def37 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/47202

[Eric Seidel (no email)]

All reviewed patches have been landed.  Closing bug.

[Joseph Pecoraro]

Excellent!  The submitted patch made it so breakpoints can be removed if you click on their "blue tags" enough times.  How about being able to delete them directly from the Breakpoints Sidebar Pane?  Or would the expected user scenario be:

1. Click the entry in the Breakpoints Sidebar Pane to jump directly to the line of code with the blue tag.
2. Click on the tag to disable and then remove.

How about unifying a UI for something like this with the UI for deleting a Watched Expression?