Tb feed code does the following:
this.request = Cc["@mozilla.org/xmlextras/xmlhttprequest;1"]
this.request.onprogress = this.onProgress;
this.request.open("GET", this.url, true);
this.request.onload = this.onDownloaded;
this.request.onerror = this.onDownloadError;
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
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..
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.
nsXMLHttpRequest::GetResponseText will fail if the document charset is the empty string.
Requesting tracking, since this would be a web-visible regression.
I wonder what the spec says to do here and whether it needs changing....
Simply ignoring the channel charset seems like the right thing to do here. That's our previous behavior, right?
We used to return "UTF-8" because document charset was initialized "UTF-8" and no body broke that.
I think GetPreferred shouldn't break the parameter on failure.
Created attachment 609627 [details] [diff] [review]
Could you add a comment explaining why temporary variable (preferred) is required?
Comment on attachment 609627 [details] [diff] [review]
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.
also, I file bug 742259 for more test.
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 "?"
Created attachment 613484 [details] [diff] [review]
rebase for aurora
[Approval Request Comment]
Regression caused by (bug #):
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:
Comment on attachment 613484 [details] [diff] [review]
rebase for aurora
Low risk fix to a regression in FF13. Approved for Aurora.