163403 2016-10-13 14:23:54 -0700 Web Inspector: Dragging to delete a Breakpoint should never trigger a ContentView change 2016-10-14 16:13:17 -0700 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 joepeck joepeck bburg commit-queue joepeck mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1239946 0 joepeck 2016-10-13 14:23:54 -0700 Summary: Dragging to delete a Breakpoint should never trigger a ContentView change Steps to Reproduce: 1. Inspect http://bogojoker.com/shell/ 2. Set a breakpoint in easySlider.min.js 3. Set a breakpoint in facebox.js 4. Show Debugger Tab 5. Select the facebox.js breakpoint in Debugger Sidebar 6. Drag to delete the Breakpoint from the TextEditor's gutter => The breakpoint is deleted but the ContentView unexpectedly changes to easySlider.min.js Notes: - When deleting BreakpointTreeElements from the sidebar via the Delete keyboard shortcuts I would expect ContentView changes - When deleting BreakpointTreeElements in other ways (Context Menu, Drag + Delete from gutter) I do not expect ContentView changes 1239947 1 webkit-bug-importer 2016-10-13 14:24:29 -0700 <rdar://problem/28762930> 1239949 2 291516 joepeck 2016-10-13 14:31:50 -0700 Created attachment 291516 [PATCH] Proposed Fix 1240439 3 291516 timothy 2016-10-14 15:41:08 -0700 Comment on attachment 291516 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=291516&action=review > Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:593 > + // If this is a BreakpointTreeElement being deleted because of a cause > + // outside of the TreeOutline then deselect if it is selected to avoid > + // TreeOutline selection changes causing unexpected ContentView changes. > + if (!debuggerTreeElement.__deletedViaDeleteKeyboardShortcut) > + debuggerTreeElement.deselect(); Maybe we should select the parent so the resource is selected to match the visible view? 1240444 4 291516 joepeck 2016-10-14 15:49:50 -0700 Comment on attachment 291516 [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=291516&action=review >> Source/WebInspectorUI/UserInterface/Views/DebuggerSidebarPanel.js:593 >> + debuggerTreeElement.deselect(); > > Maybe we should select the parent so the resource is selected to match the visible view? If this is the last BreakpointTreeElement then the parent (ResourceTreeElement) will be removed. We could select the resource in the resources TreeOutline but that might be offscreen which means, should we scroll to it? No selection is actually what I felt best with. 1240462 5 291516 commit-queue 2016-10-14 16:13:13 -0700 Comment on attachment 291516 [PATCH] Proposed Fix Clearing flags on attachment: 291516 Committed r207359: <http://trac.webkit.org/changeset/207359> 1240463 6 commit-queue 2016-10-14 16:13:17 -0700 All reviewed patches have been landed. Closing bug. 291516 2016-10-13 14:31:50 -0700 2016-10-14 16:13:13 -0700 [PATCH] Proposed Fix breakpoint-change-1.patch text/plain 3478 joepeck ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IGZjNTRlZGEuLmJmYmI2OGYgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSw1ICsxLDI2IEBACiAyMDE2LTEwLTEzICBKb3NlcGggUGVjb3Jh cm8gIDxwZWNvcmFyb0BhcHBsZS5jb20+CiAKKyAgICAgICAgV2ViIEluc3BlY3RvcjogRHJhZ2dp bmcgdG8gZGVsZXRlIGEgQnJlYWtwb2ludCBzaG91bGQgbmV2ZXIgdHJpZ2dlciBhIENvbnRlbnRW aWV3IGNoYW5nZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/ aWQ9MTYzNDAzCisgICAgICAgIDxyZGFyOi8vcHJvYmxlbS8yODc2MjkzMD4KKworICAgICAgICBS ZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmll d3MvQnJlYWtwb2ludFRyZWVFbGVtZW50LmpzOgorICAgICAgICAoV2ViSW5zcGVjdG9yLkJyZWFr cG9pbnRUcmVlRWxlbWVudC5wcm90b3R5cGUub25kZWxldGUpOgorICAgICAgICBTaWduYWwgd2hl biBhIGJyZWFrcG9pbnQgdHJlZSBlbGVtZW50IHdpbGwgYmUgZ29pbmcgYXdheSBiZWNhdXNlIGl0 IHdhcworICAgICAgICBkZWxldGVkIHZpYSB0aGUga2V5Ym9hcmQgb3BlcmF0aW9uIHdpdGhpbiB0 aGUgVHJlZU91dGxpbmUuIFRoaXMgaXMgYSBkaXJ0eQorICAgICAgICB3YXkgdG8gZG8gdGhlIHNp Z25hbCBidXQgd2UgcmVtb3ZlIEJyZWFrcG9pbnRUcmVlRWxlbWVudHMgYXN5bmNocm9ub3VzbHkK KyAgICAgICAgd2hlbiB0aGUgQnJlYWtwb2ludCBhY3R1YWxseSBnZXRzIHJlbW92ZWQgZnJvbSB0 aGUgYmFja2VuZC4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvRGVidWdnZXJTaWRl YmFyUGFuZWwuanM6CisgICAgICAgIChXZWJJbnNwZWN0b3IuRGVidWdnZXJTaWRlYmFyUGFuZWwu cHJvdG90eXBlLl9yZW1vdmVEZWJ1Z2dlclRyZWVFbGVtZW50KToKKyAgICAgICAgRGVzZWxlY3Qg YSBCcmVha3BvaW50VHJlZUVsZW1lbnQgaWYgaXQgd2FzIGRlbGV0ZWQgaW4gYSB3YXkgb3RoZXIg dGhlbgorICAgICAgICB0aGUgZGVsZXRlIGtleWJvYXJkIHNob3J0Y3V0LiBUaGlzIGVuc3VyZXMg YW5vdGhlciBUcmVlRWxlbWVudCBzZWxlY3Rpb24KKyAgICAgICAgZG9lc24ndCBmb3JjZSBDb250 ZW50VmlldyBjaGFuZ2VzLgorCisyMDE2LTEwLTEzICBKb3NlcGggUGVjb3Jhcm8gIDxwZWNvcmFy b0BhcHBsZS5jb20+CisKICAgICAgICAgV2ViIEluc3BlY3RvcjogQ2xlYW51cCBwYXJ0cyBvZiBE ZWJ1Z2dlck1hbmFnZXIKICAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTE2MzQwMAogCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlcklu dGVyZmFjZS9WaWV3cy9CcmVha3BvaW50VHJlZUVsZW1lbnQuanMgYi9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9CcmVha3BvaW50VHJlZUVsZW1lbnQuanMKaW5kZXgg YjM2ZjUxMS4uMTU0ZTUzYSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJ bnRlcmZhY2UvVmlld3MvQnJlYWtwb2ludFRyZWVFbGVtZW50LmpzCisrKyBiL1NvdXJjZS9XZWJJ bnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL0JyZWFrcG9pbnRUcmVlRWxlbWVudC5qcwpA QCAtODAsNiArODAsMTEgQEAgV2ViSW5zcGVjdG9yLkJyZWFrcG9pbnRUcmVlRWxlbWVudCA9IGNs YXNzIEJyZWFrcG9pbnRUcmVlRWxlbWVudCBleHRlbmRzIFdlYkluc3AKICAgICAgICAgaWYgKCFX ZWJJbnNwZWN0b3IuZGVidWdnZXJNYW5hZ2VyLmlzQnJlYWtwb2ludFJlbW92YWJsZSh0aGlzLl9i cmVha3BvaW50KSkKICAgICAgICAgICAgIHJldHVybiBmYWxzZTsKIAorICAgICAgICAvLyBXZSBz ZXQgdGhpcyBmbGFnIHNvIHRoYXQgVHJlZU91dGxpbmVzIHRoYXQgd2lsbCByZW1vdmUgdGhpcwor ICAgICAgICAvLyBCcmVha3BvaW50VHJlZUVsZW1lbnQgd2lsbCBrbm93IHdoZXRoZXIgaXQgd2Fz IGRlbGV0ZWQgZnJvbQorICAgICAgICAvLyB3aXRoaW4gdGhlIFRyZWVPdXRsaW5lIG9yIGZyb20g b3V0c2lkZSBpdCAoZS5nLiBUZXh0RWRpdG9yKS4KKyAgICAgICAgdGhpcy5fX2RlbGV0ZWRWaWFE ZWxldGVLZXlib2FyZFNob3J0Y3V0ID0gdHJ1ZTsKKwogICAgICAgICBXZWJJbnNwZWN0b3IuZGVi dWdnZXJNYW5hZ2VyLnJlbW92ZUJyZWFrcG9pbnQodGhpcy5fYnJlYWtwb2ludCk7CiAgICAgICAg IHJldHVybiB0cnVlOwogICAgIH0KZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9V c2VySW50ZXJmYWNlL1ZpZXdzL0RlYnVnZ2VyU2lkZWJhclBhbmVsLmpzIGIvU291cmNlL1dlYklu c3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvRGVidWdnZXJTaWRlYmFyUGFuZWwuanMKaW5k ZXggMjMyMGIxZS4uMjkyYTRhZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1Vz ZXJJbnRlcmZhY2UvVmlld3MvRGVidWdnZXJTaWRlYmFyUGFuZWwuanMKKysrIGIvU291cmNlL1dl Ykluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvRGVidWdnZXJTaWRlYmFyUGFuZWwuanMK QEAgLTU4Niw3ICs1ODYsMTMgQEAgV2ViSW5zcGVjdG9yLkRlYnVnZ2VyU2lkZWJhclBhbmVsID0g Y2xhc3MgRGVidWdnZXJTaWRlYmFyUGFuZWwgZXh0ZW5kcyBXZWJJbnNwZWMKIAogICAgIF9yZW1v dmVEZWJ1Z2dlclRyZWVFbGVtZW50KGRlYnVnZ2VyVHJlZUVsZW1lbnQpCiAgICAgewotICAgICAg ICB2YXIgcGFyZW50VHJlZUVsZW1lbnQgPSBkZWJ1Z2dlclRyZWVFbGVtZW50LnBhcmVudDsKKyAg ICAgICAgLy8gSWYgdGhpcyBpcyBhIEJyZWFrcG9pbnRUcmVlRWxlbWVudCBiZWluZyBkZWxldGVk IGJlY2F1c2Ugb2YgYSBjYXVzZQorICAgICAgICAvLyBvdXRzaWRlIG9mIHRoZSBUcmVlT3V0bGlu ZSB0aGVuIGRlc2VsZWN0IGlmIGl0IGlzIHNlbGVjdGVkIHRvIGF2b2lkCisgICAgICAgIC8vIFRy ZWVPdXRsaW5lIHNlbGVjdGlvbiBjaGFuZ2VzIGNhdXNpbmcgdW5leHBlY3RlZCBDb250ZW50Vmll dyBjaGFuZ2VzLgorICAgICAgICBpZiAoIWRlYnVnZ2VyVHJlZUVsZW1lbnQuX19kZWxldGVkVmlh RGVsZXRlS2V5Ym9hcmRTaG9ydGN1dCkKKyAgICAgICAgICAgIGRlYnVnZ2VyVHJlZUVsZW1lbnQu ZGVzZWxlY3QoKTsKKworICAgICAgICBsZXQgcGFyZW50VHJlZUVsZW1lbnQgPSBkZWJ1Z2dlclRy ZWVFbGVtZW50LnBhcmVudDsKICAgICAgICAgcGFyZW50VHJlZUVsZW1lbnQucmVtb3ZlQ2hpbGQo ZGVidWdnZXJUcmVlRWxlbWVudCk7CiAKICAgICAgICAgY29uc29sZS5hc3NlcnQocGFyZW50VHJl ZUVsZW1lbnQucGFyZW50ID09PSB0aGlzLl9icmVha3BvaW50c0NvbnRlbnRUcmVlT3V0bGluZSk7 Cg==