Closed
Bug 737966
Opened 12 years ago
Closed 12 years ago
Evaluating nsIXMLHttpRequest.responseText throws on certain parsing errors
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: alta88, Assigned: m_kato)
References
Details
(Keywords: regression)
Attachments
(2 files)
3.04 KB,
patch
|
sicking
:
review+
|
Details | Diff | Splinter Review |
3.01 KB,
patch
|
akeybl
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
Tb feed code does the following: this.request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"] .createInstance(Ci.nsIXMLHttpRequest); this.request.onprogress = this.onProgress; this.request.open("GET", this.url, true); .. this.request.overrideMimeType("text/xml"); this.request.onload = this.onDownloaded; this.request.onerror = this.onDownloadError; this.request.send(null); using an invalid feed url, like http://dilbert.com/rss/, returns a parsing error doc in .responseXML, but request.response|responseText throw on evaluation. this does not happen in Tb11, but does in Tb12 on. .status is 200. Error: Component returned failure code: 0x80500001 [nsIXMLHttpRequest.responseText] = <unknown> Source file: chrome://messenger-newsblog/content/Feed.js Line: 327 the feed code probably doesn't need to test .responseText and should be asking for responseType=document anyway, but surely the properties should be set to null and not throw. likely this is a common way to test..
Comment 1•12 years ago
|
||
Jonas?
Comment 2•12 years ago
|
||
This looks like a regression from bug 590390. dilbert.com has an invalid charset name "utf-8lias" in response headers. Before bug 590390, TryChannelCharset updates aCharset only when nsCharsetAlias::GetPreferred has succeeded. Note that nsCharsetAlias::GetPreferred will truncate the parameter if it failed. https://hg.mozilla.org/mozilla-central/diff/2893c5e0be5d/content/base/src/nsDocument.cpp nsXMLHttpRequest::GetResponseText will fail if the document charset is the empty string.
Blocks: 590390
Keywords: regression
Comment 3•12 years ago
|
||
Requesting tracking, since this would be a web-visible regression. I wonder what the spec says to do here and whether it needs changing....
tracking-firefox13:
--- → ?
tracking-firefox14:
--- → ?
Simply ignoring the channel charset seems like the right thing to do here. That's our previous behavior, right?
Comment 5•12 years ago
|
||
We used to return "UTF-8" because document charset was initialized "UTF-8" and no body broke that. https://mxr.mozilla.org/mozilla-central/source/content/xml/document/src/nsXMLDocument.cpp#177 I think GetPreferred shouldn't break the parameter on failure.
Updated•12 years ago
|
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → m_kato
Assignee | ||
Comment 6•12 years ago
|
||
Assignee | ||
Updated•12 years ago
|
Attachment #609627 -
Flags: review?(jonas)
Comment 7•12 years ago
|
||
Could you add a comment explaining why temporary variable (preferred) is required?
Comment on attachment 609627 [details] [diff] [review] fix Review of attachment 609627 [details] [diff] [review]: ----------------------------------------------------------------- Though would be great if you could add a mochitest as well to test that things work correctly in a webpage context.
Attachment #609627 -
Flags: review?(jonas) → review+
Assignee | ||
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/da10fe58893d also, I file bug 742259 for more test.
Whiteboard: [inbound]
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/da10fe58893d
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla14
Comment 11•12 years ago
|
||
[Triage Comment] Sounds like a low risk fix - please nominate for Aurora 13 uplift as soon as possible for this regression.
Makoto: Does this patch apply to the aurora branch? If so, can you set the approval‑mozilla‑aurora flag for the patch to "?"
Assignee | ||
Comment 13•12 years ago
|
||
[Approval Request Comment] Regression caused by (bug #): Bug 590390 User impact if declined: This is regression by bug 590390. After landing bug 590390, defalut charset that has no charset in HTTP header isn't UTF-8. It should keeps UTF-8 as default for compatibility of older version of Firefox. Testing completed (on m-c, etc.): Yes. It has xpcshell test. Risk to taking this patch (and alternatives if risky): Low. This is regression. String changes made by this patch: No.
Attachment #613484 -
Flags: approval-mozilla-aurora?
Comment 14•12 years ago
|
||
Comment on attachment 613484 [details] [diff] [review] rebase for aurora [Triage Comment] Low risk fix to a regression in FF13. Approved for Aurora.
Attachment #613484 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 15•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/fbd93a3a58f5
status-firefox13:
--- → fixed
status-firefox14:
--- → fixed
Updated•5 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•