19131 2008-05-19 19:20:50 -0700 Can't remove a breakpoint 2009-10-19 23:05:53 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Major --- 1 timothy webkit-unassigned aroben eric joepeck pmuellr rik timothy oldest_to_newest 80940 0 timothy 2008-05-19 19:20:50 -0700 You can only disable/enable a breakpoint. Need to be able to remove breakpoints. Drag and poof away? 114486 1 pmuellr 2009-03-20 06:56:20 -0700 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 139370 2 pmuellr 2009-08-12 09:44:14 -0700 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)); }, 139402 3 timothy 2009-08-12 10:33:31 -0700 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. 139489 4 34682 pmuellr 2009-08-12 13:01:47 -0700 Created attachment 34682 proposed patch patch as proposed in comment 2 139565 5 34682 timothy 2009-08-12 14:56:29 -0700 Comment on attachment 34682 proposed patch You will need to remove the "No new tests" line other wise the patch wont commit. 139835 6 34739 pmuellr 2009-08-13 07:29:41 -0700 Created attachment 34739 patch with "no test" changelog comment removed removed the no test line from the ChangeLog entry 139869 7 34739 eric 2009-08-13 09:29:57 -0700 Comment on attachment 34739 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 139870 8 eric 2009-08-13 09:30:01 -0700 All reviewed patches have been landed. Closing bug. 156072 9 joepeck 2009-10-19 23:05:53 -0700 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? 34682 2009-08-12 13:01:47 -0700 2009-08-13 07:29:41 -0700 proposed patch 19131.patch text/plain 1601 pmuellr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NzE0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMDktMDgtMTIgIFBhdHJpY2sgTXVlbGxlciAgPFBhdHJpY2tfTXVl bGxlckB1cy5pYm0uY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIEFkZCBhIG5ldyBnZXN0dXJlIGluIFdlYiBJbnNwZWN0b3IgdG8gcmVtb3ZlIGJy ZWFrcG9pbnRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xOTEzMQorICAgICAgICAKKyAgICAgICAgTm8gbmV3IHRlc3RzLiAoT09QUyEpCisKKyAgICAg ICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL1NvdXJjZUZyYW1lLmpzOgorICAgICAgICAoV2ViSW5z cGVjdG9yLlNvdXJjZUZyYW1lLnByb3RvdHlwZS5fZG9jdW1lbnRNb3VzZURvd24pOgorCiAyMDA5 LTA4LTEyICBMeW9uIENoZW4gIDxseW9uLmNoZW5AdG9yY2htb2JpbGUuY29tPgogCiAgICAgICAg IFJldmlld2VkIGJ5IEdlb3JnZSBTdGFpa29zLgpJbmRleDogV2ViQ29yZS9pbnNwZWN0b3IvZnJv bnQtZW5kL1NvdXJjZUZyYW1lLmpzCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvaW5zcGVjdG9yL2Zy b250LWVuZC9Tb3VyY2VGcmFtZS5qcwkocmV2aXNpb24gNDcwNjApCisrKyBXZWJDb3JlL2luc3Bl Y3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanMJKHdvcmtpbmcgY29weSkKQEAgLTI4OSw4ICsy ODksMTAgQEAgV2ViSW5zcGVjdG9yLlNvdXJjZUZyYW1lLnByb3RvdHlwZSA9IHsKICAgICAgICAg ICAgIHJldHVybjsKIAogICAgICAgICB2YXIgc291cmNlUm93ID0gZXZlbnQudGFyZ2V0LmVuY2xv c2luZ05vZGVPclNlbGZXaXRoTm9kZU5hbWUoInRyIik7Ci0gICAgICAgIGlmIChzb3VyY2VSb3cu X2JyZWFrcG9pbnRPYmplY3QpCi0gICAgICAgICAgICBzb3VyY2VSb3cuX2JyZWFrcG9pbnRPYmpl Y3QuZW5hYmxlZCA9ICFzb3VyY2VSb3cuX2JyZWFrcG9pbnRPYmplY3QuZW5hYmxlZDsKKyAgICAg ICAgaWYgKHNvdXJjZVJvdy5fYnJlYWtwb2ludE9iamVjdCAmJiBzb3VyY2VSb3cuX2JyZWFrcG9p bnRPYmplY3QuZW5hYmxlZCkKKyAgICAgICAgICAgIHNvdXJjZVJvdy5fYnJlYWtwb2ludE9iamVj dC5lbmFibGVkID0gZmFsc2U7CisgICAgICAgIGVsc2UgaWYgKHNvdXJjZVJvdy5fYnJlYWtwb2lu dE9iamVjdCkgCisgICAgICAgICAgICBXZWJJbnNwZWN0b3IucGFuZWxzLnNjcmlwdHMucmVtb3Zl QnJlYWtwb2ludChzb3VyY2VSb3cuX2JyZWFrcG9pbnRPYmplY3QpOwogICAgICAgICBlbHNlIGlm ICh0aGlzLmFkZEJyZWFrcG9pbnREZWxlZ2F0ZSkKICAgICAgICAgICAgIHRoaXMuYWRkQnJlYWtw b2ludERlbGVnYXRlKHRoaXMubGluZU51bWJlckZvclNvdXJjZVJvdyhzb3VyY2VSb3cpKTsKICAg ICB9LAo= 34739 2009-08-13 07:29:41 -0700 2009-08-13 09:29:57 -0700 patch with "no test" changelog comment removed 19131.patch text/plain 1568 pmuellr SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA0NzE0MCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTMgQEAKKzIwMDktMDgtMTIgIFBhdHJpY2sgTXVlbGxlciAgPFBhdHJpY2tfTXVl bGxlckB1cy5pYm0uY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgor CisgICAgICAgIEFkZCBhIG5ldyBnZXN0dXJlIGluIFdlYiBJbnNwZWN0b3IgdG8gcmVtb3ZlIGJy ZWFrcG9pbnRzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xOTEzMQorICAgICAgICAKKyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL1NvdXJjZUZy YW1lLmpzOgorICAgICAgICAoV2ViSW5zcGVjdG9yLlNvdXJjZUZyYW1lLnByb3RvdHlwZS5fZG9j dW1lbnRNb3VzZURvd24pOgorCiAyMDA5LTA4LTEyICBMeW9uIENoZW4gIDxseW9uLmNoZW5AdG9y Y2htb2JpbGUuY29tPgogCiAgICAgICAgIFJldmlld2VkIGJ5IEdlb3JnZSBTdGFpa29zLgpJbmRl eDogV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL1NvdXJjZUZyYW1lLmpzCj09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0K LS0tIFdlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9Tb3VyY2VGcmFtZS5qcwkocmV2aXNpb24g NDcwNjApCisrKyBXZWJDb3JlL2luc3BlY3Rvci9mcm9udC1lbmQvU291cmNlRnJhbWUuanMJKHdv cmtpbmcgY29weSkKQEAgLTI4OSw4ICsyODksMTAgQEAgV2ViSW5zcGVjdG9yLlNvdXJjZUZyYW1l LnByb3RvdHlwZSA9IHsKICAgICAgICAgICAgIHJldHVybjsKIAogICAgICAgICB2YXIgc291cmNl Um93ID0gZXZlbnQudGFyZ2V0LmVuY2xvc2luZ05vZGVPclNlbGZXaXRoTm9kZU5hbWUoInRyIik7 Ci0gICAgICAgIGlmIChzb3VyY2VSb3cuX2JyZWFrcG9pbnRPYmplY3QpCi0gICAgICAgICAgICBz b3VyY2VSb3cuX2JyZWFrcG9pbnRPYmplY3QuZW5hYmxlZCA9ICFzb3VyY2VSb3cuX2JyZWFrcG9p bnRPYmplY3QuZW5hYmxlZDsKKyAgICAgICAgaWYgKHNvdXJjZVJvdy5fYnJlYWtwb2ludE9iamVj dCAmJiBzb3VyY2VSb3cuX2JyZWFrcG9pbnRPYmplY3QuZW5hYmxlZCkKKyAgICAgICAgICAgIHNv dXJjZVJvdy5fYnJlYWtwb2ludE9iamVjdC5lbmFibGVkID0gZmFsc2U7CisgICAgICAgIGVsc2Ug aWYgKHNvdXJjZVJvdy5fYnJlYWtwb2ludE9iamVjdCkgCisgICAgICAgICAgICBXZWJJbnNwZWN0 b3IucGFuZWxzLnNjcmlwdHMucmVtb3ZlQnJlYWtwb2ludChzb3VyY2VSb3cuX2JyZWFrcG9pbnRP YmplY3QpOwogICAgICAgICBlbHNlIGlmICh0aGlzLmFkZEJyZWFrcG9pbnREZWxlZ2F0ZSkKICAg ICAgICAgICAgIHRoaXMuYWRkQnJlYWtwb2ludERlbGVnYXRlKHRoaXMubGluZU51bWJlckZvclNv dXJjZVJvdyhzb3VyY2VSb3cpKTsKICAgICB9LAo=