[webkit-reviews] review denied: [Bug 10338] When contentEditable, cursor doesn't change to hand - Need WebPreference : [Attachment 10416] patch for comment

[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 10338: When contentEditable, cursor doesn't change to hand - Need
WebPreference
http://bugzilla.opendarwin.org/show_bug.cgi?id=10338

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

------- Additional Comments from Darin Adler <darin at apple.com>
This needs a test case and a change log.

Also, I think the patch has rotted because of changes to
Frame::selectionController().

The actual code looks fine, although it's complicated enough that it might need
some comments to explain what's going on. In particular, the extra code in
selectCursor is verbose and all on one line. I'd like to see it made clearer,
perhaps by writing an inline helper function with a suitable name and an
appropriate comment.

As you say, I'm not sure the new behavior is OK for all WebKit clients. I'd
like to hear opinions about that from Tim Hatcher and Justin Garcia on this
point.

In the HTMLAnchorElement functions, are we guaranteed that the document will
have a non-nil frame? In particular I'm concerned about the default event
handler code, because I think events can be dispatched to a node after a
document is no longer in a browser window. The new code may not be the only
code in the file that's making the incorrect assumption that the document will
have a non-0 frame.