| Summary: | Allow a pre-targeted node to be specified when dispatching a GestureTap event | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Terry Anderson <tdanderson> | ||||||
| Component: | UI Events | Assignee: | 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
Terry Anderson
2012-05-01 10:46:24 PDT
Created attachment 139650 [details]
Patch
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.
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. 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). 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? Created attachment 139654 [details]
Patch for landing
Comment on attachment 139654 [details] Patch for landing Clearing flags on attachment: 139654 Committed r115747: <http://trac.webkit.org/changeset/115747> All reviewed patches have been landed. Closing bug. |