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

Don't duplicate Form Submitter [name] param #184

Merged
merged 1 commit into from Apr 7, 2021

Conversation

seanpdoyle
Copy link
Contributor

When a <form> element's submitter declares a [name] attribute
without a [value] attribute, it is appended to the submission's
FormData instance twice. Due to a quirky difference between Safari and
Firefox + Chrome, the duplication only occurs in Safari, which is a
browser that we are not running our functional test suite against.

In spite of that, this commit adds test coverage to ensure we don't
introduce a similar regression into those browsers.

Testing

Oddly enough, there are several occurrences of doubled-up await await
keywords, which this commit removes.

When a `<form>` element's submitter declares a `[name]` attribute
without a `[value]` attribute, it is appended to the submission's
`FormData` instance twice. Due to a quirky difference between Safari and
Firefox + Chrome, the duplication _only_ occurs in Safari, which is a
browser that we are not running our functional test suite against.

In spite of that, this commit adds test coverage to ensure we don't
introduce a similar regression into those browsers.

Testing
---

Oddly enough, there are several occurrences of doubled-up `await await`
keywords, which this commit removes.
@seanpdoyle
Copy link
Contributor Author

What would it take to run intern against Safari in CI? Browserstack? GitHub Actions running on macOS?

@seanpdoyle seanpdoyle added the bug Something isn't working label Apr 1, 2021
@sstephenson
Copy link
Contributor

What would it take to run intern against Safari in CI? Browserstack? GitHub Actions running on macOS?

I don't recall the specifics, but there's problems with running the test suite under Safari that are not CI- or Intern-related.

@sstephenson sstephenson merged commit 34bc2c9 into hotwired:main Apr 7, 2021
@sstephenson
Copy link
Contributor

Thank you!

@seanpdoyle seanpdoyle deleted the form-submitter-param branch April 7, 2021 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

None yet

2 participants