194169 2019-02-01 11:46:29 -0800 Web Inspector: REGRESSION: clicking a selected call frame doesn't re-scroll 2019-02-04 15:32:53 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=193838 InRadar P2 Normal --- 1 hi mattbaker commit-queue hi inspector-bugzilla-changes mattbaker webkit-bug-importer oldest_to_newest 1501198 0 hi 2019-02-01 11:46:29 -0800 # Steps to reproduce: 1. inspect <https://webkit.org> 2. set a breakpoint on global.js:2 3. reload the page 4. select the "Global Code" call frame 5. select the "(anonymous function)" call frame 6. scroll down so that the highlighted line isn't visible 7. re-select (e.g. click again) the "(anonymous function)" call frame # Expected result: The view should scroll to the line highlighted in step 6. # Actual result: The view doesn't scroll at all. # Notes: I believe this regressed with the addition of `WI.SelectionController`. `WI.SelectionController.prototype.selectItem` early-returns if the index is already selected, meaning we don't call `selectionControllerSelectionDidChange` (which therefore doesn't fire `WI.TreeOutline.Event.SelectionDidChange`). 1501199 1 webkit-bug-importer 2019-02-01 11:47:05 -0800 <rdar://problem/47743864> 1501940 2 361082 mattbaker 2019-02-04 11:57:10 -0800 Created attachment 361082 Patch 1502003 3 361082 hi 2019-02-04 13:52:01 -0800 Comment on attachment 361082 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361082&action=review rs=me > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1018 > + if (this.allowsRepeatSelection && !this.allowsMultipleSelection && treeElement.selected) { Is there a reason/example why we don't want to have this behavior of multiple selection is enabled? I think we should be a bit more "selective" (no pun intended), in that if we do allow multiple selection, but only have one item selected, re-selecting it would also fire an event. Just because we have `allowsMultipleSelection`, doesn't mean we should blanket prevent this behavior. 1502052 4 361107 mattbaker 2019-02-04 15:16:51 -0800 Created attachment 361107 Patch for landing 1502055 5 361082 mattbaker 2019-02-04 15:21:11 -0800 Comment on attachment 361082 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361082&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:1018 >> + if (this.allowsRepeatSelection && !this.allowsMultipleSelection && treeElement.selected) { > > Is there a reason/example why we don't want to have this behavior of multiple selection is enabled? I think we should be a bit more "selective" (no pun intended), in that if we do allow multiple selection, but only have one item selected, re-selecting it would also fire an event. Just because we have `allowsMultipleSelection`, doesn't mean we should blanket prevent this behavior. I can't think of an example, but I don't see a reason why not. Instead of requiring that multiple selection be disabled, we just check that only one item is selected. 1502062 6 361107 commit-queue 2019-02-04 15:32:52 -0800 Comment on attachment 361107 Patch for landing Clearing flags on attachment: 361107 Committed r240947: <https://trac.webkit.org/changeset/240947> 1502063 7 commit-queue 2019-02-04 15:32:53 -0800 All reviewed patches have been landed. Closing bug. 361082 2019-02-04 11:57:10 -0800 2019-02-04 15:16:49 -0800 Patch bug-194169-20190204115710.patch text/plain 2050 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjQwOTI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCBi Y2Q5NzlkMjcxYjFhZTZlMjI5ZmRjZjRmZjQ2ZTBhODUzM2VjNTFiLi41ZWI5NzlmYzMxZDUzMGZj NjI4ODFiNzMzMmRlMTYyZWQ0Y2IzOTFiIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxOS0wMi0wNCAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUkVHUkVTU0lPTjogY2xpY2tpbmcgYSBzZWxlY3Rl ZCBjYWxsIGZyYW1lIGRvZXNuJ3QgcmUtc2Nyb2xsCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTQxNjkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzQ3 NzQzODY0PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogVXNlckludGVyZmFjZS9WaWV3cy9UcmVlT3V0bGluZS5qczoKKyAgICAgICAgKFdJLlRyZWVP dXRsaW5lLnByb3RvdHlwZS5faGFuZGxlTW91c2VEb3duKToKKyAgICAgICAgQWRkIGEgc3BlY2lh bCBjYXNlIGZvciBhIHNpbmdsZS1zZWxlY3Rpb24gVHJlZU91dGxpbmUgd2l0aAorICAgICAgICBh bGxvd3NSZXBlYXRTZWxlY3Rpb24gZW5hYmxlZC4gU2luY2UgdGhlIGVsZW1lbnQgaXMgYWxyZWFk eQorICAgICAgICBzZWxlY3RlZCwgYnlwYXNzIHRoZSBTZWxlY3Rpb25Db25ncm9sbGVyIGFuZCBk aXNwYXRjaCBhbgorICAgICAgICBldmVudCB3aXRoIGV2ZW50LmRhdGEuc2VsZWN0ZWRCeVVzZXIg c2V0IHRvIHRydWUuCisKIDIwMTktMDItMDEgIERldmluIFJvdXNzbyAgPGRyb3Vzc29AYXBwbGUu Y29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IGNyZWF0ZSBpY29ucyBmb3IgbWVkaWEgZXZl bnQgdHlwZXMgaW5zdGVhZCBvZiB1c2luZyBhIGJsdWUgY2lyY2xlCmRpZmYgLS1naXQgYS9Tb3Vy Y2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UcmVlT3V0bGluZS5qcyBiL1Nv dXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RyZWVPdXRsaW5lLmpzCmlu ZGV4IDNmNzhjZDVjZTliYWY0MWY5ZDA5NWUyZmQ1OGVlMTFhZGQ1Y2Y4MmIuLmYxZDQ5MjVkOGIy NzY0ZDYyMDdjZjNkYTA1ZmI5NjcxNWZhOTA3YzUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNw ZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RyZWVPdXRsaW5lLmpzCisrKyBiL1NvdXJjZS9X ZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1RyZWVPdXRsaW5lLmpzCkBAIC0xMDE1 LDYgKzEwMTUsMTQgQEAgV0kuVHJlZU91dGxpbmUgPSBjbGFzcyBUcmVlT3V0bGluZSBleHRlbmRz IFdJLk9iamVjdAogICAgICAgICBpZiAoIXRyZWVFbGVtZW50LmNhblNlbGVjdE9uTW91c2VEb3du KGV2ZW50KSkKICAgICAgICAgICAgIHJldHVybjsKIAorICAgICAgICBpZiAodGhpcy5hbGxvd3NS ZXBlYXRTZWxlY3Rpb24gJiYgIXRoaXMuYWxsb3dzTXVsdGlwbGVTZWxlY3Rpb24gJiYgdHJlZUVs ZW1lbnQuc2VsZWN0ZWQpIHsKKyAgICAgICAgICAgIC8vIFNwZWNpYWwgY2FzZSBmb3IgZGlzcGF0 Y2hpbmcgYSBzZWxlY3Rpb24gZXZlbnQgZm9yIGFuIGFscmVhZHkgc2VsZWN0ZWQKKyAgICAgICAg ICAgIC8vIGl0ZW0gaW4gc2luZ2xlLXNlbGVjdGlvbiBtb2RlLgorICAgICAgICAgICAgdGhpcy5f aXRlbVdhc1NlbGVjdGVkQnlVc2VyID0gdHJ1ZTsKKyAgICAgICAgICAgIHRoaXMuX2Rpc3BhdGNo U2VsZWN0aW9uRGlkQ2hhbmdlRXZlbnQoKTsKKyAgICAgICAgICAgIHJldHVybjsKKyAgICAgICAg fQorCiAgICAgICAgIGxldCBpbmRleCA9IHRoaXMuX2luZGV4T2ZUcmVlRWxlbWVudCh0cmVlRWxl bWVudCk7CiAgICAgICAgIGlmIChpc05hTihpbmRleCkpCiAgICAgICAgICAgICByZXR1cm47Cg== 361107 2019-02-04 15:16:51 -0800 2019-02-04 15:32:52 -0800 Patch for landing bug-194169-20190204151650.patch text/plain 2069 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjQwOTI4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCBi Y2Q5NzlkMjcxYjFhZTZlMjI5ZmRjZjRmZjQ2ZTBhODUzM2VjNTFiLi40MDJmY2QzNGNkNGY0NWFm ZjE1MzAyMzJjMDNlYzEyNTVkNjExNWE0IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxOS0wMi0wNCAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUkVHUkVTU0lPTjogY2xpY2tpbmcgYSBzZWxlY3Rl ZCBjYWxsIGZyYW1lIGRvZXNuJ3QgcmUtc2Nyb2xsCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTQxNjkKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzQ3 NzQzODY0PgorCisgICAgICAgIFJldmlld2VkIGJ5IERldmluIFJvdXNzby4KKworICAgICAgICAq IFVzZXJJbnRlcmZhY2UvVmlld3MvVHJlZU91dGxpbmUuanM6CisgICAgICAgIChXSS5UcmVlT3V0 bGluZS5wcm90b3R5cGUuX2hhbmRsZU1vdXNlRG93bik6CisgICAgICAgIEFkZCBhIHNwZWNpYWwg Y2FzZSBmb3IgYSBzaW5nbGUtc2VsZWN0aW9uIFRyZWVPdXRsaW5lIHdpdGgKKyAgICAgICAgYWxs b3dzUmVwZWF0U2VsZWN0aW9uIGVuYWJsZWQuIFNpbmNlIHRoZSBlbGVtZW50IGlzIGFscmVhZHkK KyAgICAgICAgc2VsZWN0ZWQsIGJ5cGFzcyB0aGUgU2VsZWN0aW9uQ29uZ3JvbGxlciBhbmQgZGlz cGF0Y2ggYW4KKyAgICAgICAgZXZlbnQgd2l0aCBldmVudC5kYXRhLnNlbGVjdGVkQnlVc2VyIHNl dCB0byB0cnVlLgorCiAyMDE5LTAyLTAxICBEZXZpbiBSb3Vzc28gIDxkcm91c3NvQGFwcGxlLmNv bT4KIAogICAgICAgICBXZWIgSW5zcGVjdG9yOiBjcmVhdGUgaWNvbnMgZm9yIG1lZGlhIGV2ZW50 IHR5cGVzIGluc3RlYWQgb2YgdXNpbmcgYSBibHVlIGNpcmNsZQpkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVHJlZU91dGxpbmUuanMgYi9Tb3Vy Y2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UcmVlT3V0bGluZS5qcwppbmRl eCAzZjc4Y2Q1Y2U5YmFmNDFmOWQwOTVlMmZkNThlZTExYWRkNWNmODJiLi5mNzgzYWNkYWJmYTZk YWQzN2JhMDFkNTM1Yjc3MzI3Nzc1YmY4NTNlIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UcmVlT3V0bGluZS5qcworKysgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9UcmVlT3V0bGluZS5qcwpAQCAtMTAxNSw2 ICsxMDE1LDE0IEBAIFdJLlRyZWVPdXRsaW5lID0gY2xhc3MgVHJlZU91dGxpbmUgZXh0ZW5kcyBX SS5PYmplY3QKICAgICAgICAgaWYgKCF0cmVlRWxlbWVudC5jYW5TZWxlY3RPbk1vdXNlRG93bihl dmVudCkpCiAgICAgICAgICAgICByZXR1cm47CiAKKyAgICAgICAgaWYgKHRoaXMuYWxsb3dzUmVw ZWF0U2VsZWN0aW9uICYmIHRyZWVFbGVtZW50LnNlbGVjdGVkICYmIHRoaXMuX3NlbGVjdGlvbkNv bnRyb2xsZXIuc2VsZWN0ZWRJdGVtcy5zaXplID09PSAxKSB7CisgICAgICAgICAgICAvLyBTcGVj aWFsIGNhc2UgZm9yIGRpc3BhdGNoaW5nIGEgc2VsZWN0aW9uIGV2ZW50IGZvciBhbiBhbHJlYWR5 IHNlbGVjdGVkCisgICAgICAgICAgICAvLyBpdGVtIGluIHNpbmdsZS1zZWxlY3Rpb24gbW9kZS4K KyAgICAgICAgICAgIHRoaXMuX2l0ZW1XYXNTZWxlY3RlZEJ5VXNlciA9IHRydWU7CisgICAgICAg ICAgICB0aGlzLl9kaXNwYXRjaFNlbGVjdGlvbkRpZENoYW5nZUV2ZW50KCk7CisgICAgICAgICAg ICByZXR1cm47CisgICAgICAgIH0KKwogICAgICAgICBsZXQgaW5kZXggPSB0aGlzLl9pbmRleE9m VHJlZUVsZW1lbnQodHJlZUVsZW1lbnQpOwogICAgICAgICBpZiAoaXNOYU4oaW5kZXgpKQogICAg ICAgICAgICAgcmV0dXJuOwo=