432698 - mouseenter and mouseleave events are not supported

Status: VERIFIED FIXED

(Core :: DOM: Events, enhancement)

Description

[neil]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-GB; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-GB; rv:1.9b5) Gecko/2008032619 Firefox/3.0b5

I realise these are not W3C events but they would massively simplify javascript coding. 

See PPK's info: http://www.quirksmode.org/dom/events/index.html (last 2 items on the page)

Reproducible: Always

Steps to Reproduce:
1.
2.
3.

Comment 1

[John Resig]

This would be immensely useful to web developers. Right now all sorts of work has to be done in order to make mouseover/mouseout behave in this manner.

Comment 2

[Johnny Stenback (:jst)]

How are the two types of events different, exactly?

Comment 3

[John Resig]

(In reply to comment #2)
> How are the two types of events different, exactly?

They correspond as mouseleave:mouseout and mouseenter:mouseover - but the difference is that they only fire for the element that they're bound to (not the child elements). This makes it really easy to do actions equivalent to the CSS :hover - or complex actions like tree navigation. The problem with using mouseout/mouseover is that you're constantly having to verify where you are in the tree and only executing when you're mousing in/out of the correct element.

Comment 4

[Josh Rosenbaum]

I'm having to deal with this same exact issue right at this moment. I have to code a work around rather than having a simple solution.
Rough Example:
div1 onmouseover='x' onmouseout='y'
  img1 img2 img3
/div1

Every time you mouse over a different child(image) the mouseout/mouseover for the parent div fire :( mouseleave/mouseenter do not fire at all while inside the div. That would be handy.

Here's an exact link to the quirks mode page that describes it. (Posted above minus bookmark.) Currently only IE has it, but apparently it's been there since 5.5 according to the page.

http://www.quirksmode.org/dom/events/index.html#t28

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

W3C WebApps/DOMEvents WG discussed about this, and the conclusion[1] was to not 
to include mouseenter/leave in the next version (3) of the DOM Events spec.
Using current mouseover/out it is easy to get the the same information, 
just use .target and .currentTarget or .stopPropagation() at the right place. 

[1] http://lists.w3.org/Archives/Public/public-webapps/2008JulSep/0180.html

Comment 6

[Josh Rosenbaum]

(In reply to comment #5)
Thanks for the info Smaug.

That's a shame. :( Things in the web development world move so slow, because we have to wait for the old browsers to die. It would've been nice to get these into the standards now so we could have some hope of using them in the near future. I don't know if anyone can still push for this, but if so, I think it'd be very nice.

While it wasn't super hard to work around mouseover/mouseout, it still took me multiple lines of code to do what I wanted. I basically had to write a function to do the work. It would've been a lot nicer to do something like:
div1 onmouseenter="this.className='foo';" onmouseleave="'this.className='bar';"
/div

You can use mouseover and mouseout, but the problem is they cause flickering in some browsers, because of the changeover. For my more complex case, I had about 20-30 lines of code (and I used YUI) to do something I could've done in one as part of the actual element(and been much easier to read).

Comment 7

[Bill Wallace]

I agree, it isn't in the standard, you can implement it with the existing options.  So what?  If all that was ever included in the standards and browsers was a minimal set needed to allow JS to handle events, then there would be exactly 1 event handler, and it would handle all event types for all objects.  Almost everyone would hate that because it would require a huge amount of programming efort for the simplest tasks.  
The fact is, it is useful for people writing pages to be able to do more with less code.  Adding support for mouse enter/leave is a perfect example of when that occurs.  It makes it easier for many of us to not worry about having to write code that computes enter/leave based on the desired DOM element and over/out.

Comment 8

[Emil Ivanov]

Document Object Model (DOM) Level 3 Events Specification
W3C Working Draft 8 September 2009

mouseenter/mouseleave are in the latest draft

http://www.w3.org/TR/2009/WD-DOM-Level-3-Events-20090908/#event-type-mouseenter

These events are useful and could be used as soon as they are implemented in all major non-IE browsers, because they are already supported from IE5.5+

Comment 9

[Olli Pettay [:smaug][[e-mail]]]

Yes, we (DOM 3 Events subgroup) did add those events to the draft. But the
draft needs still reviewing. (Feel free to review and send comments to the
W3C DOM mailing list)
DOM 3 Events is hopefully going to last call pretty soon.
But still, since the DOM 3 Events is far from being a recommendation, I wouldn't
implement the events quite yet.

The latests editor's draft is here 
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html
IIRC, it will be updated in the next few days.

Comment 10

[Josh Rosenbaum]

This is great news to hear these were added to the draft. :) These standards work so slow I had sort of lost hope of seeing this in the next 10 years.

I doubt DOM 3 Events is going to break the way it is done in Internet Explorer. That seems like it would be a MONSTROUS backwards-compatibility problem. If that's not going to be broken, then I don't see any problem with implementing this now. Internet Explorer has had this since 5.5. That's a very long time. We're only waiting on the rest of the browsers to get this! Even if this was implemented today, we'd still have to wait years to realistically use it, so I really hope it gets implemented soon.

Comment 11

[neil]

Agreed Josh...

I now use the Mootools framework which implements the mouseenter and mouseleave events (though obviously they're not native) so I would imagine (possibly naively) that it wouldn't be that hard to do?

I really think this is an important event and Mozilla/Firefox could lead the way in the decent browsers by implementing these events.

Many thanks to all the Moz developers/contributors

Comment 12

[Johnny Stenback (:jst)]

Not blocking 1.9.3 on this, but if we get a patch in time we'd certainly take a fix.

Comment 13

[Neil Craig]

I'd gladly submit a patch if I had the skills/experience. I am promising myself I will soon.

MooTools (and probably other Javascript frameworks) has support, I wonder if someone could use that as a starting point?

http://mootools.net/download

Thanks

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Taking.

Comment 15

[iamio]

Please support the the onmouseleave feature.

Comment 16

[neil]

Just an update, people are still asking for this:

http://www.quirksmode.org/blog/archives/2011/09/event_compatibi_1.html

Comment 17

[Martijn Wargers (dead)]

Some years ago, I wrote a patch for this, but I lost it. I found it quite easy to implement (but of course I might have done it completely wrong).

Comment 18

[Olli Pettay [:smaug][[e-mail]]]

FYI, I'm writing patch for this, but getting this working fast needs some minor tweaking.
(using mouseenter/leave is in general slower than mouseout/over since there needs to be
massively more events)

Comment 19

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

I need to write mochitests

Comment 20

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +some tests

Comment 21

[Olli Pettay [:smaug][[e-mail]]]

opt builds http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-9b251b871b3d/

tests still running https://tbpl.mozilla.org/?tree=Try&usebuildbot=1&rev=792ac6cafdc7

Comment 22

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Smaug, I have a question before review.

I read the spec but I'm not sure about the timing to dispatch. Descendents can be overflowed from its ancestors or laid out outside of the ancestors. Your patch looks like using only DOM tree, that means it ignores the actual box layout. For example:

div {
  width: 400px;
  height: 100px;
  overflow: visible;
}

div > div {
  position: relative;
  left: 200px;
}

<div>outer's content
  <div>inner's content</div>
</div>

Then, it's rendered as:

+---------------------------------------------+
| outer's content                             |
|                       +------------------------------------------+
|      A                | inner's content                      B   |
+-----------------------|                                          |
                        |                                          |
                        +------------------------------------------+

In this case, even when mouse cursor is moved from A to B, mouseleave event isn't fired by your patch, right?

Have you discussed about this issue in W3C?

Comment 23

[Olli Pettay [:smaug][[e-mail]]]

mouseenter/leave work like :hover, so they use DOM tree, not layout.

Comment 24

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 560183 [details] [diff] [review]
+some tests

>diff --git a/content/events/src/nsEventStateManager.cpp b/content/events/src/nsEventStateManager.cpp
>--- a/content/events/src/nsEventStateManager.cpp
>+++ b/content/events/src/nsEventStateManager.cpp
>@@ -3809,16 +3809,19 @@ nsEventStateManager::DispatchMouseEvent(
>   event.refPoint = aEvent->refPoint;
>   event.isShift = ((nsMouseEvent*)aEvent)->isShift;
>   event.isControl = ((nsMouseEvent*)aEvent)->isControl;
>   event.isAlt = ((nsMouseEvent*)aEvent)->isAlt;
>   event.isMeta = ((nsMouseEvent*)aEvent)->isMeta;
>   event.pluginEvent = ((nsMouseEvent*)aEvent)->pluginEvent;
>   event.relatedTarget = aRelatedContent;
>   event.inputSource = static_cast<nsMouseEvent*>(aEvent)->inputSource;
>+  if (event.message == NS_MOUSEENTER || event.message == NS_MOUSELEAVE) {
>+    event.flags |= NS_EVENT_FLAG_CANT_BUBBLE;
>+  }

How about to do this in nsMouseEvent? See also following comment.

>@@ -3879,26 +3889,50 @@ nsEventStateManager::NotifyMouseOut(nsGU
>   // Don't touch hover state if aMovingInto is non-null.  Caller will update
>   // hover state itself, and we have optimizations for hover switching between
>   // two nearby elements both deep in the DOM tree that would be defeated by
>   // switching the hover state to null here.
>   if (!aMovingInto) {
>     // Unset :hover
>     SetContentState(nsnull, NS_EVENT_STATE_HOVER);
>   }
>-  
>+
>+  PRBool wantsEnterLeave = ShouldFireMouseEnterLeave(mLastMouseOverElement);
>+  nsCOMArray<nsIContent> mouseLeaveTargets;
>+  if (wantsEnterLeave) {
>+    nsINode* commonParent = nsnull;
>+    if (mLastMouseOverElement && aMovingInto) {
>+      commonParent =
>+        nsContentUtils::GetCommonAncestor(mLastMouseOverElement, aMovingInto);
>+    }
>+    nsIContent* current = mLastMouseOverElement;
>+    // Note, it is ok if commonParent is null!
>+    while (current && current != commonParent) {
>+      mouseLeaveTargets.AppendObject(current);
>+      // mouseenter/leave is fired only on elements.
>+      current = current->GetParent();
>+    }
>+  }
>+
>   // Fire mouseout
>   DispatchMouseEvent(aEvent, NS_MOUSE_EXIT_SYNTH,
>                      mLastMouseOverElement, aMovingInto);
>   
>   mLastMouseOverFrame = nsnull;
>   mLastMouseOverElement = nsnull;
>   
>   // Turn recursion protection back off
>   mFirstMouseOutEventElement = nsnull;
>+
>+  if (wantsEnterLeave) {
>+    for (PRInt32 i = 0; i < mouseLeaveTargets.Count(); ++i) {
>+      DispatchMouseEvent(aEvent, NS_MOUSELEAVE, mouseLeaveTargets[i],
>+                         aMovingInto);
>+    }
>+  }
> }

Though, I find a bug. When I move mouse cursor over a native anonymous node like <input> or scrollbars, extra mouseout, mouseover, mouseenter and mouseleave events whose originalTarget is probably the anonymous node are fired. I think we shouldn't retarget the events to content but I'm not sure where should we fix for it.

I think that it's okay you fix it in another bug, but I think we shouldn't release the new D3E events without the fix.

>diff --git a/content/events/test/test_bug432698.html b/content/events/test/test_bug432698.html
>new file mode 100644
>--- /dev/null
>+++ b/content/events/test/test_bug432698.html
>@@ -0,0 +1,102 @@
>+<!DOCTYPE HTML>
>+<html>
>+<!--
>+https://bugzilla.mozilla.org/show_bug.cgi?id=432698
>+-->
>+<head>
>+  <title>Test for Bug 432698</title>
>+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
>+  <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
>+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
>+</head>
>+<body>
>+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=432698">Mozilla Bug 432698</a>
>+<p id="display"></p>
>+<div id="content" style="display: none">
>+  
>+</div>
>+<pre id="test">
>+<script type="application/javascript">
>+
>+/** Test for Bug 432698 **/
>+SimpleTest.waitForExplicitFinish();
>+SimpleTest.waitForFocus(runTests);
>+var outer;
>+var middle;
>+var inner;
>+var outside;
>+var container;
>+var mouseentercount = 0;
>+var mouseleavecount = 0;
>+var mouseovercount = 0;
>+var mouseoutcount = 0;
>+
>+function sendMouseEvent(t, elem) {
>+  var r = elem.getBoundingClientRect();
>+  synthesizeMouse(elem, r.width / 2, r.height / 2, {type: t});
>+}
>+
>+function runTests() {
>+  outer = document.getElementById("outertest");
>+  middle = document.getElementById("middletest");
>+  inner = document.getElementById("innertest");
>+  outside = document.getElementById("outside");
>+  container = document.getElementById("container");
>+  // Make sure ESM thinks mouse is outside the test elements.
>+  sendMouseEvent("mousemove", outside);
>+  mouseentercount = 0;
>+  mouseleavecount = 0;
>+  mouseovercount = 0;
>+  mouseoutcount = 0;
>+  sendMouseEvent("mousemove", inner);
>+  is(mouseentercount, 3, "Unexpected mouseenter event count!");
>+  is(mouseovercount, 1, "Unexpected mouseover event count!");
>+  is(mouseoutcount, 0, "Unexpected mouseout event count!");
>+  is(mouseleavecount, 0, "Unexpected mouseleave event count!");
>+  sendMouseEvent("mousemove", outside);
>+  is(mouseentercount, 3, "Unexpected mouseenter event count!");
>+  is(mouseovercount, 1, "Unexpected mouseover event count!");
>+  is(mouseoutcount, 1, "Unexpected mouseout event count!");
>+  is(mouseleavecount, 3, "Unexpected mouseleave event count!");
>+
>+  SimpleTest.finish();
>+}
>+
>+function menter(evt) {
>+  ++mouseentercount;
>+  evt.stopPropagation();
>+  is(evt.target, evt.currentTarget, "Wrong event target!");
>+}
>+
>+function mleave(evt) {
>+  ++mouseleavecount;
>+  evt.stopPropagation();
>+  is(evt.target, evt.currentTarget, "Wrong event target!");
>+}
>+
>+function mover(evt) {
>+  ++mouseovercount;
>+  evt.stopPropagation();
>+}
>+
>+function mout(evt) {
>+  ++mouseoutcount;
>+  evt.stopPropagation();
>+}
>+
>+</script>
>+</pre>
>+<div id="container" onmouseenter="menter(event)" onmouseleave="mleave(event)"
>+                    onmouseout="mout(event)" onmouseover="mover(event)">
>+<div id="outside" onmouseout="event.stopPropagation()" onmouseover="event.stopPropagation()">foo</div>
>+<div id="outertest" onmouseenter="menter(event)" onmouseleave="mleave(event)"
>+                    onmouseout="mout(event)" onmouseover="mover(event)">
>+  <div id="middletest" onmouseenter="menter(event)" onmouseleave="mleave(event)"
>+                       onmouseout="mout(event)" onmouseover="mover(event)">
>+    <div id="innertest" onmouseenter="menter(event)" onmouseleave="mleave(event)"
>+                        onmouseout="mout(event)" onmouseover="mover(event)">foo</div>
>+  </div>
>+</div>
>+<div>
>+</body>
>+</html>

I think that you should test with <iframe> for testing document boundary and scrollbars for testing native anonymous node case and scrolled case for testing synthesized mouse move events.

>diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h
>--- a/dom/base/nsPIDOMWindow.h
>+++ b/dom/base/nsPIDOMWindow.h
>@@ -75,18 +75,18 @@ class nsIDocument;
> class nsIScriptTimeoutHandler;
> struct nsTimeout;
> class nsScriptObjectHolder;
> class nsXBLPrototypeHandler;
> class nsIArray;
> class nsPIWindowRoot;
> 
> #define NS_PIDOMWINDOW_IID \
>-{ 0xeee816d2, 0x2f08, 0x4b34, \
>-  { 0x97, 0x47, 0x5e, 0x5a, 0xcd, 0xc3, 0x56, 0xfa } }
>+{ 0x8ce567b5, 0xcc8d, 0x410b, \
>+  { 0xa2, 0x7b, 0x07, 0xaf, 0x31, 0xc0, 0x33, 0xb8 } }

Do you need to change the uuid? It seems that you don't change vtable of this interface, but I'm not sure.

>diff --git a/widget/public/nsGUIEvent.h b/widget/public/nsGUIEvent.h
>--- a/widget/public/nsGUIEvent.h
>+++ b/widget/public/nsGUIEvent.h

> 875   nsMouseEvent(PRBool isTrusted, PRUint32 msg, nsIWidget *w,
> 876                PRUint8 structType, reasonType aReason)
> 877     : nsMouseEvent_base(isTrusted, msg, w, structType),
> 878       acceptActivation(PR_FALSE), ignoreRootScrollFrame(PR_FALSE),
> 879       reason(aReason), context(eNormal), exit(eChild), clickCount(0)
> 880   {
> 881     if (msg == NS_MOUSE_MOVE) {
> 882       flags |= NS_EVENT_FLAG_CANT_CANCEL;
> 883     }
> 884   }

I think you should change here. mouseenter and mouseleave are not cancelable.
http://dev.w3.org/2006/webapi/DOM-Level-3-Events/html/DOM3-Events.html#event-type-mouseenter

Comment 25

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I tested with:

> data:text/html,<body><input type="text"> <span>here is span element. <br>second line of same span element. </span><br><iframe src="about:blank"></iframe><div style="width:100px;height:100px;overflow:auto;"><div style="width:1000px;height:1000px;"></div></div><p id="p" style="white-space: nowrap;"></p></body><script>function logSafe(event, prop) { try { return event[prop].toString(); } catch (e) { return "error(" + e + ")"; } } function log(event) { var p = document.getElementById("p"); p.innerHTML = "target: " + event.target.tagName + " event: " + event.type + " { clientX: " + event.clientX + ", clientY: " + event.clientY + ", button: " + event.button + ", clickCount: " + event.detail + ", target: " + event.target.toString() + ", originalTarget: " + logSafe(event, "originalTarget") + ", explicitOriginalTarget: " + logSafe(event, "explicitOriginalTarget") + " }<BR>" + p.innerHTML; } window.addEventListener("mouseenter", log, true); window.addEventListener("mouseleave", log, true); window.addEventListener("mouseover", log, true); window.addEventListener("mouseout", log, true);</script>

Comment 26

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 560183 [details] [diff] [review]
+some tests

>diff --git a/widget/public/nsGUIEvent.h b/widget/public/nsGUIEvent.h
>--- a/widget/public/nsGUIEvent.h
>+++ b/widget/public/nsGUIEvent.h
>@@ -257,16 +257,18 @@ class nsHashKey;
> #define NS_MOUSE_ENTER                  (NS_MOUSE_MESSAGE_START + 22)
> #define NS_MOUSE_EXIT                   (NS_MOUSE_MESSAGE_START + 23)
> #define NS_MOUSE_DOUBLECLICK            (NS_MOUSE_MESSAGE_START + 24)
> #define NS_MOUSE_CLICK                  (NS_MOUSE_MESSAGE_START + 27)
> #define NS_MOUSE_ACTIVATE               (NS_MOUSE_MESSAGE_START + 30)
> #define NS_MOUSE_ENTER_SYNTH            (NS_MOUSE_MESSAGE_START + 31)
> #define NS_MOUSE_EXIT_SYNTH             (NS_MOUSE_MESSAGE_START + 32)
> #define NS_MOUSE_MOZHITTEST             (NS_MOUSE_MESSAGE_START + 33)
>+#define NS_MOUSEENTER                   (NS_MOUSE_MESSAGE_START + 34)
>+#define NS_MOUSELEAVE                   (NS_MOUSE_MESSAGE_START + 35)

Ah, we already have NS_MOUSE_ENTER for the entering native widget. I think that we should rename them later:

NS_MOUSE_ENTER to NS_MOUSE_ENTER_WIDGET
NS_MOUSE_EXIT to NS_MOUSE_EXIT_WIDGET
new events to NS_MOUSE_ENTER and NS_MOUSE_LEAVE

How do you think about this?

Comment 27

[Olli Pettay [:smaug][[e-mail]]]

 
> Though, I find a bug. When I move mouse cursor over a native anonymous node
> like <input> or scrollbars, extra mouseout, mouseover,
Hmm, nsGenericElement should handle mouseout/over.

> mouseenter and
> mouseleave events whose originalTarget is probably the anonymous node are
> fired.
Good catch! I totally forgot native anon. I'll fix it in this bug.




r bug, but I think we shouldn't
> release the new D3E events without the fix.
> > #define NS_PIDOMWINDOW_IID \
> >-{ 0xeee816d2, 0x2f08, 0x4b34, \
> >-  { 0x97, 0x47, 0x5e, 0x5a, 0xcd, 0xc3, 0x56, 0xfa } }
> >+{ 0x8ce567b5, 0xcc8d, 0x410b, \
> >+  { 0xa2, 0x7b, 0x07, 0xaf, 0x31, 0xc0, 0x33, 0xb8 } }
> 
> Do you need to change the uuid? It seems that you don't change vtable of
> this interface, but I'm not sure.
I do change IID whenever I change the interface. It should be ok.


 
> I think you should change here. mouseenter and mouseleave are not cancelable.
Ah, indeed.

Comment 28

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

http://dev.w3.org/html5/spec/webappapis.html#event-handlers-on-elements-document-objects-and-window-objects

onmouseenter and onmouseleave are not defined as event handler attribute, why do you add them? For compatibility with other browsers?

Comment 29

[Olli Pettay [:smaug][[e-mail]]]

mouseenter/leave are originally from IE which has onmouseenter/leave properties, so yes,
compatibility with IE is the reason.
HTML spec is just missing the feature. I'll file a spec bug to get onmouseenter/leave added there.

Comment 30

[Olli Pettay [:smaug][[e-mail]]]

I don't see any extra mouseout/over events.
(And the problem with mouseenter/leave is easily fixable)

Comment 31

[Olli Pettay [:smaug][[e-mail]]]

Attachment: +more tests, correct native anon handling

I added MouseEnterLeaveDispatcher class which collects the needed
event targets and handles native anon problem.

Comment 32

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Can I use try build with the new patch?

Comment 33

[Olli Pettay [:smaug][[e-mail]]]

http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-fd530858da7f/

Comment 34

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 560616 [details] [diff] [review]
+more tests, correct native anon handling

>+class MouseEnterLeaveDispatcher
>+{
>+public:
>+  MouseEnterLeaveDispatcher(nsEventStateManager* aESM,
>+                            nsIContent* aTarget, nsIContent* aRelatedTarget,
>+                            nsGUIEvent* aEvent, PRUint32 aType)
>+  : mESM(aESM), mEvent(aEvent), mType(aType)
>+  {
>+    nsPIDOMWindow* win =
>+      aTarget ? aTarget->GetOwnerDoc()->GetInnerWindow() : nsnull;
>+    if (win && win->HasMouseEnterLeaveEventListeners()) {
>+      mRelatedTarget = aRelatedTarget ?
>+        aRelatedTarget->FindFirstNonNativeAnonymous() : nsnull;
>+      nsINode* commonParent = nsnull;
>+      if (aTarget && aRelatedTarget) {
>+        commonParent =
>+          nsContentUtils::GetCommonAncestor(aTarget, aRelatedTarget);
>+      }
>+      nsIContent* current = aTarget;
>+      // Note, it is ok if commonParent is null!
>+      while (current && current != commonParent) {
>+        if (!current->IsInNativeAnonymousSubtree()) {
>+          mTargets.AppendObject(current);
>+        }
>+        // mouseenter/leave is fired only on elements.
>+        current = current->GetParent();
>+      }
>+    }
>+  }

nit: you can use early return for aTarget and win && win->HasMouseEnterLeaveEventListeners(). But I don't mind.

>+  ~MouseEnterLeaveDispatcher()
>+  {
>+    for (PRInt32 i = 0; i < mTargets.Count(); ++i) {
>+      mESM->DispatchMouseEvent(mEvent, mType, mTargets[i], mRelatedTarget);
>+    }
>+  }

Are you sure that mESM must not be destroyed before here? I'm not sure about the lifetime of ESM.

>diff --git a/content/events/test/Makefile.in b/content/events/test/Makefile.in
>--- a/content/events/test/Makefile.in
>+++ b/content/events/test/Makefile.in
>@@ -105,16 +105,17 @@ _TEST_FILES = \
> 		test_bug656379-2.html \
> 		test_bug656954.html \
> 		test_bug659350.html \
> 		test_bug662678.html \
> 		test_bug667919-1.html \
> 		test_bug667919-2.html \
> 		test_bug667612.html \
> 		empty.js \
>+		test_bug432698.html \
> 		$(NULL)

The file names should be sorted by but id though...

Anyway, why don't you add the test to chrome test? It uses synthesizeMouse() and it needs to use PrivilegeManager. I heard that we shouldn't use it in new plain tests.

>+  // Initialize iframe
>+  iframe.contentDocument.documentElement.style.overflow = "hidden";
>+  iframe.contentDocument.body.style.margin = "0px";
>+  iframe.contentDocument.body.style.width = "100%";
>+  iframe.contentDocument.body.style.height = "100%";
>+  iframe.contentDocument.body.offsetLeft;
>+
>+  iframe.contentDocument.body.onmouseenter = menter;
>+  iframe.contentDocument.body.onmouseleave = mleave;
>+  r = iframe.getBoundingClientRect();
>+  expectedRelatedEnter = null;
>+  expectedRelatedLeave = null;
>+  synthesizeMouse(iframe.contentDocument.body, r.width / 2, r.height / 2, {type: "mousemove"},
>+                  iframe.contentWindow);
>+  is(mouseentercount, 5, "Unexpected mouseenter event count!");
>+  sendMouseEvent("mousemove", outside);
>+  is(mouseleavecount, 5, "Unexpected mouseleave event count!");

Oh, I meant that you should test the events of iframe. Even when mouse cursor is on another child document's element, the iframe's element shouldn't receive the mouseenter and mouseleave events.

Otherwise, looks ok for me. I'll give + after your reply to the ESM lifetime issue.

# I'd like you to review my composition event patch next week ;-)

Comment 35

[Olli Pettay [:smaug][[e-mail]]]

(In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #34)
> 
> Are you sure that mESM must not be destroyed before here? I'm not sure about
> the lifetime of ESM.
MouseEnterLeaveDispatcher is used only on stack, so the lifetime handling of
ESM is as safe as it has been so far.


> 
> The file names should be sorted by but id though...
Merging problem



> 
> Anyway, why don't you add the test to chrome test? It uses synthesizeMouse()
> and it needs to use PrivilegeManager. I heard that we shouldn't use it in
> new plain tests.

Using synthesizeMouse is ok, since it is part of testing framework.
I assume someone will convert synthesizeMouse to use SpecialPowers.
Also, I added the warning that mouseenter/leave shouldn't be used in
chrome, since they are a bit slower than mouseout/over (because there just happens
to be a lot more mouseenter/leave events than mouseout/over)
 
> Oh, I meant that you should test the events of iframe. Even when mouse
> cursor is on another child document's element, the iframe's element
> shouldn't receive the mouseenter and mouseleave events.
ah. I'll add that test.

 
> # I'd like you to review my composition event patch next week ;-)
I will. Actually, I'll try to start reviewing them even today.
Sorry about the delay.

Comment 36

[Olli Pettay [:smaug][[e-mail]]]

Attachment: patch

Add tests that mouseenter/leave happening inside iframe don't fire events in the
parent document.

Comment 37

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Comment on attachment 560651 [details] [diff] [review]
patch

(In reply to Olli Pettay [:smaug] from comment #35)
> (In reply to Masayuki Nakano (:masayuki) (Mozilla Japan) from comment #34)
> > 
> > Are you sure that mESM must not be destroyed before here? I'm not sure about
> > the lifetime of ESM.
> MouseEnterLeaveDispatcher is used only on stack, so the lifetime handling of
> ESM is as safe as it has been so far.

But mouseout/mouseover events are dispatched before MouseEnterLeaveDispatcher is destroyed. So, if the ESM can be destroyed by the event handlers, it may cause crash. But if a method which is ancestor in call stack holds the ESM, it's safe. I'm not sure about this issue.

> > Anyway, why don't you add the test to chrome test? It uses synthesizeMouse()
> > and it needs to use PrivilegeManager. I heard that we shouldn't use it in
> > new plain tests.
> 
> Using synthesizeMouse is ok, since it is part of testing framework.
> I assume someone will convert synthesizeMouse to use SpecialPowers.
> Also, I added the warning that mouseenter/leave shouldn't be used in
> chrome, since they are a bit slower than mouseout/over (because there just
> happens
> to be a lot more mouseenter/leave events than mouseout/over)

OK.

> > Oh, I meant that you should test the events of iframe. Even when mouse
> > cursor is on another child document's element, the iframe's element
> > shouldn't receive the mouseenter and mouseleave events.
> ah. I'll add that test.

Thanks.

If my worry about ESM isn't a problem, r=masayuki.

Comment 38

[Olli Pettay [:smaug][[e-mail]]]

The ESM lifetime handling doesn't change. The current code does something like
dispatch mouseout, dispatch mouseover. (mouseout may already close the window or do something else
unexpected). The patch adds just few more events.
Also, it is a COM rule that caller must keep the object, in this case ESM, alive.
And that is what Presshell does. It has nsRefPtr<nsEventStateManager> manager, and
Pre/PostHandleMethods are called using that variable.

Thanks for the quick review!

Comment 39

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

> And that is what Presshell does.

I wanted to know this. Thank you.

> Thanks for the quick review!

np.

Comment 40

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/mozilla-central/rev/9e3c4b3e0e04

Please test on Nightly (probably tomorrow's Nightly) and report bugs
(file new bugs and make them 'block' this one).

Comment 41

[[e-mail]]

Attachment: testcase for anonymous nodes

current implementation doesn't take into account anonymous children
added by xbl

is this intentional or just a bug?

Comment 42

[Olli Pettay [:smaug][[e-mail]]]

It is intentional. XBL doesn't add native anonymous children.
Also mouseover/out have special casing only for native anonymous content.

If you're using XBL, you're expected to use .originalTarget when needed.

Comment 43

[LegNeato]

We want to back this out of aurora9 due to bug 691059. See bug 691059, comment 50

Comment 44

[Olli Pettay [:smaug][[e-mail]]]

Attachment: Backout patch for aurora

Comment 45

[Olli Pettay [:smaug][[e-mail]]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/218c4fca3396

Comment 46

[u279076]

Flagged for QA verification in Firefox 10.

Comment 47

[Jean-Yves Perrier [:teoli]]

Added new doc: 
https://developer.mozilla.org/en/DOM/DOM_event_reference/mouseenter
https://developer.mozilla.org/en/DOM/DOM_event_reference/mouseleave

Completed:
https://developer.mozilla.org/en/Firefox_10_for_developers
https://developer.mozilla.org/en/DOM/DOM_event_reference

Feel free to tell here, or directly complete the artice, if you see something wrong or think about missing information.

Comment 48

[Olli Pettay [:smaug][[e-mail]]]

Thanks for documenting.
Unfortunately that isn't quite right, or at least could be more precise.
The important thing is that usually there are many mouseenter/leave events dispatched.
They don't bubble, but each element inside a subtree the mouse is leaving or entering gets an event.
So, for performance critical code which is handling deep DOM hierarchies, I wouldn't use
mouseenter/leave.

Comment 49

[Jean-Yves Perrier [:teoli]]

I see. A quick question before I update the articles:

May the performance problem (with deep DOM hierarchies) also appear if no event handler (for mouseenter/leave) is attached to most objects?

Comment 50

[Olli Pettay [:smaug][[e-mail]]]

If you don't use mouseenter/leave event listeners at all in the document, they won't be dispatched.

Comment 51

[Olli Pettay [:smaug][[e-mail]]]

So, to clarify, if mouse is moved to be inside some deeply nested element hierarchy (say 100 levels),
there will be 100 mouseenter events created, but possibly only 1 mouseover.

Comment 52

[dziastinux]

I am sorry to interrupt, but should mouseenter/mouseleave events be bound to mouseover/mouseout events?

Please confirm if this is correct: one mouseenter event listener is created on element which is positioned -1000px to the left. The rest of the page contains 2.5k nested elements. So every mouse movement through any of the elements will generate mouseenter and mouseleave events as if there where listener on all body elements?

Comment 53

[Olli Pettay [:smaug][[e-mail]]]

(In reply to dziastinux from comment #52)
> I am sorry to interrupt, but should mouseenter/mouseleave events be bound to
> mouseover/mouseout events?
I don't know what you're asking here.

> Please confirm if this is correct: one mouseenter event listener is created
> on element which is positioned -1000px to the left. The rest of the page
> contains 2.5k nested elements. So every mouse movement through any of the
> elements will generate mouseenter and mouseleave events as if there where
> listener on all body elements?
I don't understand the example.
Could you send me the example using email, or ping me on IRC (moznet #developers)

Comment 54

[Jean-Yves Perrier [:teoli]]

Thanks Olli for your time, you're input is very valuable. I've updated the two pages:
https://developer.mozilla.org/en/DOM/DOM_event_reference/mouseenter
https://developer.mozilla.org/en/DOM/DOM_event_reference/mouseleave

Hopefully it is ok now.

Comment 55

[Rob Crowther]

I'm not sure if this is what dziastinux is asking, but I'm having a hard time understanding how mouseenter/leave fires more events than mouseover/out.  The whole point of mouseenter/leave is that they fire fewer times than mouseover/out, as per the simple example at the end of this jQuery documentation:

http://api.jquery.com/mouseleave/

If you move the mouse pointer from outside this div to 'Content' then handler() fires 3 times:
 
<div onmouseover="handler()">
   <div>
      <p>Content<p>
   </div>
</div>

If you do the same on these elements, handler() fires once:

<div onmouseenter="handler()">
   <div>
      <p>Content<p>
   </div>
</div>

Comment 56

[Jean-Yves Perrier [:teoli]]

Rob, your example doesn't show what really happens. 

In your first example, one single mouseover event is sent on the <p> element, then it bubbles up to handler(), how far is determined by the cancellation inside the handler.

I just tried it on http://jsfiddle.net/MPnk3/1/ If you uncomment evt.stopPropagation(), the alert is fired only once, as I understood it from the explaination of Olli.

In the second case, three events are sent in all cases. (This didn't appear in your case as there isn't a handler on each elements, which doesn't mean that no event is sent)

Comment 57

[Rob Crowther]

> Rob, your example doesn't show what really happens.

I disagree, it shows what web developers have really been trying to do for years, hence the jQuery (and many other) implementation(s).

> In the second case, three events are sent in all cases.

If it's the exact same number, I'm still not understanding how mouseenter/leave create 'many more events' than mouseover/out?  Why is the latter so much better for performance as per the new documentation?

Comment 58

[Olli Pettay [:smaug][[e-mail]]]

jQuery probably adds event listeners only to bubble phase, so there it doesn't see more than
one event. But if you add listeners to capturing phase, you may see several mouseenter/leave
events but only one mouseover/out. This behavior is the same in IE, Opera and Firefox(Aurora and
 Nightly).

You get several mouseenter/leave events because if you move mouse from being over
one DOM subtree to being over another DOM subtree, all the elements in those subtrees get an event.
But depending on css styling and how fast you move mouse etc. only the deepest element in the
subtrees may get mouseout/over.
http://mozilla.pettay.fi/moztests/events/mouseenterleaveoverout.html

Comment 59

[Rob Crowther]

OK, I think I understand what you're saying, mouseenter/leave don't play well with capturing event handlers (which, considering their ancestry, is hardly surprising).  But surely the advice in the documentation should state the performance problems occur only with capturing event listeners, and therefore the advice should be to not use capturing event listeners for mouseenter/leave?  And even then, only when you have thousands of deeply nested elements.  Really I think that's far from a common use case.

Comment 60

[Rob Crowther]

Also, if the situation is reversed, the warning could be just as well added to the mouseover/out documentation:

http://jsfiddle.net/robertc/JQvyS/

And as a final comment, if it's appropriate to continue raising issues with the documentation here, why is the mouseleave/enter documentation here:

/DOM/DOM_event_reference/mouseleave

While the mouseover/out documentation is here:

/DOM/element.onmouseover

Is there some sort of re-org that's only part completed?  I was disappointed to not be able to just change the event name to switch between the two.

Comment 61

[dziastinux]

I can hardly find a reason why events that do not bubble actually bubble in capturing phase. 

I have a few questions about this example: http://mozilla.pettay.fi/moztests/events/mouseenterleaveoverout.html
If there where only one event listener(mouseenter, bubbling phase) in whole document on element with id `0`. No mouseenter events should be created in capturing phase. Am I right?

The main point for mouseenter/ mouseleave events is not convenience (we have jQuery for that) but performance. This means that native implementation must be faster than event simulation. Web developers really really do not care about mouseenter event capturing phase. 

P.s. I have experience with mouseenter/ mouseleave events with IE7/8. Yes, there is a little performance gain (~15%) when using native mouseenter and mouseleave events instead of simulated ones (~200 listeners, tree depth 15-30 elements).

Comment 62

[Rob Crowther]

> The main point for mouseenter/ mouseleave events is not convenience (we
> have jQuery for that)

I disagree, convenience is absolutely one of the main points, adding an entire JS library just to get two events is not convenient.

Comment 63

[Olli Pettay [:smaug][[e-mail]]]

The whole point for mouseenter/leave is convenience. You can achieve the exactly same functionality
using mouseover/out + other DOM APIs.
mouseenter/leave are nice for certain things, but it is, IMHO, good to tell to the web devs that
using those events may affect to the performance (especially because it is not very obvious
that there can be several mouseenter/leave events created whenever mouse moves to a new event target).

Comment 64

[Ioana (away)]

Verified as fixed using the URL from the bug (http://www.quirksmode.org/dom/events/tests/mouseover.html) on:
Mozilla/5.0 (Windows NT 6.1; rv:10.0) Gecko/20100101 Firefox/10.0
Mozilla/5.0 (X11; Linux i686; rv:10.0) Gecko/20100101 Firefox/10.0

Comment 65

[Ioana (away)]

Also verified as fixed on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0.