Fixes https://github.com/facebook/react/issues/20966

[jordyvandomselaar]

Summary

Please see this issue: #20966

The issue described that onMouseEnter did work as intended which gave me a pointer for where to look. Props on the codebase! First time I'm looking at it and it does make sense.

Test Plan

I added tests that covered the cases described in #20966 which initially failed, after I added the code needed, the tests passed.

I've also created a codesandbox with the latest React release: https://codesandbox.io/s/dreamy-boyd-ssbrc?file=/src/index.js

This problem doesn't occur when using React on the web & using a mouse to hover/click on a disabled button because React doesn't normally fire the mouseover event this way. It does occur when manually triggering a bubbling mouseover event on a disabled button.

[sizebot]

Comparing: 10cc400...84a5591

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js	+0.02%	122.83 kB	122.85 kB	=	39.52 kB	39.52 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.02%	129.33 kB	129.35 kB	=	41.59 kB	41.59 kB
facebook-www/ReactDOM-prod.classic.js	=	408.70 kB	408.73 kB	=	75.80 kB	75.80 kB
facebook-www/ReactDOM-prod.modern.js	=	396.96 kB	396.98 kB	=	73.86 kB	73.87 kB
facebook-www/ReactDOMForked-prod.classic.js	=	408.71 kB	408.74 kB	=	75.80 kB	75.80 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 84a5591

[gaearon]

Can you please make a sandbox so that we can check the behavior before and after?

[jordyvandomselaar]

Sure thing! I'll do so later today, it's currently 2AM and i have got to sleep 😄

[jordyvandomselaar]

@gaearon I've added the codesandbox and a little more context to the original post.

The codesandbox uses the latest version of React on NPM. I wanted to add another sandbox with my fork but I haven't found a way to make monorepos work with NPM + Github installs.

[gaearon]

So this is test-only, and doesn't fix issues observable by user? Seems a bit unfortunate to add code for this.

[jordyvandomselaar]

@gaearon it happens whenever someone programmatically fires the mouseover event which does include but is not strictly limited to tests.

[gaearon]

Can you explain in more detail what gives you certainty that this change only affects programmatic firing?

I'm asking because the symmetrical change happened in #17675, but it's curious that it wasn't mirrored for *Leave. Later people started complaining about that change and that maybe it was a wrong decision in the first place (#17675 (comment)).

So I don't want to rush this unless we understand:

• What exactly is the behavior today for both enter and leave events
• Is today's behavior actually reasonable
• What is the native browser behavior and should we align with that

[jordyvandomselaar]

I could not find information about this behaviour in the spec so I created a Sandbox to see how it works in Chrome. My event listeners don't run when entering/leaving a disabled button element with my mouse which is also how React currently does it.

I did find this Firefox issue: https://bugzilla.mozilla.org/show_bug.cgi?id=329509 which states that there was actually a bug that Firefox would not dispatch programmatic events on disabled elements, implying that that should work. I've verified this in the aformentioned CodeSandbox, in Chrome, my event listeners do trigger when programmatically firing the mouseenter event.

In this sandbox I've set up an example with React. What is odd to me is that even though the button is not disabled, the event listener added as a onMouseEnter prop does not trigger when manually firing the mouseenter event. It is only when I add the event listener through addEventListener('mouseenter') that the event listener triggers. This makes it seem that manually firing the event doesn't work at all, even though it should.

Would this be a bug in itself, intended behaviour or error on my end? Please advise.

[stale]

This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated.

[ljharb]

Bump.

[ljharb]

um, what?

[gaearon]

Apologies for closing. We've accidentally pushed a master branch to the repo (due to an old fork), then deleted it, and this lead to a bunch of PRs getting auto-closed. GitHub seems confused and unfortunately does not offer a way to reopen this against main. I can't promise when we'll get to reviewing, but if there is still interest in this PR, we'd appreciate a resubmit. Otherwise, we might revisit it the next time we go through the entire list of issues (which we tend to do a few times per year).