Fix/mouseenter event

[stepancar]

Fix for #10109

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[Andarist]

I would suspect that the 'sibling' event of mouseleave suffers from the same thing, this should be checked and fixed in the same PR I think (if indeed broken).

[Andarist]

!nativeEvent.relatedTarget.disabled - is this needed? I think it wont even be emitted by the browser when relatedTarget is disabled

[stepancar]

yes, its needed. Try to run it

[stepancar]

@Andarist #10149 (comment)
mouseleave should not be called on disabled button

[gaearon]

I don't know if this is the right fix. Leaving to @aweary @jquense @nhunzaker to review.

[jquense]

I'm not sure this fixes the bug...it seems like it's fix an opposite problem. mouseenter on some sibling depends on mouseout from the fromElement element existing. Checking if the fromElement is disabled would assume it already exists, but the browser decided to fire the event despite the element being disabled (do any browsers do this?). Checking if the element is disabled would prevent a mouseenter from firing, not cause it to

[gaearon]

Should we close this?

[jquense]

i think so...I'm trying to specifically identify what is wrong with the test.

[gaearon]

I'll leave this to you then :-P

[jquense]

Ok sorry I see what's happening. This change causes the plugin to take a path that is meant solely for the "entering an element from outside the browser window". and changes it to "there is an unpaired mouseover". It works for this one case, but I am not certain if it will lead to extra mouseenters being fired in other cases. It heavily depends on the browsers NOT firing the paired mouseout on the disabled button. If that changes or is not consistent across browsers we'll start getting double mouseenters

Do any of ya'll know what the situation is on browsers not firing these events. I remember hearing that was starting to change?

[gaearon]

I don't know. Unfortunately I don't think anyone on the team will have a lot of time this half to concentrate on the DOM issues, so if this isn't obviously the right fix, I think we won't take it.

[jquense]

ya fair enough. I generally thing the "right" fix is to use the native events

Thanks for the PR @stepancar we want to try and address these mouseenter/leave issues a bit more holistically. I appreciate the contribution and if you'r interested please join in the conversation on #11972

addendum: This is a really shaky part of the react codebase, and while generally we want to take any fixes to bugs, even if they are short term ones. It's also more dangerous than normal to mess with this code and can end up causes more issues in other places we didn't see due to not great coverage. So we take a far more cautious approach here than we might otherwise