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
Summary: A mouseover/mouseout event is not dispatched when a mouse moves from a direct...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: UI Events (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hayato Ito
URL:
Keywords:
Depends on:
Blocks: 59805 64249
  Show dependency treegraph
 
Reported: 2011-08-08 22:31 PDT by Hayato Ito
Modified: 2011-08-14 18:49 PDT (History)
5 users (show)

See Also:


Attachments
WIP - add some tests. (8.79 KB, patch)
2011-08-09 19:42 PDT, Hayato Ito
no flags Details | Formatted Diff | Diff
WIP (12.69 KB, patch)
2011-08-11 06:01 PDT, Hayato Ito
dglazkov: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hayato Ito 2011-08-08 22:31:49 PDT
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.
Comment 1 Hayato Ito 2011-08-08 22:33:17 PDT
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.
Comment 2 Hayato Ito 2011-08-09 19:42:55 PDT
Created attachment 103432 [details]
WIP - add some tests.
Comment 3 Hayato Ito 2011-08-09 19:48:02 PDT
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.
Comment 4 Dimitri Glazkov (Google) 2011-08-10 09:03:18 PDT
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.
Comment 5 Hayato Ito 2011-08-11 06:01:24 PDT
Created attachment 103607 [details]
WIP
Comment 6 Hayato Ito 2011-08-11 06:10:00 PDT
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...
Comment 7 Dimitri Glazkov (Google) 2011-08-11 08:42:43 PDT
(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...
Comment 8 Dimitri Glazkov (Google) 2011-08-11 08:49:57 PDT
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
Comment 9 Hayato Ito 2011-08-11 21:42:22 PDT
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.
Comment 10 Hayato Ito 2011-08-11 22:18:54 PDT
Committed r92922: <http://trac.webkit.org/changeset/92922>
Comment 11 Scott Franklin 2011-08-12 16:24:16 PDT
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
Comment 12 Hayato Ito 2011-08-14 16:14:13 PDT
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
Comment 13 Hayato Ito 2011-08-14 18:49:55 PDT
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