Bug 101406 - Add API for touchEventHandlerRegion to WebLayer
Summary: Add API for touchEventHandlerRegion to WebLayer
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: yusufo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-06 16:01 PST by yusufo
Modified: 2012-11-07 11:31 PST (History)
8 users (show)

See Also:


Attachments
Patch (1.57 KB, patch)
2012-11-06 16:04 PST, yusufo
no flags Details | Formatted Diff | Diff
Patch (1.63 KB, patch)
2012-11-07 10:28 PST, yusufo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description yusufo 2012-11-06 16:01:18 PST
Add API for touchEventHandlerRegion to WebLayer
Comment 1 yusufo 2012-11-06 16:04:11 PST
Created attachment 172669 [details]
Patch
Comment 2 WebKit Review Bot 2012-11-06 16:07:16 PST
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 3 yusufo 2012-11-06 16:11:45 PST
jamesr@ can you take a look at this when you get a chance? Thanks!
Comment 4 WebKit Review Bot 2012-11-06 16:13:50 PST
Comment on attachment 172669 [details]
Patch

Attachment 172669 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14745487
Comment 5 Peter Beverloo (cr-android ews) 2012-11-06 17:22:35 PST
Comment on attachment 172669 [details]
Patch

Attachment 172669 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/14745506
Comment 6 James Robinson 2012-11-06 21:17:09 PST
Comment on attachment 172669 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=172669&action=review

> Source/Platform/chromium/public/WebLayer.h:194
> +    virtual void setTouchEventHandlerRegion(const WebVector<WebRect>&) = 0;
> +    virtual WebVector<WebRect> touchEventHandlerRegion() const = 0;

I would recommend that you not make these pure virtual, even though it's not consistent with the rest of the file.  Having them be pure virtual is a pain when trying to land a two-side patch of this nature.

Can you put this a few lines up next to setNonFastScrollable..()? there's a section there that has to do with input (setHaveWheel..., setShouldScroll..., setNonFast...) and I think this goes well with those
Comment 7 James Robinson 2012-11-06 21:17:28 PST
Comment on attachment 172669 [details]
Patch

R=me with those minor issues fixed
Comment 8 yusufo 2012-11-07 10:28:27 PST
Created attachment 172830 [details]
Patch
Comment 9 yusufo 2012-11-07 10:50:45 PST
jamesr@ can you put this on the CQ if it looks good? Thanks a lot!
Comment 10 WebKit Review Bot 2012-11-07 11:31:44 PST
Comment on attachment 172830 [details]
Patch

Clearing flags on attachment: 172830

Committed r133787: <http://trac.webkit.org/changeset/133787>
Comment 11 WebKit Review Bot 2012-11-07 11:31:48 PST
All reviewed patches have been landed.  Closing bug.