112404 2013-03-14 22:12:51 -0700 Web Inspector: DataGrid should reveal and select next/previous DataGridNode upon deletion of selected node 2013-03-18 05:28:48 -0700 1 1 1 Unclassified WebKit Web Inspector (Deprecated) 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 0 sankeerth.vs webkit-unassigned apavlov keishi loislo pfeldman pmuellr vivekg vsevik web-inspector-bugs webkit.review.bot yurys oldest_to_newest 855692 0 sankeerth.vs 2013-03-14 22:12:51 -0700 While deleting the entries of the data grid by clicking on 'Delete' status bar button, deletion of selected node should trigger selection of next possible (backward/forward) DataGridNode. 855860 1 193272 sankeerth.vs 2013-03-15 03:33:50 -0700 Created attachment 193272 Patch 855950 2 193272 webkit.review.bot 2013-03-15 06:37:28 -0700 Comment on attachment 193272 Patch Clearing flags on attachment: 193272 Committed r145901: <http://trac.webkit.org/changeset/145901> 855951 3 webkit.review.bot 2013-03-15 06:37:32 -0700 All reviewed patches have been landed. Closing bug. 856835 4 vivekg 2013-03-17 22:35:37 -0700 In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience. I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly. @vsevik, I would be glad to have your views on the above comment. 856874 5 vsevik 2013-03-18 00:33:53 -0700 (In reply to comment #4) > In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. > > Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience. > > I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly. > > @vsevik, I would be glad to have your views on the above comment. Sounds resanobale, would you be willing to file a new bug a implement this? 857019 6 sankeerth.vs 2013-03-18 05:26:19 -0700 (In reply to comment #4) > In my opinion, node selection change should happen inherently within the DataGrid whenever a node is removed. Making an explicit call to changeNodeAfterDeletion() from the call site (e.g. _deleteButtonClicked) shouldn't be necessary. > > Also when the DataGrid is used in multiple views with the inline deleting option, and if the call site doesn't invoke changeNodeAfterDeletion() explicitly, the DataGrid behavior would be inconsistent with respect to other views and this would hamper the User experience. > > I would recommend that this method changeNodeAfterDeletion() be moved in the DataGridNode and make it a private API with more meaningful naming such as ( _nodeSelectionChanged() ) and should be invoked from WebInspector.DataGridNode.removeChild() implicitly. > > @vsevik, I would be glad to have your views on the above comment. Thank you for your valuable suggestions. What you mentioned is is right. I hadn't thought about it before. I shall incorporate the changes you have mentioned. 857021 7 sankeerth.vs 2013-03-18 05:28:48 -0700 > Sounds reasonable, would you be willing to file a new bug a implement this? Sure. I shall open a new bug and implement the above mentioned changes. Thank you. 193272 2013-03-15 03:33:50 -0700 2013-03-15 06:37:28 -0700 Patch bug-112404-20130315160229.patch text/plain 3930 sankeerth.vs U3VidmVyc2lvbiBSZXZpc2lvbjogMTQ1NjgyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2JhZjM1YzQ5M2I3MmM5 MDMzZjcwMDFjZDlhMmUwYTc2NDdlMzZmYS4uYTJhZjA1OWI1MGYyYjQzMDQyMzc2ZDk5OGY4Yzdk MWQ5MzQ1ZjNmNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDEzLTAzLTE0ICBTYW5r ZWVydGggViBTICA8c2Fua2VlcnRoLnZzQHNhbXN1bmcuY29tPgorCisgICAgICAgIERhdGFHcmlk IHNob3VsZCByZXZlYWwgYW5kIHNlbGVjdCBuZXh0L3ByZXZpb3VzIERhdGFHcmlkTm9kZSB1cG9u IGRlbGV0aW9uIG9mIHNlbGVjdGVkIG5vZGUKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTExMjQwNAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAo T09QUyEpLgorCisgICAgICAgIE5vIG5ldyB0ZXN0cyBhcyB0aGlzIGlzIGEgVUkgcmVsYXRlZCBj aGFuZ2UuCisKKyAgICAgICAgV2hpbGUgZGVsZXRpbmcgdGhlIGVudHJpZXMgb2YgdGhlIGRhdGEg Z3JpZCBieSBjbGlja2luZyBvbiAnRGVsZXRlJworICAgICAgICBzdGF0dXMgYmFyIGJ1dHRvbiwg ZGVsZXRpb24gb2Ygc2VsZWN0ZWQgbm9kZSBzaG91bGQgdHJpZ2dlciBzZWxlY3Rpb24gb2YKKyAg ICAgICAgbmV4dCBwb3NzaWJsZSAoYmFja3dhcmQvZm9yd2FyZCkgRGF0YUdyaWROb2RlLgorCisg ICAgICAgICogaW5zcGVjdG9yL2Zyb250LWVuZC9ET01TdG9yYWdlSXRlbXNWaWV3LmpzOgorICAg ICAgICAoV2ViSW5zcGVjdG9yLkRPTVN0b3JhZ2VJdGVtc1ZpZXcucHJvdG90eXBlLl9kZWxldGVC dXR0b25DbGlja2VkKToKKyAgICAgICAgKiBpbnNwZWN0b3IvZnJvbnQtZW5kL0RhdGFHcmlkLmpz OgorICAgICAgICAoV2ViSW5zcGVjdG9yLkRhdGFHcmlkLnByb3RvdHlwZS5fa2V5RG93bik6Cisg ICAgICAgIChXZWJJbnNwZWN0b3IuRGF0YUdyaWQucHJvdG90eXBlLmNoYW5nZU5vZGVBZnRlckRl bGV0aW9uKToKKwogMjAxMy0wMy0xMiAgQW50dGkgS29pdmlzdG8gIDxhbnR0aUBhcHBsZS5jb20+ CiAKICAgICAgICAgQmFzaWMgY2hpbGQgb2JzY3VyYXRpb24gdGVzdCBmb3IgYmFja2dyb3VuZHMK ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9mcm9udC1lbmQvRE9NU3RvcmFn ZUl0ZW1zVmlldy5qcyBiL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9mcm9udC1lbmQvRE9NU3Rv cmFnZUl0ZW1zVmlldy5qcwppbmRleCBlYmJkODc0MzU3MzZiOTcwYjgyY2VkYzdlNThiYmIyMmQ0 NDdlNDEzLi41YzdjODBiODU5YTUyYmZhYjY0M2EwY2NhMDY5ZTgyODdjYmNjZWY1IDEwMDY0NAot LS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL0RPTVN0b3JhZ2VJdGVtc1Zp ZXcuanMKKysrIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL2Zyb250LWVuZC9ET01TdG9yYWdl SXRlbXNWaWV3LmpzCkBAIC0yMTYsNiArMjE2LDcgQEAgV2ViSW5zcGVjdG9yLkRPTVN0b3JhZ2VJ dGVtc1ZpZXcucHJvdG90eXBlID0gewogICAgICAgICAgICAgcmV0dXJuOwogCiAgICAgICAgIHRo aXMuX2RlbGV0ZUNhbGxiYWNrKHRoaXMuX2RhdGFHcmlkLnNlbGVjdGVkTm9kZSk7CisgICAgICAg IHRoaXMuX2RhdGFHcmlkLmNoYW5nZU5vZGVBZnRlckRlbGV0aW9uKCk7CiAgICAgfSwKIAogICAg IF9yZWZyZXNoQnV0dG9uQ2xpY2tlZDogZnVuY3Rpb24oZXZlbnQpCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5kL0RhdGFHcmlkLmpzIGIvU291cmNlL1dlYkNv cmUvaW5zcGVjdG9yL2Zyb250LWVuZC9EYXRhR3JpZC5qcwppbmRleCA0Mzk5YzdmNjI0NmEzOTNh N2JmZjFkY2Q1ZDMzZmY3OThkZjhiZTQxLi43Y2VkM2U2ZmU2NzcyOTZmYTBjNDRhOWVkZTc0NGUy MGMzMzZmYjcxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9pbnNwZWN0b3IvZnJvbnQtZW5k L0RhdGFHcmlkLmpzCisrKyBiL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9mcm9udC1lbmQvRGF0 YUdyaWQuanMKQEAgLTgwOCwxNiArODA4LDcgQEAgV2ViSW5zcGVjdG9yLkRhdGFHcmlkLnByb3Rv dHlwZSA9IHsKICAgICAgICAgICAgIGlmICh0aGlzLl9kZWxldGVDYWxsYmFjaykgewogICAgICAg ICAgICAgICAgIGhhbmRsZWQgPSB0cnVlOwogICAgICAgICAgICAgICAgIHRoaXMuX2RlbGV0ZUNh bGxiYWNrKHRoaXMuc2VsZWN0ZWROb2RlKTsKLQotICAgICAgICAgICAgICAgIG5leHRTZWxlY3Rl ZE5vZGUgPSB0aGlzLnNlbGVjdGVkTm9kZS50cmF2ZXJzZU5leHROb2RlKHRydWUpOwotICAgICAg ICAgICAgICAgIHdoaWxlIChuZXh0U2VsZWN0ZWROb2RlICYmICFuZXh0U2VsZWN0ZWROb2RlLnNl bGVjdGFibGUpCi0gICAgICAgICAgICAgICAgICAgIG5leHRTZWxlY3RlZE5vZGUgPSBuZXh0U2Vs ZWN0ZWROb2RlLnRyYXZlcnNlTmV4dE5vZGUodHJ1ZSk7Ci0KLSAgICAgICAgICAgICAgICBpZiAo IW5leHRTZWxlY3RlZE5vZGUgfHwgbmV4dFNlbGVjdGVkTm9kZS5pc0NyZWF0aW9uTm9kZSkgewot ICAgICAgICAgICAgICAgICAgICBuZXh0U2VsZWN0ZWROb2RlID0gdGhpcy5zZWxlY3RlZE5vZGUu dHJhdmVyc2VQcmV2aW91c05vZGUodHJ1ZSk7Ci0gICAgICAgICAgICAgICAgICAgIHdoaWxlIChu ZXh0U2VsZWN0ZWROb2RlICYmICFuZXh0U2VsZWN0ZWROb2RlLnNlbGVjdGFibGUpCi0gICAgICAg ICAgICAgICAgICAgICAgICBuZXh0U2VsZWN0ZWROb2RlID0gbmV4dFNlbGVjdGVkTm9kZS50cmF2 ZXJzZVByZXZpb3VzTm9kZSh0cnVlKTsKLSAgICAgICAgICAgICAgICB9CisgICAgICAgICAgICAg ICAgdGhpcy5jaGFuZ2VOb2RlQWZ0ZXJEZWxldGlvbigpOwogICAgICAgICAgICAgfQogICAgICAg ICB9IGVsc2UgaWYgKGlzRW50ZXJLZXkoZXZlbnQpKSB7CiAgICAgICAgICAgICBpZiAodGhpcy5f ZWRpdENhbGxiYWNrKSB7CkBAIC04MzUsNiArODI2LDI0IEBAIFdlYkluc3BlY3Rvci5EYXRhR3Jp ZC5wcm90b3R5cGUgPSB7CiAgICAgICAgICAgICBldmVudC5jb25zdW1lKHRydWUpOwogICAgIH0s CiAKKyAgICBjaGFuZ2VOb2RlQWZ0ZXJEZWxldGlvbjogZnVuY3Rpb24oKQorICAgIHsKKyAgICAg ICAgdmFyIG5leHRTZWxlY3RlZE5vZGUgPSB0aGlzLnNlbGVjdGVkTm9kZS50cmF2ZXJzZU5leHRO b2RlKHRydWUpOworICAgICAgICB3aGlsZSAobmV4dFNlbGVjdGVkTm9kZSAmJiAhbmV4dFNlbGVj dGVkTm9kZS5zZWxlY3RhYmxlKQorICAgICAgICAgICAgbmV4dFNlbGVjdGVkTm9kZSA9IG5leHRT ZWxlY3RlZE5vZGUudHJhdmVyc2VOZXh0Tm9kZSh0cnVlKTsKKworICAgICAgICBpZiAoIW5leHRT ZWxlY3RlZE5vZGUgfHwgbmV4dFNlbGVjdGVkTm9kZS5pc0NyZWF0aW9uTm9kZSkgeworICAgICAg ICAgICAgbmV4dFNlbGVjdGVkTm9kZSA9IHRoaXMuc2VsZWN0ZWROb2RlLnRyYXZlcnNlUHJldmlv dXNOb2RlKHRydWUpOworICAgICAgICAgICAgd2hpbGUgKG5leHRTZWxlY3RlZE5vZGUgJiYgIW5l eHRTZWxlY3RlZE5vZGUuc2VsZWN0YWJsZSkKKyAgICAgICAgICAgICAgICBuZXh0U2VsZWN0ZWRO b2RlID0gbmV4dFNlbGVjdGVkTm9kZS50cmF2ZXJzZVByZXZpb3VzTm9kZSh0cnVlKTsKKyAgICAg ICAgfQorCisgICAgICAgIGlmIChuZXh0U2VsZWN0ZWROb2RlKSB7CisgICAgICAgICAgICBuZXh0 U2VsZWN0ZWROb2RlLnJldmVhbCgpOworICAgICAgICAgICAgbmV4dFNlbGVjdGVkTm9kZS5zZWxl Y3QoKTsKKyAgICAgICAgfQorICAgIH0sCisKICAgICAvKioKICAgICAgKiBAcGFyYW0geyFOb2Rl fSB0YXJnZXQKICAgICAgKiBAcmV0dXJuIHs/V2ViSW5zcGVjdG9yLkRhdGFHcmlkTm9kZX0K