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
Patch (139.72 KB, patch)
2012-07-30 03:38 PDT, Allan Sandfeld Jensen
no flags
Patch (134.39 KB, patch)
2012-08-24 04:41 PDT, Allan Sandfeld Jensen
no flags
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
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
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.