151628 2015-11-26 20:23:45 -0800 Web Inspector: REGRESSION: "Duplicate Selector" context menu item doesn't work 2015-11-28 00:51:27 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED https://wwwapple.com GoodFirstBug, InRadar P2 Normal --- 1 bburg hi bburg commit-queue graouts hi joepeck mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1144547 0 bburg 2015-11-26 20:23:45 -0800 This command seems to send an invalid command to the backend. Here's the relevant protocol traffic: [native code]: CONSOLE LOG frontend: {"id":222,"method":"CSS.addRule","params":{"contextNodeId":7,"selector":"#ac-gn-menustate"}} [native code]: CONSOLE LOG backend: {"error":{"code":-32602,"message":"Some arguments of method 'CSS.addRule' can't be processed","data":[{"code":-32602,"message":"Parameter 'styleSheetId' with type 'String' was not found."},{"code":-32602,"message":"Some arguments of method 'CSS.addRule' can't be processed"}]},"id":222} The command did not ship in Safari 9, so it's probably new. I don't know if it ever worked. We should extract the underlying command and add a test for this, since nobody even knew it was broken. I only found out via manual testing for another context menu related change. 1144548 1 webkit-bug-importer 2015-11-26 20:23:57 -0800 <rdar://problem/23673487> 1144549 2 webkit-bug-importer 2015-11-26 20:24:36 -0800 <rdar://problem/23673489> 1144602 3 hi 2015-11-27 10:29:20 -0800 (In reply to comment #0) > The command did not ship in Safari 9, so it's probably new. Yeah I added this over the summer. > I don't know if it ever worked. I can definitely confirm that it did work when it was added. > We should extract the underlying command and add a test for this, since nobody even knew it was broken. When I implemented this it didn't use any new commands and simply created a new rule using the selector of the rule under the contextmenu. I could be wrong, but I'm pretty sure the addRule command already has a test for it. 1144604 4 bburg 2015-11-27 11:01:17 -0800 (In reply to comment #3) > (In reply to comment #0) > > We should extract the underlying command and add a test for this, since nobody even knew it was broken. > > When I implemented this it didn't use any new commands and simply created a > new rule using the selector of the rule under the contextmenu. I could be > wrong, but I'm pretty sure the addRule command already has a test for it. What I meant was that we should expose the same action that the context menu does, as part of a model or controller so that we can "click" it using a test. The failure is probably specific to how the backend command is being generated in the context menu code. 1144631 5 266210 hi 2015-11-27 21:43:27 -0800 Created attachment 266210 Patch So I think that the reason for the errors was that the "addRuleWithSelector" function inside DOMNodeStyles didn't have any of the fallbacks present in "addEmptyRule" (which is why "addEmptyRule" worked just fine even though it used the same CSSAgent calls). I merged the two functions into one because I think it's silly to have two if they do practically the same thing. 1144640 6 266210 bburg 2015-11-28 00:05:05 -0800 Comment on attachment 266210 Patch r=me, thanks. Should be covered by existing tests. 1144645 7 266210 commit-queue 2015-11-28 00:51:24 -0800 Comment on attachment 266210 Patch Clearing flags on attachment: 266210 Committed r192784: <http://trac.webkit.org/changeset/192784> 1144646 8 commit-queue 2015-11-28 00:51:27 -0800 All reviewed patches have been landed. Closing bug. 266210 2015-11-27 21:43:27 -0800 2015-11-28 00:51:24 -0800 Patch 151628.txt text/plain 5614 hi ZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvQ2hhbmdlTG9nCmluZGV4IDQ0MTJlZWUuLmRjODM5YTUgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL1dlYkluc3BlY3Rv clVJL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI5IEBACisyMDE1LTExLTI3ICBEZXZpbiBSb3Vzc28g IDxkY3JvdXNzbyt3ZWJraXRAZ21haWwuY29tPgorCisgICAgICAgIFdlYiBJbnNwZWN0b3I6IFJF R1JFU1NJT046ICJEdXBsaWNhdGUgU2VsZWN0b3IiIGNvbnRleHQgbWVudSBpdGVtIGRvZXNuJ3Qg d29yaworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTUx NjI4CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTWVy Z2VkIHRoZSB0d28gImFkZCBydWxlIiBmdW5jdGlvbnMgaW5zaWRlIERPTU5vZGVTdHlsZXMgdG8g Y3JlYXRlIGEKKyAgICAgICAgbmV3IHJ1bGUgd2l0aCB0aGUgZ2l2ZW4gc2VsZWN0b3IgYW5kIHVz ZSB0aGUgZ2VuZXJhdGVkIGJlc3Qgc2VsZWN0b3IKKyAgICAgICAgZm9yIHRoYXQgbm9kZSBvdGhl cndpc2UuIFRoaXMgYWxzbyBwcmVzZXJ2ZXMgYWxsIGZhbGxiYWNrcyBhY3Jvc3MgYWxsCisgICAg ICAgIGZ1bmN0aW9ucyBmb3IgY3JlYXRpbmcgbmV3IENTUyBydWxlcy4KKworICAgICAgICAqIFVz ZXJJbnRlcmZhY2UvTW9kZWxzL0RPTU5vZGVTdHlsZXMuanM6CisgICAgICAgIChXZWJJbnNwZWN0 b3IuRE9NTm9kZVN0eWxlcy5wcm90b3R5cGUuYWRkRW1wdHlSdWxlKTogRGVsZXRlZC4KKyAgICAg ICAgKFdlYkluc3BlY3Rvci5ET01Ob2RlU3R5bGVzLnByb3RvdHlwZS5hZGRSdWxlV2l0aFNlbGVj dG9yKTogRGVsZXRlZC4KKyAgICAgICAgKFdlYkluc3BlY3Rvci5ET01Ob2RlU3R5bGVzLnByb3Rv dHlwZS5hZGRSdWxlKToKKyAgICAgICAgQ3JlYXRlcyBhIG5ldyBDU1MgcnVsZSB1c2luZyBlaXRo ZXIgdGhlIHByb3ZpZGVkIHNlbGVjdG9yIG9yIHRoZSBiZXN0CisgICAgICAgIHNlbGVjdG9yIGZv ciB0aGUgY3VycmVudCBub2RlLgorCisgICAgICAgICogVXNlckludGVyZmFjZS9WaWV3cy9DU1NT dHlsZURlY2xhcmF0aW9uU2VjdGlvbi5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5DU1NTdHls ZURlY2xhcmF0aW9uU2VjdGlvbi5wcm90b3R5cGUuX2hhbmRsZUNvbnRleHRNZW51RXZlbnQpOgor ICAgICAgICAqIFVzZXJJbnRlcmZhY2UvVmlld3MvUnVsZXNTdHlsZURldGFpbHNQYW5lbC5qczoK KyAgICAgICAgKFdlYkluc3BlY3Rvci5SdWxlc1N0eWxlRGV0YWlsc1BhbmVsLnByb3RvdHlwZS5u ZXdSdWxlQnV0dG9uQ2xpY2tlZCk6CisgICAgICAgICogVXNlckludGVyZmFjZS9WaWV3cy9WaXN1 YWxTdHlsZVNlbGVjdG9yU2VjdGlvbi5qczoKKyAgICAgICAgKFdlYkluc3BlY3Rvci5WaXN1YWxT dHlsZVNlbGVjdG9yU2VjdGlvbi5wcm90b3R5cGUuX2FkZE5ld1J1bGUpOgorCiAyMDE1LTExLTI0 ICBCcmlhbiBCdXJnICA8YmJ1cmdAYXBwbGUuY29tPgogCiAgICAgICAgIFdlYiBJbnNwZWN0b3I6 IHNhdmUgSW5zcGVjdG9yJ3MgYnJlYWtwb2ludHMgdG8gbG9jYWxTdG9yYWdlIHdoZW5ldmVyIHRo ZXkgYXJlIG1vZGlmaWVkCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlcklu dGVyZmFjZS9Nb2RlbHMvRE9NTm9kZVN0eWxlcy5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9V c2VySW50ZXJmYWNlL01vZGVscy9ET01Ob2RlU3R5bGVzLmpzCmluZGV4IDBhMTM5MmEuLmI2NzY5 OTEgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL01vZGVs cy9ET01Ob2RlU3R5bGVzLmpzCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJm YWNlL01vZGVscy9ET01Ob2RlU3R5bGVzLmpzCkBAIC0yMjMsNyArMjIzLDcgQEAgV2ViSW5zcGVj dG9yLkRPTU5vZGVTdHlsZXMgPSBjbGFzcyBET01Ob2RlU3R5bGVzIGV4dGVuZHMgV2ViSW5zcGVj dG9yLk9iamVjdAogICAgICAgICBDU1NBZ2VudC5nZXRDb21wdXRlZFN0eWxlRm9yTm9kZS5pbnZv a2Uoe25vZGVJZDogdGhpcy5fbm9kZS5pZH0sIGZldGNoZWRDb21wdXRlZFN0eWxlLmJpbmQodGhp cykpOwogICAgIH0KIAotICAgIGFkZEVtcHR5UnVsZSgpCisgICAgYWRkUnVsZShzZWxlY3RvciA9 IHRoaXMuX25vZGUuYXBwcm9wcmlhdGVTZWxlY3RvckZvcih0cnVlKSkKICAgICB7CiAgICAgICAg IGZ1bmN0aW9uIGFkZGVkUnVsZShlcnJvciwgcnVsZVBheWxvYWQpCiAgICAgICAgIHsKQEAgLTIz NSw4ICsyMzUsNiBAQCBXZWJJbnNwZWN0b3IuRE9NTm9kZVN0eWxlcyA9IGNsYXNzIERPTU5vZGVT dHlsZXMgZXh0ZW5kcyBXZWJJbnNwZWN0b3IuT2JqZWN0CiAgICAgICAgICAgICB0aGlzLnJlZnJl c2goKTsKICAgICAgICAgfQogCi0gICAgICAgIGxldCBzZWxlY3RvciA9IHRoaXMuX25vZGUuYXBw cm9wcmlhdGVTZWxlY3RvckZvcih0cnVlKTsKLQogICAgICAgICAvLyBDT01QQVRJQklMSVRZIChp T1MgOSk6IEJlZm9yZSBDU1MuY3JlYXRlU3R5bGVTaGVldCwgQ1NTLmFkZFJ1bGUgY291bGQgYmUg Y2FsbGVkIHdpdGggYSBjb250ZXh0Tm9kZS4KICAgICAgICAgaWYgKCFDU1NBZ2VudC5jcmVhdGVT dHlsZVNoZWV0KSB7CiAgICAgICAgICAgICBDU1NBZ2VudC5hZGRSdWxlLmludm9rZSh7Y29udGV4 dE5vZGVJZDogdGhpcy5fbm9kZS5pZCAsIHNlbGVjdG9yfSwgYWRkZWRSdWxlLmJpbmQodGhpcykp OwpAQCAtMjUxLDI0ICsyNDksNiBAQCBXZWJJbnNwZWN0b3IuRE9NTm9kZVN0eWxlcyA9IGNsYXNz IERPTU5vZGVTdHlsZXMgZXh0ZW5kcyBXZWJJbnNwZWN0b3IuT2JqZWN0CiAgICAgICAgIFdlYklu c3BlY3Rvci5jc3NTdHlsZU1hbmFnZXIucHJlZmVycmVkSW5zcGVjdG9yU3R5bGVTaGVldEZvckZy YW1lKHRoaXMuX25vZGUuZnJhbWUsIGluc3BlY3RvclN0eWxlU2hlZXRBdmFpbGFibGUuYmluZCh0 aGlzKSk7CiAgICAgfQogCi0gICAgYWRkUnVsZVdpdGhTZWxlY3RvcihzZWxlY3RvcikKLSAgICB7 Ci0gICAgICAgIGlmICghc2VsZWN0b3IpCi0gICAgICAgICAgICByZXR1cm47Ci0KLSAgICAgICAg ZnVuY3Rpb24gYWRkZWRSdWxlKGVycm9yLCBydWxlUGF5bG9hZCkKLSAgICAgICAgewotICAgICAg ICAgICAgaWYgKGVycm9yKQotICAgICAgICAgICAgICAgIHJldHVybjsKLQotICAgICAgICAgICAg RE9NQWdlbnQubWFya1VuZG9hYmxlU3RhdGUoKTsKLQotICAgICAgICAgICAgdGhpcy5yZWZyZXNo KCk7Ci0gICAgICAgIH0KLQotICAgICAgICBDU1NBZ2VudC5hZGRSdWxlLmludm9rZSh7Y29udGV4 dE5vZGVJZDogdGhpcy5fbm9kZS5pZCwgc2VsZWN0b3J9LCBhZGRlZFJ1bGUuYmluZCh0aGlzKSk7 Ci0gICAgfQotCiAgICAgZ2V0IG1hdGNoZWRSdWxlcygpCiAgICAgewogICAgICAgICByZXR1cm4g dGhpcy5fbWF0Y2hlZFJ1bGVzOwpkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1Vz ZXJJbnRlcmZhY2UvVmlld3MvQ1NTU3R5bGVEZWNsYXJhdGlvblNlY3Rpb24uanMgYi9Tb3VyY2Uv V2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9DU1NTdHlsZURlY2xhcmF0aW9uU2Vj dGlvbi5qcwppbmRleCAzOTc0MzdmLi4xY2JlZDE1IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5z cGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9DU1NTdHlsZURlY2xhcmF0aW9uU2VjdGlvbi5q cworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9DU1NTdHls ZURlY2xhcmF0aW9uU2VjdGlvbi5qcwpAQCAtNDM4LDcgKzQzOCw3IEBAIFdlYkluc3BlY3Rvci5D U1NTdHlsZURlY2xhcmF0aW9uU2VjdGlvbiA9IGNsYXNzIENTU1N0eWxlRGVjbGFyYXRpb25TZWN0 aW9uIGV4dGVuCiAgICAgICAgICAgICAgICAgaWYgKHRoaXMuX2RlbGVnYXRlICYmIHR5cGVvZiB0 aGlzLl9kZWxlZ2F0ZS5jc3NTdHlsZURlY2xhcmF0aW9uU2VjdGlvbkZvY3VzTmV4dE5ld0luc3Bl Y3RvclJ1bGUgPT09ICJmdW5jdGlvbiIpCiAgICAgICAgICAgICAgICAgICAgIHRoaXMuX2RlbGVn YXRlLmNzc1N0eWxlRGVjbGFyYXRpb25TZWN0aW9uRm9jdXNOZXh0TmV3SW5zcGVjdG9yUnVsZSgp OwogCi0gICAgICAgICAgICAgICAgdGhpcy5fc3R5bGUubm9kZVN0eWxlcy5hZGRSdWxlV2l0aFNl bGVjdG9yKHRoaXMuX2N1cnJlbnRTZWxlY3RvclRleHQpOworICAgICAgICAgICAgICAgIHRoaXMu X3N0eWxlLm5vZGVTdHlsZXMuYWRkUnVsZSh0aGlzLl9jdXJyZW50U2VsZWN0b3JUZXh0KTsKICAg ICAgICAgICAgIH0uYmluZCh0aGlzKSk7CiAgICAgICAgIH0KIApkaWZmIC0tZ2l0IGEvU291cmNl L1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvUnVsZXNTdHlsZURldGFpbHNQYW5l bC5qcyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNlL1ZpZXdzL1J1bGVzU3R5 bGVEZXRhaWxzUGFuZWwuanMKaW5kZXggNWQ0NjFiNi4uZWZlZDAyMyAxMDA2NDQKLS0tIGEvU291 cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvUnVsZXNTdHlsZURldGFpbHNQ YW5lbC5qcworKysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9S dWxlc1N0eWxlRGV0YWlsc1BhbmVsLmpzCkBAIC0zODgsNyArMzg4LDcgQEAgV2ViSW5zcGVjdG9y LlJ1bGVzU3R5bGVEZXRhaWxzUGFuZWwgPSBjbGFzcyBSdWxlc1N0eWxlRGV0YWlsc1BhbmVsIGV4 dGVuZHMgV2ViSW4KICAgICAgICAgICAgIHJldHVybjsKIAogICAgICAgICB0aGlzLl9mb2N1c05l eHROZXdJbnNwZWN0b3JSdWxlID0gdHJ1ZTsKLSAgICAgICAgdGhpcy5ub2RlU3R5bGVzLmFkZEVt cHR5UnVsZSgpOworICAgICAgICB0aGlzLm5vZGVTdHlsZXMuYWRkUnVsZSgpOwogICAgIH0KIAog ICAgIC8vIFByb3RlY3RlZApkaWZmIC0tZ2l0IGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJ bnRlcmZhY2UvVmlld3MvVmlzdWFsU3R5bGVTZWxlY3RvclNlY3Rpb24uanMgYi9Tb3VyY2UvV2Vi SW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9WaXN1YWxTdHlsZVNlbGVjdG9yU2VjdGlv bi5qcwppbmRleCBkN2I4ODk5Li44ZDFjNDU2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9WaXN1YWxTdHlsZVNlbGVjdG9yU2VjdGlvbi5qcwor KysgYi9Tb3VyY2UvV2ViSW5zcGVjdG9yVUkvVXNlckludGVyZmFjZS9WaWV3cy9WaXN1YWxTdHls ZVNlbGVjdG9yU2VjdGlvbi5qcwpAQCAtMjQwLDcgKzI0MCw3IEBAIFdlYkluc3BlY3Rvci5WaXN1 YWxTdHlsZVNlbGVjdG9yU2VjdGlvbiA9IGNsYXNzIFZpc3VhbFN0eWxlU2VsZWN0b3JTZWN0aW9u IGV4dGVuCiAgICAgICAgIGlmICghdGhpcy5fbm9kZVN0eWxlcykKICAgICAgICAgICAgIHJldHVy bjsKIAotICAgICAgICB0aGlzLl9ub2RlU3R5bGVzLmFkZEVtcHR5UnVsZSgpOworICAgICAgICB0 aGlzLl9ub2RlU3R5bGVzLmFkZFJ1bGUoKTsKICAgICAgICAgdGhpcy5fZm9jdXNOZXh0TmV3SW5z cGVjdG9yUnVsZSA9IHRydWU7CiAgICAgfQogCg==