[webkit-reviews] review denied: [Bug 3439] mouseover effects can get stuck sometimes due to missing events : [Attachment 7169] proposed fix

[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 3439: mouseover effects can get stuck sometimes due to missing events
http://bugzilla.opendarwin.org/show_bug.cgi?id=3439

Attachment 7169: proposed fix
http://bugzilla.opendarwin.org/attachment.cgi?id=7169&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I like the general direction of this patch a lot.

But I don't like the idea of adding the subframe function to
MouseEventWithHitTestResults. It's fine to have that helper function, but I
don't think it should be a member function of the event class. I think we can
just have a local function in FrameView.cpp (not a member of any class) that
takes the mouse event or node as a parameter. Might need a clearer name than
"subframe", or "subframe" might be OK.

+	     if (m_oldUnder && m_oldUnder->getDocument() !=
frame()->document())

In the check above, do we also want to check inDocument() on m_oldUnder?

Does FrameView::dispatchMouseEvent handle the case where targetNode is
destroyed as a side effect of handling the mouseout or mouseover event? I think
we may need to ref it.

By the way, the "pass subframe event to subframe" code might need to be moved
into FrameView eventually -- the strange way this code is distributed across
FrameView and Frame is an artifact of us not wanting to modify the KHTML code
much in the old days (so we put stuff in KWQKHTMLPart, now the
Macintosh-specific part of Frame).

Setting review- mainly because of the subframe function, but please also
consider the other comments.