Bug 52065 – REGRESSION(r71934): Shadow DOM nodes leak via relatedTarget

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jonathan Vingiano]

Other browsers tested: 
     Safari 5.0.3: OK
     Firefox 3.6.13: OK
     Opera 10.63: OK
     Opera 11: OK
     Rockmelt 0.8.40.147: OK

What steps will reproduce the problem?
1. Create a div with an <input> as a child element. 
2. Attach a mouseenter/leave event to this div.
3. Chrome will see a mouse over of the child input as a mouseleave event, even though it is a child of the parent <div>

What is the expected result?

Expected that Chrome will not consider mousing over the child <input> field as a mouseleave event of the parent div.

What happens instead?

Chrome considers the mousing over the child <input> as a mouseleave event even though it is not.

Please provide any additional information below. Attach a screenshot if
possible.

http://jsfiddle.net/qs9f2/
http://bugs.jquery.com/ticket/7729

[Darth]

This bug applies to all platforms btw.

[Jonathan Vingiano]

(In reply to comment #1)
> This bug applies to all platforms btw.

Figured but I couldn't test on any other platform so I didn't want to speak too soon. Will change it, thanks for testing.

[Darth]

Another site that seems to be exhibiting this issue
http://www.movietickets.com/default.asp

Mouse over the choose your movie box in the center top.
This will open up a overlay div that has an input box, mouse over the input box and you will see the overlay disappears.

[Jonathan Vingiano]

Another realworld example of this bug has been pointed out at http://www.desiringgod.org

After hovering over the sign-in link, a field will appear with input fields. Hovering over an input field will hide the whole div, which is unexpected UX and does not happen in non-Webkit browsers.

[Dimitri Glazkov (Google)]

Whoopsie.

[Dimitri Glazkov (Google)]

Here's the reduction: http://jsfiddle.net/dglazkov/qs9f2/20/

[Dimitri Glazkov (Google)]

Created attachment 81679 [details]
Patch

[Darin Adler]

Comment on attachment 81679 [details]
Patch

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

> Source/WebCore/dom/Node.cpp:2876
> -    RefPtr<Node> relatedTarget = relatedTargetArg;
> +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);

Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?

[Dimitri Glazkov (Google)]

(In reply to comment #8)
> (From update of attachment 81679 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> 
> > Source/WebCore/dom/Node.cpp:2876
> > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> 
> Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?

The relatedTarget only needs to move if we have (In reply to comment #8)
> (From update of attachment 81679 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> 
> > Source/WebCore/dom/Node.cpp:2876
> > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> 
> Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?

This is not the best or final solution, but it's enough to stop the leaks and won't exhibit gross side effects in the current uses of shadow DOM.

The event will either be stopped if the relatedTarget is inside of the same shadow boundary as target or retargeted to the boundary that's closest to the target. Doing this without DOM scopes (see bug 52963) is possible but super-tedious.

[Dimitri Glazkov (Google)]

(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 81679 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> > 
> > > Source/WebCore/dom/Node.cpp:2876
> > > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> > 
> > Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?
> 
> The relatedTarget only needs to move if we have (In reply to comment #8)
> > (From update of attachment 81679 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> > 
> > > Source/WebCore/dom/Node.cpp:2876
> > > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> > 
> > Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?
> 
> This is not the best or final solution, but it's enough to stop the leaks and won't exhibit gross side effects in the current uses of shadow DOM.
> 
> The event will either be stopped if the relatedTarget is inside of the same shadow boundary as target or retargeted to the boundary that's closest to the target. Doing this without DOM scopes (see bug 52963) is possible but super-tedious.

Oh wow, I clearly over-edited this :)

The last paragraph describes the proper solution.

[WebKit Commit Bot]

Comment on attachment 81679 [details]
Patch

Rejecting attachment 81679 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-8', 'build-..." exit_code: 2

Last 500 characters of output:
ng/hebrew .......
fast/events ........................................................................................................................................................................................................................................
fast/events/shadow-boundary-crossing.html -> failed

Exiting early after 1 failures. 8052 tests run.
170.56s total testing time

8051 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
4 test cases (<1%) had stderr output

Full output: http://queues.webkit.org/results/7791271

[Dimitri Glazkov (Google)]

Ah. I should really update results foist. :)

[Dimitri Glazkov (Google)]

Committed r78077: <http://trac.webkit.org/changeset/78077>

[WebKit Review Bot]

http://trac.webkit.org/changeset/78077 might have broken GTK Linux 64-bit Debug

[Dimitri Glazkov (Google)]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (From update of attachment 81679 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> > > 
> > > > Source/WebCore/dom/Node.cpp:2876
> > > > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > > > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> > > 
> > > Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?
> > 
> > The relatedTarget only needs to move if we have (In reply to comment #8)
> > > (From update of attachment 81679 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=81679&action=review
> > > 
> > > > Source/WebCore/dom/Node.cpp:2876
> > > > -    RefPtr<Node> relatedTarget = relatedTargetArg;
> > > > +    RefPtr<Node> relatedTarget = pullOutOfShadow(relatedTargetArg);
> > > 
> > > Is there really just one level or this? Doesn’t the event travel out of multiple layers of shadow, each time needing a different relatedTarget?
> > 
> > This is not the best or final solution, but it's enough to stop the leaks and won't exhibit gross side effects in the current uses of shadow DOM.
> > 
> > The event will either be stopped if the relatedTarget is inside of the same shadow boundary as target or retargeted to the boundary that's closest to the target. Doing this without DOM scopes (see bug 52963) is possible but super-tedious.
> 
> Oh wow, I clearly over-edited this :)
> 
> The last paragraph describes the proper solution.

... and it turns out I need it sooner than I thought.