[webkit-reviews] review denied: [Bug 12511] <use> has event dispatching issues : [Attachment 12883] Final patch

[bugzilla-request-daemon at macosforge.org]

Eric Seidel <macdome at opendarwin.org> has denied Eric Seidel
<macdome at opendarwin.org>'s request for review:
Bug 12511: <use> has event dispatching issues
http://bugs.webkit.org/show_bug.cgi?id=12511

Attachment 12883: Final patch
http://bugs.webkit.org/attachment.cgi?id=12883&action=edit

------- Additional Comments from Eric Seidel <macdome at opendarwin.org>
This could really be r+'d but since you're around and there are some minor
issues, we might as well go one more round.

1.  you owe a test, given your previous comments about ElementInstance :)

2. dispatchEvent won't compile as-is, without changes to EventTargetNode. 
Also, you might consider making dispatchEvent just take an additional target
parameter, and defautl it to "this" instead of making a separate method.  You'd
have to change all the places that override it though...  I could go either way
on that.  It's a little ocnfusing as to which one shoudl be virtual and which
one should be called (unless one is marked private) though w/o that change.

if (isSVGElement())
return this
is still ugly.	But at least you gave a nice comment in the code.  Maybe that
method shoudl be virtual?

shadowTreeParentElementForShadowTreeElement could probably be written a little
more succinctly as a do while loop, but it's fine...

You should note in your changelog that you fixed the <use> crash.  Ideally we'd
also have a test for that crash (I know, no one made a reduction of it yet, but
it's probably a pretty simple <use> with display: none)