RESOLVED FIXED Bug 57923
Change EventHandler::updateSelectionForMouseDrag to take a HitTestResult only.
https://bugs.webkit.org/show_bug.cgi?id=57923
Summary Change EventHandler::updateSelectionForMouseDrag to take a HitTestResult only.
Alice Boxhall
Reported 2011-04-05 23:06:17 PDT
Change EventHandler::updateSelectionForMouseDrag to take a HitTestResult rather than a Node* and an IntPoint&, as the selection may actually not extend into the Node found by the HitTest.
Attachments
Patch (3.53 KB, patch)
2011-04-07 16:42 PDT, Alice Boxhall
no flags
Patch (4.16 KB, patch)
2011-04-10 17:57 PDT, Alice Boxhall
no flags
Alice Boxhall
Comment 1 2011-04-07 16:42:29 PDT
Ryosuke Niwa
Comment 2 2011-04-08 00:28:04 PDT
Comment on attachment 88731 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=88731&action=review > Source/WebCore/page/EventHandler.cpp:631 > + Node* targetNode = EventHandler::targetNode(hitTestResult); > if (!targetNode) You should probably rename the local variable so that you don't have to resolve the name of member function like this. > Source/WebCore/page/EventHandler.cpp:642 > + IntPoint localPoint = hitTestResult.localPoint(); > + VisiblePosition targetPosition = targetRenderer->positionForPoint(localPoint); Why do you need to declare a local variable fot IntPoint?
Ojan Vafai
Comment 3 2011-04-10 17:03:04 PDT
Comment on attachment 88731 [details] Patch r- per the above comments. otherwise, this is a nice cleanup.
Alice Boxhall
Comment 4 2011-04-10 17:57:14 PDT
Alice Boxhall
Comment 5 2011-04-10 17:59:14 PDT
(In reply to comment #2) > (From update of attachment 88731 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=88731&action=review > > > Source/WebCore/page/EventHandler.cpp:631 > > + Node* targetNode = EventHandler::targetNode(hitTestResult); > > if (!targetNode) > > You should probably rename the local variable so that you don't have to resolve the name of member function like this. Done. > > Source/WebCore/page/EventHandler.cpp:642 > > + IntPoint localPoint = hitTestResult.localPoint(); > > + VisiblePosition targetPosition = targetRenderer->positionForPoint(localPoint); > > Why do you need to declare a local variable fot IntPoint? No reason.
WebKit Commit Bot
Comment 6 2011-04-10 19:22:01 PDT
Comment on attachment 88961 [details] Patch Clearing flags on attachment: 88961 Committed r83414: <http://trac.webkit.org/changeset/83414>
WebKit Commit Bot
Comment 7 2011-04-10 19:22:05 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.