[webkit-reviews] review denied: [Bug 7450] elementAtPoint is expensive and should return a smart dictionary : [Attachment 6706] Implementation using a smart dictionary

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Darin Adler <darin at apple.com>'s
request for review:
Bug 7450: elementAtPoint is expensive and should return a smart dictionary
http://bugzilla.opendarwin.org/show_bug.cgi?id=7450

Attachment 6706: Implementation using a smart dictionary
http://bugzilla.opendarwin.org/attachment.cgi?id=6706&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
titleDisplayString and other similar methods should get the
backslashAsCurrencySymbol from the Frame rather than the renderer. This doesn't
depend on rendering at all.

absoluteImageURL and similar functions need to make the NSURL using KURL, since
that code path uses the appropriate NSURL/CFURL method. URLWithString: won't
always work.

I suggest getNodesAtPoint: as the beginning of the method name. I think I've
noticed AppKit/Foundation methods that return multiple values using pointers
with a get prefix. In fact, you might want to pass the point last so it's "get
these things, given this thing" (ForPoint: or AtPoint:).

No need for all those nil checks in nodesAtPoint.

Please split innerNode, innerNonSharedNode, and URLElement into separate lines,
and set all 3 to nil -- if bridge is nil you want to get nil for all 3, so
please do set all 3 to nil.

nodesAtPoint and init for WebElementDictionary should do the nodes in the same
order. Currently they don't.

WebElementDictionary was not included with the patch -- please add it.