187020 2018-06-25 15:07:48 -0700 Web Inspector: REGRESSION (r213000): copy from Search results content view broken 2018-06-28 15:38:49 -0700 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 joepeck timothy webkit-bug-importer oldest_to_newest 1436278 0 mattbaker 2018-06-25 15:07:48 -0700 Summary: Copy from Search results content view broken. The selected tree element always takes precedence over the TextEditor selection. Steps to Reproduce: 1. Inspect webkit.org, search for "body" 2. Select a tree element in the Search tab 3. Select some text in the editor 4. Copy Expected: => Selection in editor is copied Actual: => Tree element is copied Regressed in https://bugs.webkit.org/show_bug.cgi?id=167074. 1436279 1 mattbaker 2018-06-25 15:08:02 -0700 <rdar://problem/40928766> 1436280 2 343547 mattbaker 2018-06-25 15:11:27 -0700 Created attachment 343547 Patch 1436285 3 343547 joepeck 2018-06-25 15:25:10 -0700 Comment on attachment 343547 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343547&action=review > Source/WebInspectorUI/UserInterface/Views/SearchTabContentView.js:102 > handleCopyEvent(event) > { > + let currentContentView = this.contentBrowser.currentContentView; Is this specific to the SearchTab? If so why doesn't it happen in other tabs? 1436286 4 343547 timothy 2018-06-25 15:26:39 -0700 Comment on attachment 343547 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343547&action=review > Source/WebInspectorUI/UserInterface/Views/SearchTabContentView.js:113 > + let currentContentView = this.contentBrowser.currentContentView; > + if (currentContentView) { > + if (currentContentView instanceof WI.ClusterContentView) > + currentContentView = currentContentView.contentViewContainer.currentContentView; > + > + if (currentContentView.textEditor) { > + let range = currentContentView.textEditor.selectedTextRange; > + // If the TextEditor contains a selection, don't override the default copy behavior. > + if (range.startLine !== range.endLine || range.startColumn !== range.endColumn) > + return; > + } > + } This seems like it should be passed and handled at another level, since Search isn't the only client of cluster views and nested content browsers. For example, WI.ClusterContentView passes through handleCopyEvent already. Maybe ContentBrowser needs this most of this code? But it shouldn't know about TextEditor either. Hmm. 1436309 5 343547 mattbaker 2018-06-25 16:26:35 -0700 Comment on attachment 343547 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343547&action=review >> Source/WebInspectorUI/UserInterface/Views/SearchTabContentView.js:102 >> + let currentContentView = this.contentBrowser.currentContentView; > > Is this specific to the SearchTab? If so why doesn't it happen in other tabs? It is. A check for handleCopyEvent support was added for TabContentView, and SearchTabContentView is the only implementor. 1436314 6 343547 mattbaker 2018-06-25 16:34:05 -0700 Comment on attachment 343547 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=343547&action=review >> Source/WebInspectorUI/UserInterface/Views/SearchTabContentView.js:113 >> + } > > This seems like it should be passed and handled at another level, since Search isn't the only client of cluster views and nested content browsers. > > For example, WI.ClusterContentView passes through handleCopyEvent already. Maybe ContentBrowser needs this most of this code? But it shouldn't know about TextEditor either. Hmm. This doesn't have anything to do with ClusterContentView or nesting. SearchTabContentView is implementing handleCopyEvent, which causes WI._copy to override the default behavior (coping the CodeMirror selection). What would your expectation be if this were performed at a higher level? Should WI._copy bail out early if focusedContentView has a text selection? 1437531 7 343847 mattbaker 2018-06-28 14:02:24 -0700 Created attachment 343847 Patch 1437578 8 343847 timothy 2018-06-28 15:11:14 -0700 Comment on attachment 343847 Patch I like this better. This might be something we should do elsewhere or generally in WI._copy. But I don't see any other handleCopyEvent code paths that would have this same issue. 1437597 9 343847 commit-queue 2018-06-28 15:38:48 -0700 Comment on attachment 343847 Patch Clearing flags on attachment: 343847 Committed r233334: <https://trac.webkit.org/changeset/233334> 1437598 10 commit-queue 2018-06-28 15:38:49 -0700 All reviewed patches have been landed. Closing bug. 343547 2018-06-25 15:11:27 -0700 2018-06-28 14:02:22 -0700 Patch bug-187020-20180625151126.patch text/plain 2232 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzMTY0CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCA2 ZDY4ZjVmYTk4ODA2MjMyYzMxMjk3MThmN2JjMzVlZjUwNTNhY2M1Li5kMDJkZGQyOTEzZGZmMTJh Y2E0YWEwYmIwY2JmMDNiM2FjOTFlZjU1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNSBAQAorMjAxOC0wNi0yNSAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUkVHUkVTU0lPTiAocjIxMzAwMCk6IGNvcHkgZnJv bSBTZWFyY2ggcmVzdWx0cyBjb250ZW50IHZpZXcgYnJva2VuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODcwMjAKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzQwOTI4NzY2PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgICogVXNlckludGVyZmFjZS9WaWV3cy9TZWFyY2hUYWJDb250ZW50Vmlldy5qczoKKyAg ICAgICAgKFdJLlNlYXJjaFRhYkNvbnRlbnRWaWV3LnByb3RvdHlwZS5oYW5kbGVDb3B5RXZlbnQp OgorICAgICAgICBQcmVmZXIgY29udGVudCB2aWV3IHNlbGVjdGlvbiB0byB0aGUgc2VsZWN0ZWQg dHJlZSBlbGVtZW50LCBpZiBpdCBleGlzdHMuCisKIDIwMTgtMDYtMjUgIE5pa2l0YSBWYXNpbHll diAgPG52YXNpbHlldkBhcHBsZS5jb20+CiAKICAgICAgICAgV2ViIEluc3BlY3RvcjogRGFyayBN b2RlOiBjb2xvciBvdXRsaW5lIGlzIHRvbyBkYXJrCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5z cGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9TZWFyY2hUYWJDb250ZW50Vmlldy5qcyBiL1Nv dXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NlYXJjaFRhYkNvbnRlbnRW aWV3LmpzCmluZGV4IGRiNzcxYWNjODI3MjQxMjZhOTQyNjQ5ZDFlMDM5YzQ5M2JlMTQyOTguLjlm OTc5YjQ0MTFjZmI2YzMzNmI5OGE3OWQ1MDhkNDBjNDUwMmJkNjggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NlYXJjaFRhYkNvbnRlbnRWaWV3 LmpzCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NlYXJj aFRhYkNvbnRlbnRWaWV3LmpzCkBAIC05OSw2ICs5OSwxOSBAQCBXSS5TZWFyY2hUYWJDb250ZW50 VmlldyA9IGNsYXNzIFNlYXJjaFRhYkNvbnRlbnRWaWV3IGV4dGVuZHMgV0kuQ29udGVudEJyb3dz ZXJUYQogCiAgICAgaGFuZGxlQ29weUV2ZW50KGV2ZW50KQogICAgIHsKKyAgICAgICAgbGV0IGN1 cnJlbnRDb250ZW50VmlldyA9IHRoaXMuY29udGVudEJyb3dzZXIuY3VycmVudENvbnRlbnRWaWV3 OworICAgICAgICBpZiAoY3VycmVudENvbnRlbnRWaWV3KSB7CisgICAgICAgICAgICBpZiAoY3Vy cmVudENvbnRlbnRWaWV3IGluc3RhbmNlb2YgV0kuQ2x1c3RlckNvbnRlbnRWaWV3KQorICAgICAg ICAgICAgICAgIGN1cnJlbnRDb250ZW50VmlldyA9IGN1cnJlbnRDb250ZW50Vmlldy5jb250ZW50 Vmlld0NvbnRhaW5lci5jdXJyZW50Q29udGVudFZpZXc7CisKKyAgICAgICAgICAgIGlmIChjdXJy ZW50Q29udGVudFZpZXcudGV4dEVkaXRvcikgeworICAgICAgICAgICAgICAgIGxldCByYW5nZSA9 IGN1cnJlbnRDb250ZW50Vmlldy50ZXh0RWRpdG9yLnNlbGVjdGVkVGV4dFJhbmdlOworICAgICAg ICAgICAgICAgIC8vIElmIHRoZSBUZXh0RWRpdG9yIGNvbnRhaW5zIGEgc2VsZWN0aW9uLCBkb24n dCBvdmVycmlkZSB0aGUgZGVmYXVsdCBjb3B5IGJlaGF2aW9yLgorICAgICAgICAgICAgICAgIGlm IChyYW5nZS5zdGFydExpbmUgIT09IHJhbmdlLmVuZExpbmUgfHwgcmFuZ2Uuc3RhcnRDb2x1bW4g IT09IHJhbmdlLmVuZENvbHVtbikKKyAgICAgICAgICAgICAgICAgICAgcmV0dXJuOworICAgICAg ICAgICAgfQorICAgICAgICB9CisKICAgICAgICAgbGV0IHNlbGVjdGVkVHJlZUVsZW1lbnQgPSB0 aGlzLm5hdmlnYXRpb25TaWRlYmFyUGFuZWwuY29udGVudFRyZWVPdXRsaW5lLnNlbGVjdGVkVHJl ZUVsZW1lbnQ7CiAgICAgICAgIGlmICghc2VsZWN0ZWRUcmVlRWxlbWVudCkKICAgICAgICAgICAg IHJldHVybjsK 343847 2018-06-28 14:02:24 -0700 2018-06-28 15:38:48 -0700 Patch bug-187020-20180628140223.patch text/plain 2365 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMjMzMjcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCBj NDY4NzI0YjdiNDk3NTY0YWMzYzhjMWMxYzFlN2QzN2QxOTUwYWJjLi4wYzRhYmRjZTViNWNlNWNi OGU3N2IwNTQ0M2ZlZjA4YmQ4YTU4ZDkwIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyNCBAQAorMjAxOC0wNi0yOCAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogUkVHUkVTU0lPTiAocjIxMzAwMCk6IGNvcHkgZnJv bSBTZWFyY2ggcmVzdWx0cyBjb250ZW50IHZpZXcgYnJva2VuCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODcwMjAKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzQwOTI4NzY2PgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFNpbmNlIFdJLl9jb3B5IGxpc3RlbnMgZm9yIGEgY29weSBldmVudCBmcm9tIHRoZSBk b2N1bWVudCwgaXQgaXMgY2FsbGVkCisgICAgICAgIGFmdGVyIENvZGVNaXJyb3IgaGFuZGxlcyB0 aGUgZXZlbnQgYW5kIHNldHMgY2xpcGJvYXJkIGRhdGEuIElmIFdJLl9jb3B5CisgICAgICAgIGZp bmRzIGEgY3VzdG9tIGNvcHkgaGFuZGxlciB0byBjYWxsLCB0aGF0IGhhbmRsZXIgY2FuIGRldGVy bWluZSB3aGV0aGVyCisgICAgICAgIHRvIG92ZXJ3cml0ZSB0aGUgY3VycmVudCBjbGlwYm9hcmQg ZGF0YSwgb3IgbGVhdmUgaXQgYWxvbmUuCisKKyAgICAgICAgU2VhcmNoVGFiQ29udGVudFZpZXcn cyBoYW5kbGVDb3B5RXZlbnQgbWV0aG9kIHNob3VsZCByZXR1cm4gZWFybHkgaWYgdGhlCisgICAg ICAgIGNvbnRlbnQgdHJlZSBvdXRsaW5lIGRvZXNuJ3QgaGF2ZSB0aGUgZm9jdXMuIFRoaXMgcHJl dmVudHMgdGhlIHNlbGVjdGlvbgorICAgICAgICBpbiB0aGUgVGV4dEVkaXRvciBmcm9tIGJlaW5n IG92ZXJ3cml0dGVuLCB3aXRob3V0IGFueSBzcGVjaWFsIGtub3dsZWRnZSBvZgorICAgICAgICB0 aGUgY29udGVudCBicm93c2VyJ3MgY3VycmVudCB2aWV3LgorCisgICAgICAgICogVXNlckludGVy ZmFjZS9WaWV3cy9TZWFyY2hUYWJDb250ZW50Vmlldy5qczoKKyAgICAgICAgKFdJLlNlYXJjaFRh YkNvbnRlbnRWaWV3LnByb3RvdHlwZS5oYW5kbGVDb3B5RXZlbnQpOgorCiAyMDE4LTA2LTI1ICBO aWtpdGEgVmFzaWx5ZXYgIDxudmFzaWx5ZXZAYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNw ZWN0b3I6IERhcmsgTW9kZTogQm94IE1vZGVsIHNlY3Rpb24gc2hvdWxkIGhhdmUgZGFyayBiYWNr Z3JvdW5kCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9W aWV3cy9TZWFyY2hUYWJDb250ZW50Vmlldy5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2Vy SW50ZXJmYWNlL1ZpZXdzL1NlYXJjaFRhYkNvbnRlbnRWaWV3LmpzCmluZGV4IGRiNzcxYWNjODI3 MjQxMjZhOTQyNjQ5ZDFlMDM5YzQ5M2JlMTQyOTguLmYzYzVjOTA1NGU2ZjJiZWY5NmM2NzhkOTE3 MjRmM2UwNWQ4YmEyOTIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50 ZXJmYWNlL1ZpZXdzL1NlYXJjaFRhYkNvbnRlbnRWaWV3LmpzCisrKyBiL1NvdXJjZS9XZWJJbnNw ZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1NlYXJjaFRhYkNvbnRlbnRWaWV3LmpzCkBAIC05 OSw3ICs5OSwxMSBAQCBXSS5TZWFyY2hUYWJDb250ZW50VmlldyA9IGNsYXNzIFNlYXJjaFRhYkNv bnRlbnRWaWV3IGV4dGVuZHMgV0kuQ29udGVudEJyb3dzZXJUYQogCiAgICAgaGFuZGxlQ29weUV2 ZW50KGV2ZW50KQogICAgIHsKLSAgICAgICAgbGV0IHNlbGVjdGVkVHJlZUVsZW1lbnQgPSB0aGlz Lm5hdmlnYXRpb25TaWRlYmFyUGFuZWwuY29udGVudFRyZWVPdXRsaW5lLnNlbGVjdGVkVHJlZUVs ZW1lbnQ7CisgICAgICAgIGxldCBjb250ZW50VHJlZU91dGxpbmUgPSB0aGlzLm5hdmlnYXRpb25T aWRlYmFyUGFuZWwuY29udGVudFRyZWVPdXRsaW5lOworICAgICAgICBpZiAoY29udGVudFRyZWVP dXRsaW5lLmVsZW1lbnQgIT09IGRvY3VtZW50LmFjdGl2ZUVsZW1lbnQpCisgICAgICAgICAgICBy ZXR1cm47CisKKyAgICAgICAgbGV0IHNlbGVjdGVkVHJlZUVsZW1lbnQgPSBjb250ZW50VHJlZU91 dGxpbmUuc2VsZWN0ZWRUcmVlRWxlbWVudDsKICAgICAgICAgaWYgKCFzZWxlY3RlZFRyZWVFbGVt ZW50KQogICAgICAgICAgICAgcmV0dXJuOwogCg==