Bug 97685 - [chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events.
Summary: [chromium] Do not propagate touch-events to plugins unless they explicitly re...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Sadrul Habib Chowdhury
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-26 08:24 PDT by Sadrul Habib Chowdhury
Modified: 2012-09-26 15:04 PDT (History)
3 users (show)

See Also:


Attachments
Patch (1.53 KB, patch)
2012-09-26 08:26 PDT, Sadrul Habib Chowdhury
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sadrul Habib Chowdhury 2012-09-26 08:24:16 PDT
[chromium] Do not propagate touch-events to plugins unless they explicitly request for touch-events.
Comment 1 Sadrul Habib Chowdhury 2012-09-26 08:26:21 PDT
Created attachment 165808 [details]
Patch
Comment 2 Adam Barth 2012-09-26 09:12:53 PDT
Comment on attachment 165808 [details]
Patch

Can we test this change?  Perhaps using a Chromium unit test?
Comment 3 Robert Kroeger 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?)
Comment 4 Adam Barth 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.
Comment 5 WebKit Review Bot 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>
Comment 6 WebKit Review Bot 2012-09-26 11:15:16 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Sadrul Habib Chowdhury 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.