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

MIME type parsing, parameter canonicalization #46

Closed
domenic opened this issue Nov 27, 2017 · 5 comments
Closed

MIME type parsing, parameter canonicalization #46

domenic opened this issue Nov 27, 2017 · 5 comments

Comments

@domenic
Copy link
Member

@domenic domenic commented Nov 27, 2017

Apologies if this is a duplicate; I've kind of lost track.

https://github.com/w3c/web-platform-tests/pull/8422/files seems to imply there is no canonicalization of parameters performed. I would have imagined that the data model only contained strings, with quotes and slashes used to escape cases during serialization where necessary. Instead it appears that while slash-escapes are canonicalized, quotes are preserved.

If I am understanding correctly, this seems annoying, as then specs and implementations cannot do stuff like if the parameter value is "UTF-8" but instead have to do if the parameter value is "UTF-8" or ""UTF-8"".

@annevk
Copy link
Member

@annevk annevk commented Nov 27, 2017

This was #40. My plan is to have a special "get value" accessor (already defined in the PR). But maybe we should just do what you propose and see if it's compatible. There's not that many double quoted parameter values anyway.

@yutakahirano?

annevk added a commit that referenced this issue Nov 28, 2017
@annevk
Copy link
Member

@annevk annevk commented Nov 28, 2017

#36 now includes this in the draft.

@yutakahirano
Copy link
Member

@yutakahirano yutakahirano commented Nov 29, 2017

I would like to have 'UTF-8' and '"UTF-8"' identically. I think it's ok that parsing and serializing 'text/plain;charset="x"' results in 'text/plain;charset=x'.

@annevk
Copy link
Member

@annevk annevk commented Nov 29, 2017

Great, that's what the proposal does now. Still need to work on test coverage.

@annevk
Copy link
Member

@annevk annevk commented Nov 29, 2017

All PRs in #42 cover this now, including the less obvious cases.

@annevk annevk closed this Nov 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.