[webkit-reviews] review granted: [Bug 6844] elementAtPoint returns the list when the point is over a list marker : [Attachment 6689] patch

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 6844: elementAtPoint returns the list when the point is over a list marker
http://bugzilla.opendarwin.org/show_bug.cgi?id=6844

Attachment 6689: patch
http://bugzilla.opendarwin.org/attachment.cgi?id=6689&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
+    bool insideMarker = m_marker->nodeAtPoint(i, x, y, tx + m_x, ty + m_y,
hitTestAction);
+    if (insideMarker)
+	 setInnerNode(i);
+    return insideMarker;

To match the logic above, I'd do instead:

+    if (!m_marker->nodeAtPoint(i, x, y, tx + m_x, ty + m_y, hitTestAction)
+	 return false;
+
+    setInnerNode(i);
+    return true;

But obviously the code is OK as-is, just a suggestion.

RenderListMarker::nodeAtPoint does not set the innerNode when it returns true.
That means that if anyone other than RenderListItem::nodeAtPoint calls it, it
won't work properly. Either it should call setInnerNode or it should not be
called nodeAtPoint.

The patch otherwise looks good. I'm going to say r=me, because the nodeAtPoint
quibble is a minor one.