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

Move _charset_ handling from "multipart/form-data encoding algorithm"… #3645

Merged
merged 4 commits into from
Apr 27, 2018

Conversation

tkent-google
Copy link
Contributor

@tkent-google tkent-google commented Apr 25, 2018

… and "text/plain encoding algorithm" to "construct the form data set" algorithm

The updated algorithms match to Chrome, Firefox, and Safari.

This fixes #3560.


/acknowledgements.html ( diff )
/form-control-infrastructure.html ( diff )

… and "text/plain encoding algorithm" to "construct the form data set" algorithm

The updated algorithms match to Chrome, Firefox, and Safari.

This fixes whatwg#3560.
tkent-google added a commit to web-platform-tests/wpt that referenced this pull request Apr 25, 2018
@tkent-google
Copy link
Contributor Author

URL spec: whatwg/url#382
WPT: web-platform-tests/wpt#10623

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

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

The text/plain encoding algorithm no longer needs its encoding parameter due to this change. We should remove it.

Thoughts on doing this for <input type=file>? I guess it makes less sense given the three encodings actually encode that differently.

source Outdated
element whose <code data-x="attr-input-type">type</code> attribute is in
the <span data-x="attr-input-type-hidden">Hidden</span> state and
<var>name</var> is "<code data-x="attr-fe-name-charset">_charset_</code>":
</p>
Copy link
Member

Choose a reason for hiding this comment

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

This </p> needs to be on the previous line. Some rewrapping to account for 100 columns might also be needed. I'm happy to help with that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

source Outdated
<ol>
<li><p>Let <var>charset</var> be the <span data-x="encoding name">name
</span> of <var>encoding</var> if <var>encoding</var> is specified, be
"<code data-x="">UTF-8</code>" otherwise.</p></li>
Copy link
Member

Choose a reason for hiding this comment

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

if encoding is given*

and "UTF-8" otherwise*

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

source Outdated
"<code data-x="">UTF-8</code>" otherwise.</p></li>

<li><p>Append an entry to the <var>form data set</var> with <var>name
</var> as the name, <var>charset</var> as the value, and <var>type</var>
Copy link
Member

Choose a reason for hiding this comment

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

No newlines inside inline elements.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@tkent-google
Copy link
Contributor Author

The text/plain encoding algorithm no longer needs its encoding parameter due to this change. We should remove it.

Done.

Thoughts on doing this for ? I guess it makes less sense given the three encodings actually encode that differently.

I think keeping file-specific handling in each of form encodings would make introducing formdata event easier because an event handler may add files.

@annevk
Copy link
Member

annevk commented Apr 27, 2018

That makes a lot of sense.

@annevk
Copy link
Member

annevk commented Apr 27, 2018

@tkent-google I pushed a formatting nits commit and another commit to align your Acknowledgments line with what you have for URL. Hope that's okay, can revert if you want.

@tkent-google
Copy link
Contributor Author

I pushed a formatting nits commit and another commit to align your Acknowledgments line with what you have for URL. Hope that's okay, can revert if you want.

That's ok. Thank you!

annevk pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 27, 2018
@annevk annevk merged commit 8c212e5 into whatwg:master Apr 27, 2018
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request May 2, 2018
Automatic update from web-platform-testsHTML: form's _charset_ handling

HTML change: whatwg/html#3645.

URL change: whatwg/url#382.
--

wpt-commits: c70b18e9e43e3a41f6988a2a1f88d84b01a3dfcc
wpt-pr: 10623
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
Automatic update from web-platform-testsHTML: form's _charset_ handling

HTML change: whatwg/html#3645.

URL change: whatwg/url#382.
--

wpt-commits: c70b18e9e43e3a41f6988a2a1f88d84b01a3dfcc
wpt-pr: 10623

UltraBlame original commit: bfc1a530e1c8cc210e2ad9fc866c3caeb4b2ec4d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 3, 2019
Automatic update from web-platform-testsHTML: form's _charset_ handling

HTML change: whatwg/html#3645.

URL change: whatwg/url#382.
--

wpt-commits: c70b18e9e43e3a41f6988a2a1f88d84b01a3dfcc
wpt-pr: 10623

UltraBlame original commit: bfc1a530e1c8cc210e2ad9fc866c3caeb4b2ec4d
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 3, 2019
Automatic update from web-platform-testsHTML: form's _charset_ handling

HTML change: whatwg/html#3645.

URL change: whatwg/url#382.
--

wpt-commits: c70b18e9e43e3a41f6988a2a1f88d84b01a3dfcc
wpt-pr: 10623

UltraBlame original commit: bfc1a530e1c8cc210e2ad9fc866c3caeb4b2ec4d
ohno418 added a commit to ohno418/servo that referenced this pull request Apr 5, 2023
For "text/plain encoding algorithm", the specification [1] has changed
[2] and "_charset_" is not now handled here. It's supposed to be handled
in "construct the form data set" algorithm, and we've already
implemented that [3]. So we now have extra steps for "text/plain
encoding" algorithm.

Remove no longer necessary steps from text/plain encoding algorithm so
that it meets the specification.

[1]: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#text/plain-encoding-algorithm
[2]: whatwg/html#3645
[3]: servo#25217

Signed-off-by: Yutaro Ohno <yutaro.ono.418@gmail.com>
bors-servo added a commit to servo/servo that referenced this pull request Apr 5, 2023
Remove unnecessary steps from "text/plain encoding algorithm"

For "text/plain encoding algorithm", the specification [1] has changed [2] and "_charset_" is not now handled here. It's supposed to be handled in "construct the form data set" algorithm, and we've already implemented that [3]. So we now have extra steps for "text/plain encoding" algorithm.

Remove no longer necessary steps from text/plain encoding algorithm so that it meets the specification.

[1]: https://html.spec.whatwg.org/multipage/form-control-infrastructure.html#text/plain-encoding-algorithm
[2]: whatwg/html#3645
[3]: #25217

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #25221 (GitHub issue number if applicable)

<!-- Either: -->
- [x] These changes do not require tests because this patch doesn't expect actual behavior, just removing extra steps.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
ohno418 added a commit to ohno418/servo that referenced this pull request Apr 6, 2023
The specifications for the "multipart/form-data encoding algorithm" has
changed [1], and the "_charset_" is not now handled here.

Remove no longer necessary steps from the "multipart/form-data encoding
algorithm".

Similar to the "text/plain encoding algorithm" case fixed by f0818aa
(Remove unnecessary steps from "text/plain encoding algorithm").

[1]: whatwg/html#3645

Signed-off-by: Yutaro Ohno <yutaro.ono.418@gmail.com>
bors-servo added a commit to servo/servo that referenced this pull request Apr 7, 2023
…form-data-encoding-algorithm, r=jdm

Remove unnecessary steps from "multipart/form-data encoding algorithm"

The specifications for the "multipart/form-data encoding algorithm" has changed [1], and the "_charset_" is not now handled here.

Remove no longer necessary steps from the "multipart/form-data encoding algorithm".

Similar to the "text/plain encoding algorithm" case fixed by f0818aa (Remove unnecessary steps from "text/plain encoding algorithm").

[1]: whatwg/html#3645

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #29593 (GitHub issue number if applicable)

<!-- Either: -->
- [x]  These changes do not require tests because this patch doesn't expect actual behavior, just removing extra steps.

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->
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.

<input type=hidden name=_charset_> doesn't match to browser implementations
2 participants