RESOLVED FIXED 97685
[chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events.
https://bugs.webkit.org/show_bug.cgi?id=97685
Summary [chromium] Do not propagate touch-events to plugins unless they explicitly re...
Sadrul Habib Chowdhury
Reported 2012-09-26 08:24:16 PDT
[chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events.
Attachments
Patch (1.53 KB, patch)
2012-09-26 08:26 PDT, Sadrul Habib Chowdhury
no flags
Sadrul Habib Chowdhury
Comment 1 2012-09-26 08:26:21 PDT
Adam Barth
Comment 2 2012-09-26 09:12:53 PDT
Comment on attachment 165808 [details] Patch Can we test this change? Perhaps using a Chromium unit test?
Robert Kroeger
Comment 3 2012-09-26 10:27:04 PDT
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?)
Adam Barth
Comment 4 2012-09-26 10:33:16 PDT
Comment on attachment 165808 [details] 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.
WebKit Review Bot
Comment 5 2012-09-26 11:15:13 PDT
Comment on attachment 165808 [details] Patch Clearing flags on attachment: 165808 Committed r129674: <http://trac.webkit.org/changeset/129674>
WebKit Review Bot
Comment 6 2012-09-26 11:15:16 PDT
All reviewed patches have been landed. Closing bug.
Sadrul Habib Chowdhury
Comment 7 2012-09-26 15:04:53 PDT
(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.
Note You need to log in before you can comment on or make changes to this bug.