97685 2012-09-26 08:24:16 -0700 [chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events. 2012-09-26 15:04:53 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 sadrul sadrul abarth rjkroege webkit.review.bot oldest_to_newest 728583 0 sadrul 2012-09-26 08:24:16 -0700 [chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events. 728589 1 165808 sadrul 2012-09-26 08:26:21 -0700 Created attachment 165808 Patch 728645 2 165808 abarth 2012-09-26 09:12:53 -0700 Comment on attachment 165808 Patch Can we test this change? Perhaps using a Chromium unit test? 728708 3 rjkroege 2012-09-26 10:27:04 -0700 It seems to me that the patch is pretty simple and likely to be right as long as setIsAcceptingTouchEvents gets called when appropriate. Could we test for that somehow? (Perhaps in a different patch?) 728716 4 165808 abarth 2012-09-26 10:33:16 -0700 Comment on attachment 165808 Patch I mean, this patch is fine, but we really should be testing this stuff. We don't have very many plugin test, especially for PPAPI plugins. I suspect writing a test here is going to be a fair amount of work because I don't think we have an API-level testing framework for plugins. That's something we should add, but I feel mean blocking this two-line patch on creating that testing framework. 728769 5 165808 webkit.review.bot 2012-09-26 11:15:13 -0700 Comment on attachment 165808 Patch Clearing flags on attachment: 165808 Committed r129674: <http://trac.webkit.org/changeset/129674> 728770 6 webkit.review.bot 2012-09-26 11:15:16 -0700 All reviewed patches have been landed. Closing bug. 729011 7 sadrul 2012-09-26 15:04:53 -0700 (In reply to comment #4) > (From update of attachment 165808 [details]) > I mean, this patch is fine, but we really should be testing this stuff. We don't have very many plugin test, especially for PPAPI plugins. I suspect writing a test here is going to be a fair amount of work because I don't think we have an API-level testing framework for plugins. That's something we should add, but I feel mean blocking this two-line patch on creating that testing framework. Thanks a lot! :) There are some ppapi-tests for this in chrome (ppapi/tests/test_input_event.cc), especially to make sure that calling setIsAcceptingTouchEvents() updates the state of the renderer and the corresponding host in the browser. However, we do not have any test at the moment to verify that a plugin does not receive any touch event at all unless it explicitly asks for it. I will look into adding a test like this. Thanks again for the quick review. 165808 2012-09-26 08:26:21 -0700 2012-09-26 11:15:13 -0700 Patch bug-97685-20120926112540.patch text/plain 1564 sadrul U3VidmVyc2lvbiBSZXZpc2lvbjogMTI5NDI2CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L2No cm9taXVtL0NoYW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCmluZGV4 IDI4ZTcyNjg0ZjRiNTkzZmE2Y2RmZDAxYWJiMTllOWU2OGRjODEwZTQuLjY1YWRjNjAyMzA5ZjQx NzY0ZDYyMDMxYmI5ZjEyZjNiOWI3ZTRkYzggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvY2hy b21pdW0vQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJLaXQvY2hyb21pdW0vQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTItMDktMjYgIFNhZHJ1bCBIYWJpYiBDaG93ZGh1cnkgIDxzYWRy dWxAY2hyb21pdW0ub3JnPgorCisgICAgICAgIFtjaHJvbWl1bV0gRG8gbm90IGRpc3BhdGNoIHRv dWNoLWV2ZW50cyB0byBwbHVnaW5zIHVubGVzcyB0aGV5IGV4cGxpY2l0bHkgcmVxdWVzdCBmb3Ig dG91Y2gtZXZlbnRzLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5j Z2k/aWQ9OTc2ODUKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICBQbHVnaW5zIG5lZWQgdG8gY2FsbCBzZXRJc0FjY2VwdGluZ1RvdWNoRXZlbnRzIGlmIHRo ZXkgd2FudCB0byByZWNlaXZlIHRvdWNoLWV2ZW50cy4gU28KKyAgICAgICAgYXZvaWQgZGlzcGF0 Y2hpbmcgdG91Y2gtZXZlbnRzIHRvIHBsdWdpbnMgdW5sZXNzIHRoZXkgaGF2ZSBleHBsaWNpdGx5 IHJlcXVlc3RlZCBmb3IgdGhlbS4KKworICAgICAgICAqIHNyYy9XZWJQbHVnaW5Db250YWluZXJJ bXBsLmNwcDoKKwogMjAxMi0wOS0yNCAgVG9ueSBDaGFuZyAgPHRvbnlAY2hyb21pdW0ub3JnPgog CiAgICAgICAgIFVucmV2aWV3ZWQuICBSb2xsZWQgREVQUy4KZGlmZiAtLWdpdCBhL1NvdXJjZS9X ZWJLaXQvY2hyb21pdW0vc3JjL1dlYlBsdWdpbkNvbnRhaW5lckltcGwuY3BwIGIvU291cmNlL1dl YktpdC9jaHJvbWl1bS9zcmMvV2ViUGx1Z2luQ29udGFpbmVySW1wbC5jcHAKaW5kZXggNWMxNjQz MTllNmZmNDM4YmFiZjZiOTc4ZjMxYzU4OTE0ZTY3ZDk0MC4uZTNjNGJlZmI4M2YyMWFjZTllODA3 YTViMGZmMzEyNGJjNTNmODI3NiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYktpdC9jaHJvbWl1bS9z cmMvV2ViUGx1Z2luQ29udGFpbmVySW1wbC5jcHAKKysrIGIvU291cmNlL1dlYktpdC9jaHJvbWl1 bS9zcmMvV2ViUGx1Z2luQ29udGFpbmVySW1wbC5jcHAKQEAgLTc0Nyw2ICs3NDcsOCBAQCB2b2lk IFdlYlBsdWdpbkNvbnRhaW5lckltcGw6OmhhbmRsZUtleWJvYXJkRXZlbnQoS2V5Ym9hcmRFdmVu dCogZXZlbnQpCiAKIHZvaWQgV2ViUGx1Z2luQ29udGFpbmVySW1wbDo6aGFuZGxlVG91Y2hFdmVu dChUb3VjaEV2ZW50KiBldmVudCkKIHsKKyAgICBpZiAoIW1faXNBY2NlcHRpbmdUb3VjaEV2ZW50 cykKKyAgICAgICAgcmV0dXJuOwogICAgIFdlYlRvdWNoRXZlbnRCdWlsZGVyIHdlYkV2ZW50KHRo aXMsICpldmVudCk7CiAgICAgaWYgKHdlYkV2ZW50LnR5cGUgPT0gV2ViSW5wdXRFdmVudDo6VW5k ZWZpbmVkKQogICAgICAgICByZXR1cm47Cg==