Closed Bug 432698 Opened 16 years ago Closed 13 years ago

mouseenter and mouseleave events are not supported

Categories

(Core :: DOM: Events, enhancement)

enhancement
Not set
normal

Tracking

()

VERIFIED FIXED
mozilla10
Tracking Status
firefox9 + wontfix
firefox10 --- verified
blocking2.0 --- -

People

(Reporter: neil, Assigned: smaug)

References

(Blocks 1 open bug, )

Details

(Keywords: dev-doc-complete, Whiteboard: [wanted1.9.3][qa!])

Attachments

(3 files, 3 obsolete files)

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.
Component: General → Event Handling
Product: Firefox → Core
Version: unspecified → Trunk
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: wanted1.9.1?
QA Contact: general → events
How are the two types of events different, exactly?
(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.
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
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
(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).
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.
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+
blocking2.0: --- → ?
Summary: mouseenter and mouseleave events are not supported by Firefox → mouseenter and mouseleave events are not supported
Component: Event Handling → DOM: Events
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.
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.
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
Not blocking 1.9.3 on this, but if we get a patch in time we'd certainly take a fix.
blocking2.0: ? → -
Whiteboard: [wanted1.9.3]
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
Taking.
Assignee: nobody → Olli.Pettay
Please support the the onmouseleave feature.
Status: NEW → ASSIGNED
Just an update, people are still asking for this:

http://www.quirksmode.org/blog/archives/2011/09/event_compatibi_1.html
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).
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)
Attached patch patch (obsolete) — Splinter Review
I need to write mochitests
Attached patch +some tests (obsolete) — Splinter Review
Attachment #560165 - Attachment is obsolete: true
Attachment #560183 - Flags: review?(masayuki)
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?
mouseenter/leave work like :hover, so they use DOM tree, not layout.
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
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 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?
 
> 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.
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?
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.
I don't see any extra mouseout/over events.
(And the problem with mouseenter/leave is easily fixable)
Blocks: 687088
I added MouseEnterLeaveDispatcher class which collects the needed
event targets and handles native anon problem.
Attachment #560183 - Attachment is obsolete: true
Attachment #560616 - Flags: review?(masayuki)
Attachment #560183 - Flags: review?(masayuki)
Can I use try build with the new patch?
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 ;-)
(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.
Attached patch patchSplinter Review
Add tests that mouseenter/leave happening inside iframe don't fire events in the
parent document.
Attachment #560616 - Attachment is obsolete: true
Attachment #560651 - Flags: review?(masayuki)
Attachment #560616 - Flags: review?(masayuki)
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.
Attachment #560651 - Flags: review?(masayuki) → review+
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!
> And that is what Presshell does.

I wanted to know this. Thank you.

> Thanks for the quick review!

np.
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).
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Keywords: dev-doc-needed
Target Milestone: --- → mozilla9
Depends on: 691059
current implementation doesn't take into account anonymous children
added by xbl

is this intentional or just a bug?
Attachment #565711 - Attachment mime type: application/octet-stream → text/xml
Attachment #565711 - Attachment mime type: text/xml → text/plain
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.
We want to back this out of aurora9 due to bug 691059. See bug 691059, comment 50
Attachment #566950 - Flags: approval-mozilla-aurora?
Attachment #566950 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flagged for QA verification in Firefox 10.
Whiteboard: [wanted1.9.3] → [wanted1.9.3][qa+]
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.
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?
If you don't use mouseenter/leave event listeners at all in the document, they won't be dispatched.
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.
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?
(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)
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.
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>
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)
> 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?
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
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.
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.
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).
> 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.
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).
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
Status: RESOLVED → VERIFIED
Whiteboard: [wanted1.9.3][qa+] → [wanted1.9.3][qa!]
Also verified as fixed on:
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:10.0) Gecko/20100101 Firefox/10.0.
You need to log in before you can comment on or make changes to this bug.