WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
18930
mouseenter and mouseleave events not supported
https://bugs.webkit.org/show_bug.cgi?id=18930
Summary
mouseenter and mouseleave events not supported
Neil Craig
Reported
2008-05-07 14:20:46 PDT
As per PPK's article, despite these being non-W3C events, they would be incredibly useful if supported. Further info (by PPK) is at:
http://www.quirksmode.org/dom/events/index.html
Attachments
Patch
(18.73 KB, patch)
2012-05-23 05:16 PDT
,
Takashi Sakamoto
dglazkov
: review-
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Archive of layout-test-results from ec2-cr-linux-04
(545.15 KB, application/zip)
2012-05-23 06:50 PDT
,
WebKit Review Bot
no flags
Details
Patch
(20.09 KB, patch)
2012-09-27 07:48 PDT
,
Allan Sandfeld Jensen
webkit.review.bot
: commit-queue-
Details
Formatted Diff
Diff
Patch
(20.38 KB, patch)
2012-09-27 08:15 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Patch
(20.40 KB, patch)
2012-09-27 08:18 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Patch
(28.38 KB, patch)
2013-04-11 05:34 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Patch
(27.00 KB, patch)
2013-04-26 02:17 PDT
,
Allan Sandfeld Jensen
no flags
Details
Formatted Diff
Diff
Show Obsolete
(5)
View All
Add attachment
proposed patch, testcase, etc.
Teun
Comment 1
2009-01-14 05:20:33 PST
This would be incredibly usefull to have, imho. In the mean time, use something like this:
http://blog.stchur.com/2007/03/15/mouseenter-and-mouseleave-events-for-firefox-and-other-non-ie-browsers/
Erik Arvidsson
Comment 2
2010-12-16 16:26:39 PST
I agree that this is useful and it is part of the latest draft of DOM Level 3 Events. When thinking through how to implement this we realized that there are issues with capturing events. Should we dispatch an event on every ancestor of the target? How many times does a capturing listener get called during a mouseenter? I don't have access to IE9 at the moment but I'm curious about the behavior there.
Alexey Proskuryakov
Comment 3
2010-12-17 12:25:29 PST
> would be incredibly useful
Could someone please post a rationale for having these events? Lots of devices don't even have a mouse these days. I'm not saying that these events shouldn't be implemented, but there's likely a difficult discussion ahead, and listing the reasons for having the events is essential.
Neal Kanodia
Comment 4
2011-01-05 12:42:04 PST
Currently, it's slow, error-prone, and requires extra bytes to implement standard idioms (e.g, a context menu). mouseenter/mouseleave mirror CSS :hover. Quirks mode has a good discussion of how the events work, and the boilerplate needed to test an onmouse[over/out] event to see if the mouse actually entered/left:
http://www.quirksmode.org/js/events_mouse.html
http://www.quirksmode.org/dom/events/mouseover.html
Dimitri Glazkov (Google)
Comment 5
2011-02-22 10:37:31 PST
(In reply to
comment #3
)
> > would be incredibly useful > > Could someone please post a rationale for having these events? Lots of devices don't even have a mouse these days. > > I'm not saying that these events shouldn't be implemented, but there's likely a difficult discussion ahead, and listing the reasons for having the events is essential.
Why would there be a difficult discussion? It seems like a no-brainer. It's a standard event and most browsers support it. Web devs want it. What's difficult?
Dimitri Glazkov (Google)
Comment 6
2011-02-22 10:40:04 PST
For the context of how I found this bug. We already hand-roll it in at least one place:
http://google.com/codesearch/p#OAMlx_jo-ck/src/third_party/WebKit/Source/WebCore/html/shadow/MediaControls.cpp&exact_package=chromium&l=543
And this implementation is flawed, because it doesn't account for multiple mouseover events fired, thus unnecessarily calling updateControlVisibility().
Dimitri Glazkov (Google)
Comment 7
2011-02-22 10:57:08 PST
(In reply to
comment #5
)
> (In reply to
comment #3
) > > > would be incredibly useful > > > > Could someone please post a rationale for having these events? Lots of devices don't even have a mouse these days. > > > > I'm not saying that these events shouldn't be implemented, but there's likely a difficult discussion ahead, and listing the reasons for having the events is essential. > > Why would there be a difficult discussion? It seems like a no-brainer. It's a standard event and most browsers support it. Web devs want it. What's difficult?
DOOH. Only IE supports them. Sorry about that :) Still, it seems that it's a pretty needed event. See ppk's shouting and search for jquery mouseenter/mouselave.
Alexey Proskuryakov
Comment 8
2011-02-22 11:19:00 PST
> Why would there be a difficult discussion?
I explained it in
comment 3
. There is no mouse on many devices used to access the Internet these days. If these are to be added to WebKit, it's compatibility and not "incredible usefulness" that should drive adding them, in my opinion.
Dimitri Glazkov (Google)
Comment 9
2011-02-22 11:41:59 PST
(In reply to
comment #8
)
> > Why would there be a difficult discussion? > > I explained it in
comment 3
. There is no mouse on many devices used to access the Internet these days. If these are to be added to WebKit, it's compatibility and not "incredible usefulness" that should drive adding them, in my opinion.
Can you explain a bit more here? For no-mouse devices, sure, these events would not be very useful. But there are plenty (still the absolute majority, right?) of devices with mice and there's lots of JS code written trying to solve (inefficiently) this same problem. Why not help out the Web developers?
Xavier Morel
Comment 10
2011-09-06 05:14:01 PDT
(In reply to
comment #8
)
> > Why would there be a difficult discussion? > > I explained it in
comment 3
. There is no mouse on many devices used to access the Internet these days.
Well sure but Webkit supports *all* other mouse event types (even mousewheel) and mouseenter and mouseleave are part of the DOM Level 3 events[0], so I'm not sure the objection still has much (if any) relevance, especially since mouseenter and mouseleave are far more useful than *already included* mouse events (namely mouseover and mouseout)
> If these are to be added to WebKit, it's compatibility and not "incredible usefulness" that should drive adding them, in my opinion.
Why would "incredible usefulness" not play a part? Because they *are* incredibly useful (in that they're useful at all, which mouseover and mouseout aren't exactly, you usually have to hack around *a lot* to make them useable) [0]
http://www.w3.org/TR/DOM-Level-3-Events/#event-type-mouseenter
Jarred Nicholls
Comment 11
2011-09-06 05:30:32 PDT
Consider the 4 year old
bug #13343
, incorrect margin right computed style values. It was old, there were workarounds everywhere, and devs were dealing with it. It didn't stop us from fixing it. Now both IE and Opera 11 support these events. Let's leave Mozilla in the dust and just do it, or likely the opposite will occur.
Olli Pettay (:smaug)
Comment 12
2011-09-18 09:50:18 PDT
(In reply to
comment #11
)
> Now both IE and Opera 11 support these events. Let's leave Mozilla in the > dust and just do it, or likely the opposite will occur.
Heh. Just FYI, Webkit is now the only engine not supporting mouseenter/leave. There are some differences in the implementations, and I'll file also some (HTML) spec bugs.
Bronislav Klucka
Comment 13
2012-04-15 09:16:36 PDT
FF is already supporting those events....
Takashi Sakamoto
Comment 14
2012-05-23 05:16:10 PDT
Created
attachment 143540
[details]
Patch
Olli Pettay (:smaug)
Comment 15
2012-05-23 05:23:54 PDT
I don't understand how the patch works. You may need to send multiple events if mouse is moved to be on top of some DOM subtree to be over another DOM subtree. All the elements in the subtrees get mouseleave or mouseenter.
Jarred Nicholls
Comment 16
2012-05-23 05:27:35 PDT
Comment on
attachment 143540
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=143540&action=review
> Source/WebCore/page/EventHandler.cpp:2206 > + // sned mouseleave event to the old node if the new node is not a descendant of the old node.
Typo: s/sned/send Wouldn't the proper behavior be to fire the events on all nodes, not just the "old node"? Any of the nodes could have a mouseenter/mouseleave event handler attached. Maybe I'm missing something, but a simple explanation is warranted I believe.
Jarred Nicholls
Comment 17
2012-05-23 05:30:01 PDT
Comment on
attachment 143540
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=143540&action=review
> Source/WebCore/page/EventHandler.cpp:2215 > + // send mouseenter event to the new node if the old node is not a descendant of the new node.
Same here, again wouldn't you fire the event on all nodes and not just the "new node"?
WebKit Review Bot
Comment 18
2012-05-23 06:50:24 PDT
Comment on
attachment 143540
[details]
Patch
Attachment 143540
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/12769225
New failing tests: fast/events/mouseover-mouseout2.html
WebKit Review Bot
Comment 19
2012-05-23 06:50:37 PDT
Created
attachment 143560
[details]
Archive of layout-test-results from ec2-cr-linux-04 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-04 Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'> Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Dimitri Glazkov (Google)
Comment 20
2012-05-23 08:41:51 PDT
Comment on
attachment 143540
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=143540&action=review
Glad to see you are trying to fix this! This is a bit trickier than it seems at first. Here are a bunch of comments you may find useful:
> Source/WebCore/ChangeLog:6 > + Implemented mouseenter and mouseleave events, because firefox has already supported these
I see Firefox is getting no respect -- all lower-case :) Actually, a more accurate statement would be "because all other browsers support them".
> Source/WebCore/ChangeLog:28 > + can-bubble and not cancelable. These are different from other mouse events.
"are not can-bubble" -> don't bubble. The last sentence is extraneous.
> Source/WebCore/ChangeLog:32 > + Added mouseenterAttr nad mouseleaveAttr in the same manner as mouseoverAttr
Just "added respective attributes" is enough.
> Source/WebCore/ChangeLog:37 > + Added mouseover and mouseleave interfaces.
They are not interfaces. Just attributes.
>> Source/WebCore/page/EventHandler.cpp:2206 >> + // sned mouseleave event to the old node if the new node is not a descendant of the old node. > > Typo: s/sned/send > > Wouldn't the proper behavior be to fire the events on all nodes, not just the "old node"? Any of the nodes could have a mouseenter/mouseleave event handler attached. Maybe I'm missing something, but a simple explanation is warranted I believe.
Jarred and Olli are right. You need to consider this: what effect does entering/leaving node A to B have on ancestors of A and B? What does it mean in terms dispatching events? Additionally, since you will be dispatching multiple events in an existing code path, we must consider the performance impact of this change. I would be very careful here and make sure we don't regress.
> LayoutTests/fast/events/mouseenter-mouseleave.html:39 > +<div id="inner2" style="width:20px; height:20px; background-color:yellow; top:30px; left:60px; position:absolute"
Probably need a few more tests based on discussion so far.
Bronislav Klucka
Comment 21
2012-07-13 09:01:13 PDT
any news here?
Allan Sandfeld Jensen
Comment 22
2012-09-27 07:48:00 PDT
Created
attachment 165998
[details]
Patch A new modified version of Takashi's patch. The logic for issuing the events have been rewritten to be more correct with respect to all the nodes that should and should not have these events issued, but has an optimization where they are not issued when there is no listeners for them. This last part should avoid any performance regressions, but at the cost that mouseevent and mouseleave will not always working as might be expected for capture phase event listeners installed on ancestor elements (They will just have to rely on mouseover and mouseout instead).
Erik Arvidsson
Comment 23
2012-09-27 08:06:23 PDT
Comment on
attachment 165998
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=165998&action=review
> Source/WebCore/dom/Node.cpp:2739 > + if (!hovered() && hasEventListeners(eventNames().mouseleaveEvent)) {
hasEventListeners is not correct. We need to check if any of the ancestors have an event listener for this.
Allan Sandfeld Jensen
Comment 24
2012-09-27 08:11:32 PDT
(In reply to
comment #23
)
> (From update of
attachment 165998
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=165998&action=review
> > > Source/WebCore/dom/Node.cpp:2739 > > + if (!hovered() && hasEventListeners(eventNames().mouseleaveEvent)) { > > hasEventListeners is not correct. We need to check if any of the ancestors have an event listener for this.
Not when the event doesn't bubble, of course if we want capture to work, then we would need to check all the ancestor.
WebKit Review Bot
Comment 25
2012-09-27 08:13:29 PDT
Comment on
attachment 165998
[details]
Patch
Attachment 165998
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://queues.webkit.org/results/14035847
Allan Sandfeld Jensen
Comment 26
2012-09-27 08:15:35 PDT
Created
attachment 166005
[details]
Patch Update the test-case so it actually tests a case where a single mouseout/mouseover event becomes multiple mouseenter/mouseleave events.
Allan Sandfeld Jensen
Comment 27
2012-09-27 08:18:23 PDT
Created
attachment 166007
[details]
Patch Fix build issue on Chromium.
Erik Arvidsson
Comment 28
2012-09-27 08:18:49 PDT
(In reply to
comment #22
)
> ...will not always working as might be expected for capture phase event listeners installed on ancestor elements (They will just have to rely on mouseover and mouseout instead).
That seems unacceptable to me. (In reply to
comment #24
)
> Not when the event doesn't bubble, of course if we want capture to work, then we would need to check all the ancestor.
We do want capture to work. We need to potentially dispatch an event on every ancestor. I think the cleanest thing to do here is to define a custom dispatch. This way we can gather the ancestors once instead of once per ancestor. This custom dispatch can check if there is a listener too. In the case where there are no listeners we walk the ancestors once. In the case where there are listeners the dispatch will still be O(n^2) where n is the depth of the tree.
Allan Sandfeld Jensen
Comment 29
2012-09-28 02:47:55 PDT
(In reply to
comment #28
)
> (In reply to
comment #22
) > > ...will not always working as might be expected for capture phase event listeners installed on ancestor elements (They will just have to rely on mouseover and mouseout instead). > > That seems unacceptable to me. >
Okay, I thought it was a very clean and easily communicable limitation. It even has easy workaround for anybody who wants to use capture event listeners, they just need to add empty listeners to the elements they want events for.
> (In reply to
comment #24
) > I think the cleanest thing to do here is to define a custom dispatch. This way we can gather the ancestors once instead of once per ancestor. This custom dispatch can check if there is a listener too. In the case where there are no listeners we walk the ancestors once. In the case where there are listeners the dispatch will still be O(n^2) where n is the depth of the tree.
Checking the ancestors is not the only thing that cause O(n²) behaviour. There is also a Node::contains() check to avoid wrong mouseleave events. My plan for a correct solution is to eventually move the handling to Document::updateHoverActiveState, which has a list of nodes whose hover state changes. The problem is that it currently operates in hittesting and not in event-handling, so it does not have access to all the mouse-event parameters it needs to dispatch a mouse event. To avoid iterating over all the ancestors again and again, we would need a mechanism to collect the ancestors for event dispatching once, and reuse it for several events. The problem with all this is that it requires a few refactorings. The solution I posted here was a fast easy way to solve the problem with one clear and simple limitation, which can be used until we have a full implementation.
Michał Gołębiowski-Owczarek
Comment 30
2013-01-10 16:53:44 PST
Any progress on this issue? Just asking because last activity happened 4 months ago.
Allan Sandfeld Jensen
Comment 31
2013-04-11 05:34:19 PDT
Created
attachment 197580
[details]
Patch New rebased patch, now with support for capturing event handlers too
Allan Sandfeld Jensen
Comment 32
2013-04-20 03:50:07 PDT
Discussion on webkit-dev raised no objections and Maciej wrote: 'Seems like a reasonable feature to add.'. The feature has also been discussed on blink-dev, and blink is just waiting for the final patch before porting. Now that the political side about whether we should have the feature in this form has been decided on, does anyone dare review the details of the implementation?
Dave Hyatt
Comment 33
2013-04-25 10:31:55 PDT
Comment on
attachment 197580
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=197580&action=review
r=me
> Source/WebCore/dom/Document.cpp:5994 > + bool seenCommonAncestor = false;
This should be "sawCommonAncestor"
Allan Sandfeld Jensen
Comment 34
2013-04-26 02:17:21 PDT
Created
attachment 199805
[details]
Patch
WebKit Commit Bot
Comment 35
2013-04-26 02:41:06 PDT
Comment on
attachment 199805
[details]
Patch Clearing flags on attachment: 199805 Committed
r149173
: <
http://trac.webkit.org/changeset/149173
>
WebKit Commit Bot
Comment 36
2013-04-26 02:41:11 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug