Skip to content

fix(modal-checkout): return array when getting event handlers #2162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

miguelpeixe
Copy link
Member

All Submissions:

Changes proposed in this Pull Request:

When getting the event handlers to be used here:

// Remove default form event handlers.
originalFormHandlers = getEventHandlers( $form[ 0 ], 'submit' ).slice( 0 );
originalFormHandlers.forEach( handler => {
$form.off( 'submit', handler.handler );
} );

The function can return undefined if the element has events but not the event being requested. It's a rare edge case only seen in a race condition, where a click event handler is registered, but no submit event handler has been registered yet.

This crashes the code above with Uncaught TypeError: Cannot read properties of undefined (reading 'slice').

How to test the changes in this Pull Request:

  1. While on the release branch, tweak lines 219-225 of src/modal-checkout/index.js to run behind setTimeout:
setTimeout(() => {
  setEditingDetails( true );
  if ( ! $gift_options.length ) {
    // Perform initial validation so it can skip 1st step if possible.
    validateForm( true, setReady );
  } else {
    setReady();
  }
}, 10);
  1. Start the modal checkout as an anonymous user (register a new reader in the process if your site requires you to)
  2. If the checkout doesn't crash, try again or tweak the timeout delay until you hit it. Mine was 10, but also intermittent.
  3. Checkout this branch, reapply the setTimeout, and confirm that there's no crash when there are event handlers but none in the submit event
  4. Complete the checkout flow and confirm it behaves as expected

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@miguelpeixe miguelpeixe self-assigned this Jul 7, 2025
@miguelpeixe miguelpeixe requested a review from a team as a code owner July 7, 2025 20:04
Copy link
Contributor

@claudiulodro claudiulodro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't able to replicate the race condition failure locally, so not sure if you want a second opinion, but I can confirm that Checkout works fine with this PR and the code change doesn't seem to be doing anything unreasonable. :)

@miguelpeixe miguelpeixe merged commit c9ba6ec into release Jul 8, 2025
10 checks passed
@miguelpeixe miguelpeixe deleted the hotfix/modal-checkout-form-event-handler-error branch July 8, 2025 12:04
matticbot pushed a commit that referenced this pull request Jul 8, 2025
## [4.14.3](v4.14.2...v4.14.3) (2025-07-08)

### Bug Fixes

* **modal-checkout:** return array when getting event handlers ([#2162](#2162)) ([c9ba6ec](c9ba6ec))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 4.14.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants