fix: checkbox with preventDefault on click should not trigger SyntheticEvent

[zombieJ]

Summary

React use SyntheticEvent to trigger change event on checkbox & radio by click event. But when called preventDefault on click event, it does not trigger native change event but will trigger on React. It's caused by the checked state will tmp set to opposite but reset when finished by browser:

https://html.spec.whatwg.org/multipage/input.html#the-input-element

Add check to ignore change event when prevent is marked

resolve #9023

How did you test this change?

Update ChangeEventPlugin-test.js for preventDefault case. (Since this is prevent wrong event trigger, seems can do nothing for video record)

related issue:

• event.preventDefault in click handler does not prevent onChange from being called #9023
• [Switch] preventDefault is ignored mui/material-ui#23709
• Tree的title内嵌Checkbox.Group时，checked状态更新有问题 ant-design/ant-design#32790

[sizebot]

Comparing: e0aa5e2...461333c

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.05%	129.68 kB	129.75 kB	+0.05%	41.46 kB	41.48 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	+0.05%	134.66 kB	134.72 kB	+0.07%	42.94 kB	42.97 kB
facebook-www/ReactDOM-prod.classic.js	+0.04%	424.11 kB	424.28 kB	+0.02%	78.18 kB	78.19 kB
facebook-www/ReactDOM-prod.modern.js	+0.04%	412.66 kB	412.83 kB	+0.03%	76.43 kB	76.45 kB
facebook-www/ReactDOMForked-prod.classic.js	+0.04%	424.11 kB	424.28 kB	+0.02%	78.18 kB	78.20 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 461333c

[zombieJ]

hi @rickhanlonii,
I'm not sure if I missing something. Could you help to confirm this PR?

Thanks~

[zombieJ]

Anyone can take a look? Half month passed ...

@gaearon @sebmarkbage

[eps1lon]

This doesn't seem to work in a Chrome Version 96.0.4664.45 (Official Build) (64-bit) when we call event.preventDefault in a React onClick handler: https://codesandbox.io/s/react-18-checkbox-click-preventdefault-change-quirk-4mwl8

[eps1lon]

Here is a test that still fails in this PR which is based on https://codesandbox.io/s/react-18-checkbox-click-preventdefault-change-quirk-4mwl8?file=/src/index.js. Notice that handleNativeChange is not called as expected but handleReactChange is. If we don't call event.preventDefault then both handlers are called as expected.

it('should not fire change for checkbox input when preventDefault is called in onClick', () => {
  const handleReactChange = jest.fn();
  const handleNativeChange = jest.fn();

  const node = ReactDOM.render(
    <input
      type="checkbox"
      defaultChecked
      onClick={event => {
        event.preventDefault();
      }}
      onChange={handleReactChange}
    />,
    container,
  );
  node.addEventListener('change', handleNativeChange);

  node.click();

  expect(handleNativeChange).toHaveBeenCalledTimes(0);
  expect(handleReactChange).toHaveBeenCalledTimes(0);
});

[zombieJ]

@eps1lon,
Thx for your feedback. Let me check out~

[zombieJ]

Hmmm...
dispatchQueue seems push the change event before real onClick call. It makes additional change event trigger.

[zombieJ]

@eps1lon,
Fixed. Please check: https://codesandbox.io/s/react-18-checkbox-click-preventdefault-change-quirk-forked-ptol7?file=/src/index.js

[eps1lon]

@eps1lon, Fixed. Please check: codesandbox.io/s/react-18-checkbox-click-preventdefault-change-quirk-forked-ptol7?file=/src/index.js

Fixed in Chrome 96 👍🏻 For safe measure, could you check various other browsers as well (e.g. Safari and IE 11 are notorous for inconsistent behavior).

[zombieJ]

Update:
Tested in Chrome, Firefox, Safari worked as expect.

Since CodeSandbox do not support IE11, I need additional work for test it.

[zombieJ]

@eps1lon,
I've tested follow browser with latest version. All work as expect : )

• Chrome
• Safari
• IE11 (With polyfill to make React work)
• Firefox

[zombieJ]

@eps1lon, wait your feed back : )

[zombieJ]

@eps1lon , anything more I can do? : )

[zombieJ]

@sebmarkbage could you pls help to take a look?

[zombieJ]

Anyone can help on this?

[zombieJ]

@eps1lon, could you help to take a look?~ Thanks

[zombieJ]

@bvaughn, could you help to merge this? thx

[n0099]

LGTM

[zombieJ]

Hi @bvaughn, is any chance that this PR will be merged or something else that I can help?~

[zombieJ]

Hi @bvaughn, is there any chance to merge this PR?

[zombieJ]

@sebmarkbage, could you help to take a look?

[zombieJ]

@Huxpro Could you help to find someone can handle this? thx

[n0099]

@gnoff plz take a look on this

[zombieJ]

hi @kassens,
Could you help to check this? This PR has been wait for over one year : )

[zombieJ]

@eps1lon, safe to merge? : )

[zombieJ]

Why PR is such hard. I don't want to disturb but none care about this : (

@eps1lon @gnoff @kassens @alunyov @sebmarkbage @gaearon @poteto

[zombieJ]

@eps1lon @gnoff @kassens @alunyov @sebmarkbage @gaearon @poteto

[github-actions]

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.

[github-actions]

Closing this pull request after a prolonged period of inactivity. If this issue is still present in the latest release, please ask for this pull request to be reopened. Thank you!