194254 2019-02-04 16:36:20 -0800 Web Inspector: REGRESSION (r240947): Resources tab: can't select main frame after refreshing page 2019-02-05 15:33:12 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 mattbaker mattbaker commit-queue hi inspector-bugzilla-changes webkit-bug-importer oldest_to_newest 1502105 0 mattbaker 2019-02-04 16:36:20 -0800 Summary: Can't select main frame after refreshing page. Steps to Reproduce: 1. Inspect webkit.org 2. Open Resource tab 3. In the sidebar, select a child of the main resource 4. Reload page 5. The child should be re-selected after reload 6. Try to select the main resource using the mouse => Cannot click to select Note: Hitting the up arrow key to select the main resource from the child works fine. 1502106 1 webkit-bug-importer 2019-02-04 16:36:49 -0800 <rdar://problem/47805023> 1502150 2 mattbaker 2019-02-04 17:16:16 -0800 Regressed in https://trac.webkit.org/changeset/240947. 1502180 3 361142 mattbaker 2019-02-04 17:47:33 -0800 Created attachment 361142 Patch 1502181 4 mattbaker 2019-02-04 17:48:28 -0800 (In reply to Matt Baker from comment #2) > Regressed in https://trac.webkit.org/changeset/240947. This patch actually just allowed the bug to manifest. It was a defect lurking in TreeOutline from before this change was made. 1502199 5 361142 hi 2019-02-04 18:34:20 -0800 Comment on attachment 361142 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361142&action=review r-, can you write some sort of test for this? I think we've long passed the point where we need regression tests for our common UI classes. > Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:825 > + const omitFocus = true; This doesn't seem right. If we try to select a `WI.TreeElement` via `_handleMouseDown`, we'd never focus the parent. Either provide a justification for this, or maybe be a bit more "limiting" (e.g. only `omitFocus` when there is more than one `selectedItems`). 1502202 6 361142 mattbaker 2019-02-04 18:43:03 -0800 Comment on attachment 361142 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=361142&action=review >> Source/WebInspectorUI/UserInterface/Views/TreeOutline.js:825 >> + const omitFocus = true; > > This doesn't seem right. If we try to select a `WI.TreeElement` via `_handleMouseDown`, we'd never focus the parent. Either provide a justification for this, or maybe be a bit more "limiting" (e.g. only `omitFocus` when there is more than one `selectedItems`). Actually that's not the case. The tree receives the focus by virtue of being clicked; the awful `omitFocus` flag is only for forcing the focus when selecting elements programmatically. 1502220 7 mattbaker 2019-02-04 19:32:25 -0800 (In reply to Devin Rousso from comment #5) > Comment on attachment 361142 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=361142&action=review > > r-, can you write some sort of test for this? I think we've long passed the > point where we need regression tests for our common UI classes. Added a comment to https://bugs.webkit.org/show_bug.cgi?id=192044. 1502501 8 361142 hi 2019-02-05 14:01:28 -0800 Comment on attachment 361142 Patch rs=me, please don't forget to write tests :) 1502549 9 361142 commit-queue 2019-02-05 15:33:11 -0800 Comment on attachment 361142 Patch Clearing flags on attachment: 361142 Committed r240997: <https://trac.webkit.org/changeset/240997> 1502550 10 commit-queue 2019-02-05 15:33:12 -0800 All reviewed patches have been landed. Closing bug. 361142 2019-02-04 17:47:33 -0800 2019-02-05 15:33:11 -0800 Patch bug-194254-20190204174732.patch text/plain 2363 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjQwOTUxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCAz MTA2YzViNWExOTE2YzVlYmQwOWM2ZGYyN2E5YmQ4ZmVhNjc1NjYwLi44YTc5MzdkZWFkNzg5NDQz YjQ1M2I5NjU1YWU4NWZjZDA3NGU2ZmFmIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxOCBAQAorMjAxOS0wMi0wNCAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUkVHUkVTU0lPTiAocjI0MDk0Nyk6IFJlc291cmNl cyB0YWI6IGNhbid0IHNlbGVjdCBtYWluIGZyYW1lIGFmdGVyIHJlZnJlc2hpbmcgcGFnZQorICAg ICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTk0MjU0CisgICAg ICAgIDxyZGFyOi8vcHJvYmxlbS80NzgwNTAyMz4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvVHJlZU91dGxpbmUu anM6CisgICAgICAgIChXSS5UcmVlT3V0bGluZS5wcm90b3R5cGUuc2VsZWN0aW9uQ29udHJvbGxl clNlbGVjdGlvbkRpZENoYW5nZSk6CisgICAgICAgIFRyZWVPdXRsaW5lIHNob3VsZCBhbHdheXMg Y2FsbCB0aGUgVHJlZUVsZW1lbnQgYHNlbGVjdGAgYW5kIGBkZXNlbGVjdGAKKyAgICAgICAgbWV0 aG9kcyB3aGlsZSBwcm9jZXNzaW5nIHNlbGVjdGlvbiBjaGFuZ2VzLiBIYXZpbmcgbm90aWZpY2F0 aW9ucworICAgICAgICBzdXBwcmVzc2VkIGJ5IGB0aGlzLl9zdXBwcmVzc05leHRTZWxlY3Rpb25E aWRDaGFuZ2VFdmVudGAgc2hvdWxkIG9ubHkKKyAgICAgICAgYWZmZWN0IHRoZSBkaXNwYXRjaGlu ZyBvZiBUcmVlT3V0bGluZSBldmVudHMuCisKIDIwMTktMDItMDQgIE1hdHQgQmFrZXIgIDxtYXR0 YmFrZXJAYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6IFJFR1JFU1NJT046IFJl c291cmNlczogV0kuVHJlZU91dGxpbmUgYXNzZXJ0aW9ucyB3aGVuIHJlZnJlc2hpbmcgdGhlIHBh Z2UKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdz L1RyZWVPdXRsaW5lLmpzIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmll d3MvVHJlZU91dGxpbmUuanMKaW5kZXggZjc4M2FjZGFiZmE2ZGFkMzdiYTAxZDUzNWI3NzMyNzc3 NWJmODUzZS4uNGVmM2U4NmVlYTIzM2QyODI2MWIxMzgxYTBmZjMxMGM3NmJiNzliMCAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVHJlZU91dGxp bmUuanMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvVHJl ZU91dGxpbmUuanMKQEAgLTgxMiw4ICs4MTIsNyBAQCBXSS5UcmVlT3V0bGluZSA9IGNsYXNzIFRy ZWVPdXRsaW5lIGV4dGVuZHMgV0kuT2JqZWN0CiAgICAgICAgICAgICBpZiAodHJlZUVsZW1lbnQp IHsKICAgICAgICAgICAgICAgICBpZiAodHJlZUVsZW1lbnQubGlzdEl0ZW1FbGVtZW50KQogICAg ICAgICAgICAgICAgICAgICB0cmVlRWxlbWVudC5saXN0SXRlbUVsZW1lbnQuY2xhc3NMaXN0LnJl bW92ZSgic2VsZWN0ZWQiKTsKLSAgICAgICAgICAgICAgICBpZiAoIXRoaXMuX3N1cHByZXNzTmV4 dFNlbGVjdGlvbkRpZENoYW5nZUV2ZW50KQotICAgICAgICAgICAgICAgICAgICB0cmVlRWxlbWVu dC5kZXNlbGVjdCgpOworICAgICAgICAgICAgICAgIHRyZWVFbGVtZW50LmRlc2VsZWN0KCk7CiAg ICAgICAgICAgICB9CiAgICAgICAgIH0KIApAQCAtODIzLDggKzgyMiw4IEBAIFdJLlRyZWVPdXRs aW5lID0gY2xhc3MgVHJlZU91dGxpbmUgZXh0ZW5kcyBXSS5PYmplY3QKICAgICAgICAgICAgIGlm ICh0cmVlRWxlbWVudCkgewogICAgICAgICAgICAgICAgIGlmICh0cmVlRWxlbWVudC5saXN0SXRl bUVsZW1lbnQpCiAgICAgICAgICAgICAgICAgICAgIHRyZWVFbGVtZW50Lmxpc3RJdGVtRWxlbWVu dC5jbGFzc0xpc3QuYWRkKCJzZWxlY3RlZCIpOwotICAgICAgICAgICAgICAgIGlmICghdGhpcy5f c3VwcHJlc3NOZXh0U2VsZWN0aW9uRGlkQ2hhbmdlRXZlbnQpCi0gICAgICAgICAgICAgICAgICAg IHRyZWVFbGVtZW50LnNlbGVjdCgpOworICAgICAgICAgICAgICAgIGNvbnN0IG9taXRGb2N1cyA9 IHRydWU7CisgICAgICAgICAgICAgICAgdHJlZUVsZW1lbnQuc2VsZWN0KG9taXRGb2N1cyk7CiAg ICAgICAgICAgICB9CiAgICAgICAgIH0KIAo=