Bug 60663

Summary:

Change nodeAtPoint to take IntPoint instead of int x, int y

Product:

WebKit

Reporter:

Emil A Eklund <eae>

Component:

Layout and Rendering

Assignee:

Emil A Eklund <eae>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, darin, eric, leviw, simon.fraser

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

60318

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none

Description Emil A Eklund 2011-05-11 14:42:22 PDT

Change nodeAtPoint hit testing code to take an IntPoint argument instead of a pair of ints for x, y.

Comment 1 Levi Weintraub 2011-05-11 15:41:56 PDT

Woo!

Comment 2 Emil A Eklund 2011-05-16 16:28:22 PDT

Created attachment 93710 [details]
Patch

Comment 3 Emil A Eklund 2011-05-16 16:28:41 PDT

I tried to keep this as small as I could, more clean up and changing functions called by nodeAtPoint will follow in separate patches.

I realize that the name pointInContainer might not be entirely correct but it was the best I could come up with. If you can think of a better name please do tell.

Comment 4 Simon Fraser (smfr) 2011-05-16 16:42:25 PDT

Comment on attachment 93710 [details]
Patch

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

> Source/WebCore/rendering/RenderBlock.cpp:3902
> +    tx += x();
> +    ty += y();

I don't like it when methods change parameter values. Maybe a local IntSize localOffset(tx + x(), ty + y())?

Comment 5 Eric Seidel (no email) 2011-05-16 17:01:10 PDT

Comment on attachment 93710 [details]
Patch

LGTM.  Please respond to smfr before landing of course.

Comment 6 Emil A Eklund 2011-05-16 17:16:11 PDT

Created attachment 93715 [details]
Patch

Comment 7 Emil A Eklund 2011-05-16 17:17:25 PDT

> I don't like it when methods change parameter values. Maybe a local IntSize localOffset(tx + x(), ty + y())?

That's a good idea. Made the change you suggested.

Please take another look.

Comment 8 Eric Seidel (no email) 2011-05-16 17:52:47 PDT

Comment on attachment 93715 [details]
Patch

still LGTM.

Comment 9 Emil A Eklund 2011-05-17 10:25:29 PDT

Created attachment 93788 [details]
Patch for landing

Comment 10 WebKit Commit Bot 2011-05-17 11:29:30 PDT

Comment on attachment 93788 [details]
Patch for landing

Rejecting attachment 93788 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'apply-..." exit_code: 1

Last 500 characters of output:
autoinstalled/mechanize/_urllib2_fork.py", line 332, in _call_chain
    result = func(*args)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1170, in https_open
    return self.do_open(conn_factory, req)
  File "/mnt/git/webkit-commit-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/mechanize/_urllib2_fork.py", line 1118, in do_open
    raise URLError(err)
urllib2.URLError: <urlopen error [Errno 60] Operation timed out>

Full output: http://queues.webkit.org/results/8709232

Comment 11 WebKit Commit Bot 2011-05-17 13:57:42 PDT

Comment on attachment 93788 [details]
Patch for landing

Clearing flags on attachment: 93788

Committed r86705: <http://trac.webkit.org/changeset/86705>

Comment 12 WebKit Commit Bot 2011-05-17 13:57:48 PDT

All reviewed patches have been landed.  Closing bug.

Comment 13 WebKit Commit Bot 2011-05-17 14:58:49 PDT

The commit-queue encountered the following flaky tests while processing attachment 93788 [details]:

http/tests/websocket/tests/multiple-connections.html bug 53825 (author: abarth@webkit.org)
The commit-queue is continuing to process your patch.