Bug 65899 – A mouseover/mouseout event is not dispatched when a mouse moves from a direct child node of shadow root into a sibling node

• Component: UI Events
• Status: RESOLVED FIXED

[Hayato Ito]

Suppose the following Dom tree:

  <document>
    - <shadowhost> 
        .. shadowroot
             - <input> input1
             - <input> input2

When a mouse moves from input1 element into input2 element, mouseover/mouseout event should be dispatched at input2/input1 elements. But it doesn't.

[Hayato Ito]

Related issue is:
  https://bugs.webkit.org/show_bug.cgi?id=61979

I've found the root cause. I'll update the patch soon with a layout test.

[Hayato Ito]

Created attachment 103432 [details]
WIP - add some tests.

[Hayato Ito]

I've added a layout test. I'll add more tests later.

I've found yet another case where the current event handling implementation cannot handle nicely. I guess there are more corner cases that we should check.  I am sure I should rewrite most part of EventDispatcher::adjustRelatedTarget, EventDispatcher::adjustToShadowBoundaries and so on to handle these corner cases.

[Dimitri Glazkov (Google)]

Can you explain a bit more about what's happening? Is the mouseover/out dispatch not following the algorithm described in http://dglazkov.github.com/component-model/events.html?


The test you submitted is hard to understand by inspection.

[Hayato Ito]

Created attachment 103607 [details]
WIP

[Hayato Ito]

Hi Dimitry,
Thank you for the comment.

(In reply to comment #4)
> Can you explain a bit more about what's happening? Is the mouseover/out dispatch not following the algorithm described in http://dglazkov.github.com/component-model/events.html?
>
 
Let me update the current status.
I've uploaded a patch working in progress. But I think that's worth for reviewing. That fixes some corner cases. Please see a ChangeLog to see what this patch try to fix.

> The test you submitted is hard to understand by inspection.

Yeah, sorry for the inconvenience. It's difficult to write a readable test for these type of issues.
I've added more tests and slightly updated tests so that it can be more understandable. Let me share tests now as is. I'll update the tests later.

Actually I've tried to reuse fast/events/shadow-boundary-crossing.html, but I'd like to create new LayoutTests in order to adding more complex tests easily.

I appreciate if you could review. I am not 100% sure my understanding of how to handle events crossing shadow boundaries is correct...

[Dimitri Glazkov (Google)]

(In reply to comment #6)
> Hi Dimitry,
> Thank you for the comment.
> 
> (In reply to comment #4)
> > Can you explain a bit more about what's happening? Is the mouseover/out dispatch not following the algorithm described in http://dglazkov.github.com/component-model/events.html?
> >
> 
> Let me update the current status.
> I've uploaded a patch working in progress. But I think that's worth for reviewing. That fixes some corner cases. Please see a ChangeLog to see what this patch try to fix.

Ok.

> 
> > The test you submitted is hard to understand by inspection.
> 
> Yeah, sorry for the inconvenience. It's difficult to write a readable test for these type of issues.
> I've added more tests and slightly updated tests so that it can be more understandable. Let me share tests now as is. I'll update the tests later.
> 
> Actually I've tried to reuse fast/events/shadow-boundary-crossing.html, but I'd like to create new LayoutTests in order to adding more complex tests easily.

Sounds good. Can you move fast/events/shadow-boundary-crossing.html to fast/dom/shadow? It would be nice to keep all shadow DOM-related tests in one place.

> 
> I appreciate if you could review. I am not 100% sure my understanding of how to handle events crossing shadow boundaries is correct...

[Dimitri Glazkov (Google)]

Comment on attachment 103607 [details]
WIP

View in context: https://bugs.webkit.org/attachment.cgi?id=103607&action=review

> LayoutTests/fast/dom/shadow/shadow-boundary-events.html:51
> +    //   eventRecores[eventType] = ['target.id(@currentTarget.id)(<-relatedTarget.id)', 'target.id(@currentTarget.id)(<-relatedTarget.id)', ...]

eventRecores -> eventRecords

> LayoutTests/fast/dom/shadow/shadow-boundary-events.html:118
> +              'Move mouse from a node to its sibling node. Every nodes are outside of shadow boundary.');
> +    shouldBe('dispatchedEvent("mouseover")', '["div2(@div2)(<-div1)", "div2(@divA)(<-div1)"]');
> +    shouldBe('dispatchedEvent("mouseout")', '["div1(@div1)(<-div2)", "div1(@divA)(<-div2)"]');

I like this format! It's much easier to understand.

> LayoutTests/fast/dom/shadow/shadow-boundary-events.html:121
> +              'Target is an ancestor of realtedTarget. Every nodes are outside of shadow boundary.');

realtedTarget -> relatedTarget, Every -> All (here and elsewhere)

> LayoutTests/fast/dom/shadow/shadow-boundary-events.html:131
> +              'Both target and relatedTarget are direct child of the same shadow root.');

direct->immediate, child->children

> LayoutTests/fast/dom/shadow/shadow-boundary-events.html:166
> +              'Target and relatedTarget exist in separated subtree, crossing shadow boundaries. Making sure that event is not dispatched beyond the lowest command bandary.');

command bandary->common boundary

[Hayato Ito]

Hi Dimitri, thank you for the review.

I'll land this patch after addressing your comments.

(In reply to comment #7)
> 
> Sounds good. Can you move fast/events/shadow-boundary-crossing.html to fast/dom/shadow? It would be nice to keep all shadow DOM-related tests in one place.

Sure. I'll move it and include that move in this patch.

[Hayato Ito]

Committed r92922: <http://trac.webkit.org/changeset/92922>

[Scott Franklin]

Not sure if it's an issue with this patch or Chrome but,

Chromium 15.0.851.0
<details><summary>abc<canvas></canvas>def</summary></details>
and mouseover the canvas.

Observed behavior: sad tab
ASSERTION FAILED: targetAncestor - 1 >= m_ancestors.begin()
third_party/WebKit/Source/WebCore/dom/EventDispatcher.cpp(168) : WTF::PassRefPtr<WebCore::EventTarget> WebCore::EventDispatcher::adjustToShadowBoundaries(WTF::PassRefPtr<WebCore::Node>, WTF::Vector<WebCore::Node*, 0ul>)

Expected behavior: no sad tab

[Hayato Ito]

Thank you for the report. Let me take a look later.

(In reply to comment #11)
> Not sure if it's an issue with this patch or Chrome but,
> 
> Chromium 15.0.851.0
> <details><summary>abc<canvas></canvas>def</summary></details>
> and mouseover the canvas.
> 
> Observed behavior: sad tab
> ASSERTION FAILED: targetAncestor - 1 >= m_ancestors.begin()
> third_party/WebKit/Source/WebCore/dom/EventDispatcher.cpp(168) : WTF::PassRefPtr<WebCore::EventTarget> WebCore::EventDispatcher::adjustToShadowBoundaries(WTF::PassRefPtr<WebCore::Node>, WTF::Vector<WebCore::Node*, 0ul>)
> 
> Expected behavior: no sad tab

[Hayato Ito]

Files as https://bugs.webkit.org/show_bug.cgi?id=66210
Let me continue to investigate that.

(In reply to comment #12)
> Thank you for the report. Let me take a look later.
> 
> (In reply to comment #11)
> > Not sure if it's an issue with this patch or Chrome but,
> > 
> > Chromium 15.0.851.0
> > <details><summary>abc<canvas></canvas>def</summary></details>
> > and mouseover the canvas.
> > 
> > Observed behavior: sad tab
> > ASSERTION FAILED: targetAncestor - 1 >= m_ancestors.begin()
> > third_party/WebKit/Source/WebCore/dom/EventDispatcher.cpp(168) : WTF::PassRefPtr<WebCore::EventTarget> WebCore::EventDispatcher::adjustToShadowBoundaries(WTF::PassRefPtr<WebCore::Node>, WTF::Vector<WebCore::Node*, 0ul>)
> > 
> > Expected behavior: no sad tab