Parsing Content-Type #10525
Parsing Content-Type #10525
Conversation
Very much WIP.
annevk
force-pushed
the
annevk/content-type-parsing
branch
from
b90bf84
to
e1d3019
Compare
Also known as "extract a MIME type" down right. Tests: web-platform-tests/wpt#10525. Helps with #814. Fixes #529. Closes whatwg/mimesniff#30.
Also known as "extract a MIME type" down right. Tests: web-platform-tests/wpt#10525. Helps with #814. Fixes #529. Closes whatwg/mimesniff#30.
|
@MattMenke2 could you review these too? |
|
Happy to review the test cases, but I'm not at all familiar with the python+js environment used by the wpt tests, so at least that aspect should probably be reviewed by someone familiar with the test environment. |
| "encoding": null | ||
| }, | ||
| { | ||
| "input": ["text / javascript"], |
There was a problem hiding this comment.
Suggest splitting this in two, one with the space in each location.
Also, should the cases be checked (instead or additionally) in wpt/mimesniff/mime-types/resources/mime-types.json?
fetch/content-type/script.window.js
Outdated
| document.head.appendChild(script); | ||
| if (testData.executes) { | ||
| script.onload = t.step_func_done(() => { | ||
| assert_equals(self.isThereABetterWay, testData.encoding === "windows-1252" ? "€" : "€"); |
There was a problem hiding this comment.
Is there a reason for checking the encoding here? Also, I have no idea what all the isThereABetterWay stuff is doing.
There was a problem hiding this comment.
It's basically making sure the script got decoded correctly. isThereABetterWay is a global variable that the script sets with the decoded string value. It's called that way because I was hoping someone would come up with a better mechanism than a global variable. I'll rename it to something more appropriate.
|
Thanks a lot, these are great. I'm not doing I fixed your other points (or left comments), and added documentation to avoid renaming |
|
@domenic note that I added some new generic MIME type tests here as well per feedback, you want to run those against your implementation. |
fetch/content-type/README.md
Outdated
|
|
||
| An array of tests. Each test has these fields: | ||
|
|
||
| * `input`: an array of values for the `Content-Type` header. A harness needs to run the test twice if there are multiple values. One time with the values concatenated with `,` followed by a space and one time with multiple `Content-Type` declarations, each on their own line with one of the values, in order. |
There was a problem hiding this comment.
I think think it's better to rename input so the tests are more self-contained. Not going to block on that, though.
|
Updates to mimesniff/mime-types/resources/mime-types.json LGTM. One day maybe we'll have a jsdom implementation of fetch that can fully test the other JSON files. |
|
Fair enough, I renamed |
Also known as "extract a MIME type" done right. Tests: web-platform-tests/wpt#10525. Helps with #814. Fixes #529. Closes whatwg/mimesniff#30.
Includes web-platform-tests/wpt#10525, which has a few new MIME type parsing tests.
See whatwg/mimesniff#30 (comment) and whatwg/mimesniff#30 (comment) for the processing model this is meant to test.