When running the following case in any Safari release, the portion of the contentType argument ";charset=utf-8" is missing from the content-type header when the response is received: const blob = new Blob([`<style></style>`], { type: 'text/html' }); const normalizedBlob = blob.slice(0, blob.size, `text/html;charset=utf-8`); const url = URL.createObjectURL(normalizedBlob); const xhr = new XMLHttpRequest(); xhr.addEventListener('load', () => { console.log('(xhr-get) content-type', xhr.getResponseHeader('content-type')); }); xhr.open('GET', url); xhr.send(); fetch(url) .then(response => { console.log('(fetch) content-type', response.headers.get('content-type')) }); Expected output: "(xhr-get) content-type", "text/html;charset=utf-8" "(fetch) content-type", "text/html;charset=utf-8" Actual output: "(xhr-get) content-type", "text/html" "(fetch) content-type", "text/html" The expected output appears correct in Chrome (+ Edge) and Firefox I've made the test case available here: https://jsfiddle.net/rwaldron/acnxz7sv/
<rdar://problem/77175697>
Created attachment 427092 [details] Patch
Comment on attachment 427092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427092&action=review > LayoutTests/fast/files/blob-content-type-expected.txt:1 > +ALERT: content-type text/html2;charset=utf-8 Please write a test with actual PASS / FAIL lines. This output really does not make it obvious if the test is failing or passing. If I were to make a change breaking this, I could easily rebaseline this test by mistake and still think it is fine.
Created attachment 427093 [details] Patch
Comment on attachment 427093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427093&action=review > LayoutTests/fast/files/blob-content-type.html:14 > + alert(response.headers.get('content-type') == "" ? "PASS" : "FAIL") This fails in Chrome. response.headers.get('content-type') returns null. Who is correct?
Comment on attachment 427093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427093&action=review >> LayoutTests/fast/files/blob-content-type.html:14 >> + alert(response.headers.get('content-type') == "" ? "PASS" : "FAIL") > > This fails in Chrome. response.headers.get('content-type') returns null. Who is correct? And Firefox gives "text/html" lol :) Seems browsers really don't agree on this.
Comment on attachment 427093 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427093&action=review > LayoutTests/fast/files/blob-content-type-expected.txt:2 > +ALERT: PASS > +ALERT: PASS Would you mind making the test more conventional in having some output that explains what is expected, what bug was fixed etc? There isn't always the time to do a blame to see what a test was added for. Can this use the usual harness to avoid the need for manual testRunner.dumpAsText, and to have the "test completed" line?
Oh, no. I'm glad I added that test. In the simple case where a content type is provided, all browsers agree that that should be used. Where a content type is not provided, since browsers don't agree maybe we should continue our existing behavior and inherit from the sliced-from blob. I think there is always enough time to git blame, but I can indeed make the test nicer.
Created attachment 427103 [details] Patch
Comment on attachment 427103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427103&action=review > LayoutTests/fast/files/blob-content-type-expected.txt:3 > +TEST COMPLETE Well this is wrong!
Comment on attachment 427103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427103&action=review >> LayoutTests/fast/files/blob-content-type-expected.txt:3 >> +TEST COMPLETE > > Well this is wrong! Indeed. > LayoutTests/fast/files/blob-content-type.html:1 > +<script src="../../resources/js-test-pre.js"></script> You can just use js-test.js > LayoutTests/fast/files/blob-content-type.html:3 > +async function runTest() { This test is missing a description("What the test does") call and `jsTestIsAsync = true;` > LayoutTests/fast/files/blob-content-type.html:26 > +<script src="../../resources/js-test-post.js"></script> Not needed.
Comment on attachment 427103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427103&action=review > LayoutTests/fast/files/blob-content-type.html:10 > + shouldBe("response.headers.get('content-type')", "'text/html2;charset=utf-8'") You can use shouldBeEqualToString() for this. > LayoutTests/fast/files/blob-content-type.html:15 > + shouldBe("response.headers.get('content-type')", "'text/original-blob-content-type;'") this > LayoutTests/fast/files/blob-content-type.html:20 > + shouldBe("response.headers.get('content-type')", "'text/original-blob-content-type;'") and this.
Comment on attachment 427103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427103&action=review > LayoutTests/fast/files/blob-content-type.html:22 > + asyncTestPassed(); finishJSTest();
Created attachment 427108 [details] Patch
Comment on attachment 427108 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=427108&action=review > LayoutTests/fast/files/blob-content-type.html:5 > + const blob = new Blob([`<style></style>`], { type: 'text/original-blob-content-type;' }); Why the weird ` quotes? > LayoutTests/fast/files/blob-content-type.html:8 > + const slice = blob.slice(0, blob.size, `text/html2;charset=utf-8`); ditto. > LayoutTests/fast/files/blob-content-type.html:22 > + const slice = blob.slice(0, blob.size, ""); and then here double quotes, even though the rest of the file seems to be using single quotes.
(In reply to Chris Dumez from comment #3) > Comment on attachment 427092 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=427092&action=review > > > LayoutTests/fast/files/blob-content-type-expected.txt:1 > > +ALERT: content-type text/html2;charset=utf-8 > > Please write a test with actual PASS / FAIL lines. This output really does > not make it obvious if the test is failing or passing. If I were to make a > change breaking this, I could easily rebaseline this test by mistake and > still think it is fine. I've updated the jsfiddle, but here is the test code: const blob = new Blob([`<style></style>`], { type: 'text/html' }); const normalizedBlob = blob.slice(0, blob.size, `text/html;charset=utf-8`); const url = URL.createObjectURL(normalizedBlob); const xhr = new XMLHttpRequest(); xhr.addEventListener('load', () => { console.log(xhr.getResponseHeader('content-type') === 'text/html;charset=utf-8' ? 'PASS' : 'FAIL'); }); xhr.open('GET', url); xhr.send(); Chrome and Firefox: PASS Safari: FAIL
(In reply to Rick Waldron from comment #16) > (In reply to Chris Dumez from comment #3) > > Comment on attachment 427092 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=427092&action=review > > > > > LayoutTests/fast/files/blob-content-type-expected.txt:1 > > > +ALERT: content-type text/html2;charset=utf-8 > > > > Please write a test with actual PASS / FAIL lines. This output really does > > not make it obvious if the test is failing or passing. If I were to make a > > change breaking this, I could easily rebaseline this test by mistake and > > still think it is fine. > > I've updated the jsfiddle, but here is the test code: > > > const blob = new Blob([`<style></style>`], { type: 'text/html' }); > const normalizedBlob = blob.slice(0, blob.size, > `text/html;charset=utf-8`); > const url = URL.createObjectURL(normalizedBlob); > const xhr = new XMLHttpRequest(); > xhr.addEventListener('load', () => { > console.log(xhr.getResponseHeader('content-type') === > 'text/html;charset=utf-8' ? 'PASS' : 'FAIL'); > }); > xhr.open('GET', url); > xhr.send(); > > > Chrome and Firefox: PASS > Safari: FAIL Thanks Rick. I was actually talking to Alex who wrote the patch. Our code changes always need to be committed with a corresponding test. Alex did include a test in his latest iteration and the fix should get committed soon.
Created attachment 427160 [details] Patch
Committed r276677 (237094@main): <https://commits.webkit.org/237094@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 427160 [details].