Bug 85296

Summary: Allow a pre-targeted node to be specified when dispatching a GestureTap event
Product: WebKit Reporter: Terry Anderson <tdanderson>
Component: UI EventsAssignee: Terry Anderson <tdanderson>
Status: RESOLVED FIXED    
Severity: Normal CC: rjkroege, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 83947, 85101    
Attachments:
Description Flags
Patch
none
Patch for landing none

Terry Anderson
Reported 2012-05-01 10:46:24 PDT
Add a PassRefPtr<Node> parameter to EventHandler::handleGestureTap representing a DOM Node which has been pre-computed as the target of the GestureTap event. If the parameter is unused, the existing path of code execution does not change. If the parameter is used, adjustedPoint is changed to be the center of the node's bounding rectangle.
Attachments
Patch (3.62 KB, patch)
2012-05-01 10:57 PDT, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Patch for landing (3.69 KB, patch)
2012-05-01 11:58 PDT, Terry Anderson 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Terry Anderson
Comment 1 2012-05-01 10:57:51 PDT
Created attachment 139650 [details] Patch
WebKit Review Bot
Comment 2 2012-05-01 11:02:22 PDT
Attachment 139650 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/ChangeLog:8: You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] Total errors found: 1 in 3 files If any of these errors are false positives, please file a bug against check-webkit-style.
Robert Kroeger
Comment 3 2012-05-01 11:16:03 PDT
Comment on attachment 139650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139650&action=review FWIW: this looks good to me and aligns with what we discussed. > Source/WebCore/page/EventHandler.cpp:2444 > + // FIXME: Refactor to avoid hit testing multiple times (this is only an interim step) put a period at the end of the sentence.
Adam Barth
Comment 4 2012-05-01 11:42:34 PDT
Comment on attachment 139650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139650&action=review >> Source/WebCore/ChangeLog:8 >> + No new tests. (OOPS!) > > You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] The style bot is right. It doesn't look like anyone calls this function with this new parameter. Is this going to be used in the future? > Source/WebCore/page/EventHandler.h:166 > - bool handleGestureTap(const PlatformGestureEvent&); > + bool handleGestureTap(const PlatformGestureEvent&, PassRefPtr<Node> preTargetedNode = 0); Is there a reason why we're using a PassRefPtr here? It seems like just a Node* would work fine. Generally, we use PassRefPtr when transferring ownership (see http://www.webkit.org/coding/RefPtr.html for more details).
Adam Barth
Comment 5 2012-05-01 11:44:38 PDT
Comment on attachment 139650 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=139650&action=review >>> Source/WebCore/ChangeLog:8 >>> + No new tests. (OOPS!) >> >> You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible. [changelog/nonewtests] [5] > > The style bot is right. It doesn't look like anyone calls this function with this new parameter. Is this going to be used in the future? Would you be willing to replace this line with a link to https://bugs.webkit.org/show_bug.cgi?id=85101 and an explanation that the new parameter will be used (and hopefully tested!) by that patch?
Terry Anderson
Comment 6 2012-05-01 11:58:59 PDT
Created attachment 139654 [details] Patch for landing
WebKit Review Bot
Comment 7 2012-05-01 13:49:13 PDT
Comment on attachment 139654 [details] Patch for landing Clearing flags on attachment: 139654 Committed r115747: <http://trac.webkit.org/changeset/115747>
WebKit Review Bot
Comment 8 2012-05-01 13:49:18 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.