[webkit-reviews] review granted: [Bug 55515] Implement proper handling of mouseover/mouseout events in regard to shadow DOM boundaries. : [Attachment 88935] More feedback addressed.

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has granted Dimitri Glazkov (Google)
<dglazkov at chromium.org>'s request for review:
Bug 55515: Implement proper handling of mouseover/mouseout events in regard to
shadow DOM boundaries.
https://bugs.webkit.org/show_bug.cgi?id=55515

Attachment 88935: More feedback addressed.
https://bugs.webkit.org/attachment.cgi?id=88935&action=review

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=88935&action=review

> Source/WebCore/dom/EventDispatcher.cpp:128
> +    // Assume divergent boundary is the relatedTarget itself (in other
words, related target ancestor chaing does not cross any shadow DOM
boundaries).

typo: chaing

> Source/WebCore/dom/EventDispatcher.cpp:154
> +	       targetAncestor = m_ancestors.begin();

Do you need to set this? Once diverged == true, we never care about the valur
of targetAncestor anymore, right?

> Source/WebCore/dom/EventDispatcher.cpp:170
> +    } else {
> +	   // Since ancestors does not contain target itself, we must account
> +	   // for the possibility that target is a shadowHost of relatedTarget
> +	   // and thus serves as the lowestCommonBoundary.
> +	   // Luckily, in this case the firstDivergentBoundary is target.
> +	   if ((*firstDivergentBoundary) == m_node.get())
> +	       lowestCommonBoundary = m_ancestors.begin();
> +    }

nit: this could be an else-if. your call.