WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 90992
Rename HitTestPoint and pointInContainer
https://bugs.webkit.org/show_bug.cgi?id=90992
Summary
Rename HitTestPoint and pointInContainer
Allan Sandfeld Jensen
Reported
2012-07-11 10:43:38 PDT
The implementations of the hit-testing function called nodeAtPoint make consistent used of a parameter called pointInContainer of type HitTestPoint. This value is not necessarly a single point however, but can be an area from a rect-based hit test. To make the code clearer, these values and classes should be renamed to something indicating that they are often more than a point.
Attachments
Patch
(135.64 KB, patch)
2012-07-30 03:30 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Patch
(139.72 KB, patch)
2012-07-30 03:38 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Patch
(134.39 KB, patch)
2012-08-24 04:41 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Eric Seidel (no email)
Comment 1
2012-07-11 11:02:36 PDT
And How!
Allan Sandfeld Jensen
Comment 2
2012-07-26 04:47:04 PDT
I am currently thinking of renaming HitTestPoint to HitTestLocation, since location is more ambigiously either a point or an area. At the same time I would rename isRectBased to isAreaBased.
Antonio Gomes
Comment 3
2012-07-26 07:04:37 PDT
Does the term "target" apply well?
Allan Sandfeld Jensen
Comment 4
2012-07-27 02:08:27 PDT
(In reply to
comment #3
)
> Does the term "target" apply well?
Yes, that is perhaps an even better name.
Allan Sandfeld Jensen
Comment 5
2012-07-30 03:11:17 PDT
Okay, I have started a rename for HitTestPoint to HitTestTarget, but should nodeAtPoint be renamed as well, and in that case: To what? Neither nodeAtTarget, nor nodesAtTarget really cover all cases. So perhaps a return to hitTest? So far I am leaving it unchanged in the first patch. A better rename can be delayed until we succesfully join nodeAtPoint and nodeAtFloatPoint.
Allan Sandfeld Jensen
Comment 6
2012-07-30 03:30:06 PDT
Created
attachment 155243
[details]
Patch
Allan Sandfeld Jensen
Comment 7
2012-07-30 03:38:29 PDT
Created
attachment 155246
[details]
Patch Also rename isRectBased to isAreaBased as promised
Simon Fraser (smfr)
Comment 8
2012-07-30 08:20:25 PDT
Target sounds too much like an event target; it sounds like you've already computed what's getting hit (the target). How about HitTestArea? I'm OK with HitTestLocation too.
Allan Sandfeld Jensen
Comment 9
2012-07-30 08:43:22 PDT
(In reply to
comment #8
)
> Target sounds too much like an event target; it sounds like you've already computed what's getting hit (the target). >
Right, the target could sound like the result, not the query.
> How about HitTestArea? I'm OK with HitTestLocation too.
I try to avoid hit-test area because it doesn't sound like it can be a single point. Also the term is already used in RenderLayer for the clipping area of the hit-test.
Allan Sandfeld Jensen
Comment 10
2012-08-01 01:58:59 PDT
The recent quadroanual ring-themed sporting event, had me look up what shooting and archery sports call it. Apparently, they talk about where the 'hit' is in the 'target', so target is what they aim for and 'hit' is where they hit it. So if we were to follow that terminology, it would be just HitTestHit (or just Hit), nodeAtHit and hitInContainer.
Allan Sandfeld Jensen
Comment 11
2012-08-24 04:41:08 PDT
Created
attachment 160395
[details]
Patch
Eric Seidel (no email)
Comment 12
2012-08-27 13:56:47 PDT
Comment on
attachment 160395
[details]
Patch Seems reasonable, and seems Simon agrees. r=me. I would also be fine with HitLocation or HitArea.
WebKit Review Bot
Comment 13
2012-08-28 02:41:33 PDT
Comment on
attachment 160395
[details]
Patch Clearing flags on attachment: 160395 Committed
r126859
: <
http://trac.webkit.org/changeset/126859
>
WebKit Review Bot
Comment 14
2012-08-28 02:41:37 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.
Top of Page
Format For Printing
XML
Clone This Bug