Skip to content
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

Fix for SubmitEvent polyfill breaking requestSubmit() for newer Safari versions #933

Merged
merged 1 commit into from
Jun 18, 2023
Merged

Conversation

mrtnin
Copy link
Contributor

@mrtnin mrtnin commented Jun 14, 2023

This PR addresses an issue where the current polyfill for SubmitEvent is not functioning as expected on the newer versions of Safari. It appears that the original webkit issue the polyfill was meant to address has been resolved, and the polyfill is now introducing undesired behavior.

Specifically, this issue is observed when invoking the requestSubmit() method with a submitter as an argument in Safari 16.5. According to the current behavior, the submitter argument seems to be ignored (ref https://developer.mozilla.org/en-US/docs/Web/API/HTMLFormElement/requestSubmit).

In an attempt to correct this, I've made adjustments to the polyfill logic and added a browser test to confirm the fix. The test fails when executed in the webkit environment with the old polyfill but passes successfully with the revised version in this PR. The command used to run the test is as follows:

yarn test:browser src/tests/functional/form_submission_tests.ts:237 --project=webkit

Please note that for this test to work, I had to add the webkit browser to the playwright.config.ts. I've omitted those changes in this PR to keep it focused on the issue at hand.

@dhh dhh merged commit 96a4f58 into hotwired:main Jun 18, 2023
@dhh
Copy link
Member

dhh commented Jun 18, 2023

Thanks! Please do open a PR with the playwright changes as well so we have tests.

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

Successfully merging this pull request may close these issues.

2 participants