Summary: | Add hasTouchEventhandlersAt to WebWidget API | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | yusufo | ||||||||
Component: | New Bugs | Assignee: | yusufo | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | abarth, aelias, dglazkov, fishd, jamesr, leviw, rjkroege, tkent+wkapi, webkit.review.bot, yusufo | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
yusufo
2012-11-16 11:13:10 PST
Created attachment 174724 [details]
Patch
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI. Comment on attachment 174724 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=174724&action=review > Source/WebKit/chromium/public/WebInputEventAck.h:36 > +enum EventDisposition { DidHandle, DidNotHandle, DropEvent }; We don't need this. Look at WebCompositorInputHandlerClient - it has didHandleInputEvent() and didNotHandleInputEvent(bool) already What I meant was exposing the distinction to classes outside WebCompositorInputHandler and Client. The client makes that distinction, but when the ACK IPC is send to render_widget_host, we only have the boolean. My plan was to start sending this enum with the IPC(ViewHostMsg_HandleInputEvent_ACK) instead of a bool, so we can make decisions based on whether the event was not handled or dropped on the browser side. (In reply to comment #3) > (From update of attachment 174724 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=174724&action=review > > > Source/WebKit/chromium/public/WebInputEventAck.h:36 > > +enum EventDisposition { DidHandle, DidNotHandle, DropEvent }; > > We don't need this. Look at WebCompositorInputHandlerClient - it has didHandleInputEvent() and didNotHandleInputEvent(bool) already (In reply to comment #4) > What I meant was exposing the distinction to classes outside WebCompositorInputHandler and Client. The client makes that distinction, but when the ACK IPC is send to render_widget_host, we only have the boolean. My plan was to start sending this enum with the IPC(ViewHostMsg_HandleInputEvent_ACK) instead of a bool, so we can make decisions based on whether the event was not handled or dropped on the browser side. > WebCompositorInputHandlerClient is implemented in chromium (specifically in content/renderer/gpu/compositor_thread.cc), so you have access to all the information you need there and can do whatever you like (such as do something different than just sending the ACK). You don't need any changes on the WebKit side. I chatted with leviw@ and we agreed that we need to send a didHandle/didNotHandle/dropped tri-state from the WebKit thread as well, to have a fallback for the case where the compositor-thread rects are wrong. So this will need some kind of API change, though maybe instead of exposing a tri-state like this we could add a new WebView method to hit test for touch handler existence: bool hasTouchHandlerAt(const WebPoint&); Created attachment 175250 [details]
Patch
Renamed bug, uploaded new patch that adds hasTouchEventHandlersAt to WebView API. PTAL. Comment on attachment 175250 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=175250&action=review The interface should move I think, but otherwise looks good. > Source/WebKit/chromium/public/WebView.h:275 > + // Check whether the given point hits any registered touch event handlers. > + virtual bool hasTouchEventHandlersAt(const WebPoint&) = 0; This should be on WebWidget - that's where handleInputEvent() is Created attachment 176069 [details]
Patch
Renamed bug to reflect changes and uploaded new patch. PTAL. Comment on attachment 176069 [details] Patch Clearing flags on attachment: 176069 Committed r135787: <http://trac.webkit.org/changeset/135787> All reviewed patches have been landed. Closing bug. |