[webkit-reviews] review requested: [Bug 40197] Enhance the hit testing to take a rectangle instead of a point : [Attachment 62569] patch v7 - feature + tests

[bugzilla-daemon at webkit.org]

Antonio Gomes <tonikitoo at webkit.org> has asked	for review:
Bug 40197: Enhance the hit testing to take a rectangle instead of a point
https://bugs.webkit.org/show_bug.cgi?id=40197

Attachment 62569: patch v7 - feature + tests
https://bugs.webkit.org/attachment.cgi?id=62569&action=review

------- Additional Comments from Antonio Gomes <tonikitoo at webkit.org>
(In reply to comment #46)
> (From update of attachment 62453 [details])
> "void static dumpHitTestResult(const HitTestResult& result)"
> I really don't like this function sitting in Document.  It has nothing to do
with Document.	I'd prefer it either be removed or ...

Removed. It was useful for debuggind, and does not need to get checked in.

> Let's use 0,0 as the default value for padding instead of -1,-1 and make
rect-based hit testing be activated if either value of padding is > 0 (rather
than >= 0).  We don't want to fall into rect-based hit testing because somebody
passed 0 values for padding in to these functions.

Done: default 0,0 padding wont trigger a rect-based hit test anymore.

> And to clarify, I still think padding of 0 should work with nodesForRect,
which means you have to special case the 0 padding case (or just hack it so you
force rect based testing on for that hit test by setting the boolean
explicitly).

Added a special "handler" for nodesFromRect(x, y, 0, 0) as suggested:
handleZeroPadding as a private method to Document class.

(In reply to comment #48)
> Another question... shouldn't the padding be unsigned?  It seems like you
shouldn't allow negative padding values at the IDL level.

Done.

> Everything else looks good to me.

Thank you, Dave.