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(ie/edge): form.method='delete', raises Invalid argument. #586

Merged
merged 1 commit into from
Jun 19, 2022

Conversation

mfo
Copy link
Contributor

@mfo mfo commented May 13, 2022

IE issue (edge) : FormElement.method attribute only accepts "get|post". To my surprise, it seems to be the expected behaviour from the spec: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#form-submission-attributes. TLDR ;

The method and formmethod content attributes are [enumerated attributes](https://html.spec.whatwg.org/multipage/common-microsyntaxes.html#enumerated-attribute) with the following keywords and states:
    The keyword get, mapping to the state GET, indicating the HTTP GET method.
    The keyword post, mapping to the state POST, indicating the HTTP POST method.
    The keyword dialog, mapping to the state dialog, indicating that submitting the [form](https://html.spec.whatwg.org/multipage/forms.html#the-form-element) is intended to close the [dialog](https://html.spec.whatwg.org/multipage/interactive-elements.html#the-dialog-element) box in which the form finds itself, if any, and otherwise not submit.

So when the form.method attribute is not get or post IE/edge raises an Invalid argument error. How to reproduce: open ie edge console : document.createElement('form').method="delete" > raise Invalid arg
At first, to avoid this issue, we I went the rails way creating an <input type="hidden" value="{linkMethod}" name="_method" /> appended to the "virtual" form. But tchak is right ; form.setAttribute('method') make it works

@tchak
Copy link

tchak commented May 13, 2022

An alternative (and less framework dependent) implementation would be to replace form.method with form.setAttribute('method'). The actuall request is done by fetch/xhr anyway so it will work. But using setAttribute would prevent some browsers from crashing.

@dhh
Copy link
Member

dhh commented Jun 19, 2022

Please strip ; as we don't use those in our linter. Thanks!

@mfo
Copy link
Contributor Author

mfo commented Jun 19, 2022

Please strip ; as we don't use those in our linter. Thanks!

done. thanks for the review and turbo.

@dhh dhh merged commit 700c921 into hotwired:main Jun 19, 2022
dhh added a commit to feliperaul/turbo that referenced this pull request Jul 16, 2022
* main:
  Allow frames to scroll smoothly into view (hotwired#607)
  Export Type declarations for `turbo:` events (hotwired#452)
  Add .php as a valid isHTML extension (hotwired#629)
  Add original click event to 'turbo:click' details (hotwired#611)
  Drive Browser tests with `playwright` (hotwired#609)
  Allow Turbo Streams w/ GET via `data-turbo-stream` (hotwired#612)
  Only update history when Turbo visit is renderable (hotwired#601)
  Support development ChromeDriver version overrides (hotwired#606)
  Turbo stream source (hotwired#415)
  Expose Frame load state via `[complete]` attribute (hotwired#487)
  fix(ie/edge): form.method='delete', raises Invalid argument. (hotwired#586)
  Do not declare global types/constants (hotwired#524)
  Defensively create custom turbo elements (hotwired#483)
  Use `replaceChildren` in StreamActions.update (hotwired#534)
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.

3 participants