fix: make EventHandler better handle mouseenter/mouseleave events

[alpadev]

This PR makes the EventHandler handle mouseenter and mouseleave more like their native counterparts.

Background

Currently the EventHandler is implemented to simply map mouseenter to mouseover and mouseleave to mouseout events - see this comment #31646 (comment)

The problem is that mouseenter and mouseleave behave different to some extent but this wasn't respected yet. As such there are situations e.g. within the tooltip plugin where one does expect nested elements not to trigger the bound event handlers.

With the PR #33289 by @RyanBerliner this is addressed although in my opinion (and I assume Ryan agrees to some degree) some general fix to the EventHandler module would be more appropriate.

TBD

With this implementation the targets are not rewritten. In some situations e.g. when the window loses focus and some nested element is hovered when it receives back focus, the targets differ between mouseenter and mouseover.

Also like asked here #33220 - are there any reasons as to why mouseover/mouseout was used in the first place?

[RyanBerliner]

tl;dr I think that the custom event mapping can be removed altogether. Otherwise if there is a reason it exists, it would be beneficial to add documentation inline as to why.

---

I think it should be understood why these 2 custom events were replaced in the first place. I've done some digging and found the origin of this event swapping to be this commit, which is converting the carousel from jquery to vanilla js. I'm assuming here that the reason these events were treated as "custom" were because of jquerys claims about implementing these events themselves. This is an assumption, so there's a good chance I'm wrong, though reading this on jquery's website is the only thing I can find that would explain to me why its treated as "custom" in bs5.

The mouseenter JavaScript event is proprietary to Internet Explorer. Because of the event's general utility, jQuery simulates this event so that it can be used regardless of browser.

Again, I'm assuming that this is what caused the reaction to treat these as custom events. I'd bet that the carousel was the first component with these mouseenter/mouseleave events to be converted to vanilla js, so the event swapping was set up at this point. Then, the fact that the actual behavior was never implemented correctly wasn't caught because in the case of the carousel, it doesn't actually matter! It still works no matter what mouse event is used.

From what I can tell jquery's claim about these events are no longer correct (at least for the browsers that bs5 supports), so I don't think any custom event mapping is necessary, and this PR can simply be removing the mapping altogether.

[alpadev]

@RyanBerliner nice investigation there! And your conclusion about the why seems reasonable. 👍

Initially, my naive me had the same thoughts.😊

But there is a problem though by simply removing the event mapping. With capturing listeners, that we use for our delegated handlers, the mouseenter and mouseleave events behave a bit different - like a mixture of mouseover/mouseout and mouseenter/mouseleave. (Some codepen to check for yourself.)

So they need some sort of special treatment I guess. I can try to optimize it a bit, so to use the native events and only fix the behaviour in the delegated version.

WDYT?

[RyanBerliner]

Gotcha - I hadn't considered delegated mouseenter/mouseleave events (in any event phase really, capturing or not). I agree that if you want delegated mouseenter/mouseleave handlers to act in a meaningfully different way than mouseover/mouseout, you can't rely on the events native implementation alone. Disregard my previous notes! And if my understanding of the goal here is correct, I think adding a short message like this near the customEvents = {} assignment would provide a lot of clarity.

If we want delegated mouseenter/mouseleave handlers to act in a meaningful way, we can't rely on their native implementation alone.

EDIT: Said in another way - If we want to leverage event delegation for mouseenter/mouseleave events, we must force the handlers to attach to the capturing phase. Although, as I write this, I'm thinking maybe that's all that's needed? You should be able to use the events at they are and just force useCapture?

EDIT AGAIN: But using event delegation for native mouseenter/mouseleave events on the capturing phase is pretty much identical to use mouseover/mouseout events, but with performance issues since native mouseover seems to be more performant than a mouseenter being added on an element with many children. I've successfully talked myself into exactly what you've done here 😆

[alpadev]

Note to reviewers, this fixes a bug with the one time delegated event handlers. Added a test to the specs too.

[GeoSot]

childish comment: better leave it as string inline, instead of use a var. Is very specific (either way is ok)

[XhmikosR]

It doesn't matter, though, since the minifier should already inline it :)