Bug 128534 - 'mouseenter' mouse compat event not fired when listeners for touch events
Summary: 'mouseenter' mouse compat event not fired when listeners for touch events
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Event Handling (show other bugs)
Version: 528+ (Nightly build)
Hardware: iPhone / iPad iOS 7.0
: P3 Minor
Assignee: Benjamin Poulain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-10 07:59 PST by Patrick H. Lauke
Modified: 2014-03-22 09:15 PDT (History)
7 users (show)

See Also:


Attachments
Event test for touch, pointer, MSPointer, mouse events (3.55 KB, text/html)
2014-02-11 03:12 PST, Patrick H. Lauke
no flags Details
Event test just for mouse events (3.04 KB, text/html)
2014-02-11 03:13 PST, Patrick H. Lauke
no flags Details
iOS/Safari when zoomed out - no mouseenter fired on "all events" test (55.19 KB, image/png)
2014-02-11 03:37 PST, Patrick H. Lauke
no flags Details
iOS/Safari when zoomed in - mouseenter fired on "all events" test (55.00 KB, image/png)
2014-02-11 03:38 PST, Patrick H. Lauke
no flags Details
Patch (22.48 KB, patch)
2014-02-13 21:49 PST, Benjamin Poulain
kling: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick H. Lauke 2014-02-10 07:59:31 PST
Not sure if it's a bug or a "feature", but in any case it's a strange inconsistency and I'd like clarification if it's intended behavior or not: it seems that "mouseenter" mouse compatibility event is only fired if there are no touch* events also being listened to.

on iOS/Safari, go to http://patrickhlauke.github.io/touch/tests/event-listener_mouse-only.html and tap the test button. This page only shows the mouse events, and the sequence of events for a clean tap are usually:

mousenter > mouseover > mousemove > mousedown > mouseup > click

now, on http://patrickhlauke.github.io/touch/tests/event-listener_all-no-timings.html doing the same clean tap results in:

touchstart > touchend > mouseover >  mousemove > mousedown > mouseup > click

Note that "mouseenter" is absent in this case. The only difference in the scripts here is that the second example also listens for touch events (as well as pointer events, with and without MS prefix, but surely that shouldn't have any effect).
Comment 1 Benjamin Poulain 2014-02-10 20:01:14 PST
That looks like a input event bug. Next time you can CC me on Twitter too :)

Can I attach the two files to the bug for future reference?
Comment 2 Patrick H. Lauke 2014-02-11 03:12:32 PST
Created attachment 223834 [details]
Event test for touch, pointer, MSPointer, mouse events
Comment 3 Patrick H. Lauke 2014-02-11 03:13:00 PST
Created attachment 223835 [details]
Event test just for mouse events
Comment 4 Patrick H. Lauke 2014-02-11 03:13:10 PST
Cheers Ben. I assume you're @awfulben ?

I made the two tests self-contained (stuck the styles directly in there) and attached them.
Comment 5 Patrick H. Lauke 2014-02-11 03:37:24 PST
Created attachment 223836 [details]
iOS/Safari when zoomed out - no mouseenter fired on "all events" test
Comment 6 Patrick H. Lauke 2014-02-11 03:38:39 PST
Created attachment 223838 [details]
iOS/Safari when zoomed in - mouseenter fired on "all events" test

Just testing this further, some more weirdness which may point towards the cause of the bug: it seems that mouseenter is not fired in my test when Safari is zoomed out. If I pinch-zoom into my test page, mouseenter DOES fire.
Comment 7 Benjamin Poulain 2014-02-11 13:57:40 PST
(In reply to comment #4)
> Cheers Ben. I assume you're @awfulben ?

I am :)

> I made the two tests self-contained (stuck the styles directly in there) and attached them.

Thanks for attaching the tests.

I can reproduce locally, I will take the bug.

I suspect we changes a hover state when delivering the touch events, and when we generate the synthetic mouse events eventHandler thinks mouseenter was already delivered. Just a wild guess though, I'll find out when I'll have time to debug this.
Comment 8 Benjamin Poulain 2014-02-13 21:49:09 PST
Created attachment 224159 [details]
Patch
Comment 9 Andreas Kling 2014-02-18 14:29:59 PST
Comment on attachment 224159 [details]
Patch

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

r=me

> Source/WebCore/ChangeLog:8
> +        The code dispatchign mouseenter and mouseleave events was making the assumption that

dispatching

> Source/WebCore/ChangeLog:15
> +        processing of mouse events. The update is done along the events mouseover and mouseout.

along what?

> Source/WebCore/dom/Document.cpp:5873
> +    for (size_t i = 0; i < removeCount; ++i)

range for

> Source/WebCore/page/EventHandler.cpp:2384
> +            RenderElement* oldHoverObj = m_lastElementUnderMouse ? m_lastElementUnderMouse->renderer() : nullptr;
> +            RenderElement* newHoverObj = m_elementUnderMouse ? m_elementUnderMouse->renderer() : nullptr;

I'd call these *Renderer instead of *Obj.

> Source/WebCore/page/EventHandler.cpp:2387
> +            Vector<RefPtr<Element>, 32> leftElementsChain;

This could be Vector<Ref<Element>> since it never contains null pointers.

> Source/WebCore/page/EventHandler.cpp:2400
> +            Vector<RefPtr<Element>, 32> enteredElementsChain;

Ditto.

> Source/WebCore/page/EventHandler.cpp:2415
> +            for (size_t i = 0; i < leftElementsChain.size(); ++i) {

range for

> Source/WebCore/page/EventHandler.cpp:2425
> +            for (size_t i = 0, size = enteredElementsChain.size(); i < size; ++i) {

range for
Comment 10 Benjamin Poulain 2014-02-21 13:24:55 PST
Committed r164495: <http://trac.webkit.org/changeset/164495>