Bug 14288 - REGRESSION: XMLHttpRequest doesn't use a correct content type for file:// URLs
Summary: REGRESSION: XMLHttpRequest doesn't use a correct content type for file:// URLs
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: XML (show other bugs)
Version: 523.x (Safari 3)
Hardware: Mac OS X 10.4
: P1 Normal
Assignee: Alexey Proskuryakov
URL:
Keywords: Regression
Depends on:
Blocks:
 
Reported: 2007-06-21 12:04 PDT by Alexey Proskuryakov
Modified: 2007-06-27 09:38 PDT (History)
1 user (show)

See Also:


Attachments
proposed fix (9.22 KB, patch)
2007-06-24 02:10 PDT, Alexey Proskuryakov
mjs: review+
Details | Formatted Diff | Diff
proposed fix (8.08 KB, patch)
2007-06-26 10:20 PDT, Alexey Proskuryakov
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alexey Proskuryakov 2007-06-21 12:04:17 PDT
XMLHttpRequest::responseMIMEType() gets its result from response headers, but those are not available for non-HTTP requests. It should ask ResourceResponse instead.
Comment 1 Alexey Proskuryakov 2007-06-24 02:10:25 PDT
This is a regression in a way: since shipping Safari/WebKit blindly used an HTML/XML parser for all responses, it didn't care whether the request was an HTTP one.
Comment 2 Alexey Proskuryakov 2007-06-24 02:10:51 PDT
Created attachment 15206 [details]
proposed fix
Comment 3 Mark Rowe (bdash) 2007-06-24 23:31:16 PDT
-PASS -- testing: foo,bar/baz+xml -- responseXML: null
+FAIL (got document -- response type: foo,bar/baz+xml) -- testing: foo,bar/baz+xml -- responseXML: [object Document]

Your change adds this FAIL line, which doesn't look right.  Is that expected?
Comment 4 Alexey Proskuryakov 2007-06-25 03:00:44 PDT
(In reply to comment #3)
> Your change adds this FAIL line, which doesn't look right.  Is that expected?

Yes, as I mentioned in the ChangeLog, this is an edge case resulting from different Content-Type parsing (I suppose this particular failure can be qualified as an NSURLConnection bug, but I haven't investigated it enough to file a bug).
Comment 5 mitz 2007-06-25 03:07:58 PDT
FAIL lines in expected results are seriously confusing. Since WebKit tests are regression tests, it is best, in my opinion, to design the test so that it does not say FAIL (a comment about how current behavior is incorrect can be added).
Comment 6 Alexey Proskuryakov 2007-06-25 04:18:46 PDT
Personally, I do not have a problem with a failing test saying that it fails - especially when it verifies a large set of cases.

Anyway, it's not a new issue with this test, as it already had FAIL lines.
Comment 7 Maciej Stachowiak 2007-06-25 20:17:52 PDT
Comment on attachment 15206 [details]
proposed fix

This patch looks ok to me as is. However, maybe it would be better to parse Content-Type ourselves for http, but trust the network layer for other protocols. In particular, we might want to avoid the network layer's possible content-based sniffing for XHMLHttpRequest.

The patch is fine as is however.
Comment 8 Alexey Proskuryakov 2007-06-25 21:45:01 PDT
(In reply to comment #7)
> In particular, we might want to avoid the network layer's possible
> content-based sniffing for XHMLHttpRequest.

Agreed; I'm going to test this in other browsers.
Comment 9 Alexey Proskuryakov 2007-06-26 10:20:48 PDT
Created attachment 15249 [details]
proposed fix

OK, testing sniffing in other browsers doesn't make all that much sense, since they don't honor HTML metas anyway. But getting HTTP content sniffing out of the equation for XHR is definitely nice!
Comment 10 Geoffrey Garen 2007-06-26 14:33:06 PDT
Comment on attachment 15249 [details]
proposed fix

r=me

I believe this patch addresses all the comments above.
Comment 11 Alexey Proskuryakov 2007-06-27 09:38:03 PDT
Committed revision 23815.