User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:22.214.171.124) Gecko/20101012 Firefox/3.6.11 (.NET CLR 3.5.30729)
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-GB; rv:126.96.36.199) Gecko/20101013 Thunderbird/3.1.5
Subscription to RSS feeds from the Yahoo Blog always stop at "verifying the feed"
Steps to Reproduce:
1. Right click an account and choose Subscribe
2. Add feed URLs from Yahoo Blog (e.g. http://hk.myblog.yahoo.com/bmanikichow/rss)
3. TB keeps verifying the feed without completion
TB keeps verifying the feed without completion
RSS subscribed to the corresponding folders of the account.
Created attachment 511932 [details] [diff] [review]
Illustation of a workaround
I had this problem with the Planet Postgresql feed ( http://planet.postgresql.org/rss20.xml ). Looking in Thunderbirds error console, I found this:
Error: [Exception... "Component returned failure code: 0x804b000a (NS_ERROR_MALFORMED_URI) [nsIStandardURL.init]" nsresult: "0x804b000a (NS_ERROR_MALFORMED_URI)" location: "JS frame :: chrome://messenger-newsblog/content/FeedItem.js :: anonymous :: line 58" data: no]
Source File: chrome://messenger-newsblog/content/FeedItem.js
As a simple fix, I tried wrapping the setter found there in a try/catch. This enabled Thunerbird to get past the Verifying-state and into "Importing feed" (I think it was called), but then it stopped half way through with this in the error console:
Error: s is null
Source File: chrome://messenger-newsblog/content/utils.js
Treating null as the empty string allowed to feed to be imported.
The attached patch illustrates the changes I made to get the feed imported, but I don't known if this is the correct way to solve the problem.
Hum asking on irc in #maildev is your best option (try to find myk and ask him).
The happens to me in http://planet.postgresql.org/rss20.xml
Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:188.8.131.52) Gecko/20110221 Thunderbird/3.1.8
Created attachment 563192 [details]
The workaround I now use is illustrated in this patch (unzip omni.jar, make the change and re-zip omni.jar). Instead of clearing the url, it just appends "#badurl" to the problematic urls to ease debugging.
Created attachment 578721 [details] [diff] [review]
*** Bug 555789 has been marked as a duplicate of this bug. ***
Comment on attachment 578721 [details] [diff] [review]
I had a look to see what these invalid URIs were, and they were tag: URIs. Perhaps we should be using the IO service to parse these URIs instead of hoping that they are standard URLs?
it seems many uris are standard, but certainly not all. my assumption here is, if they are standard then process that way, if not then just use what the publisher gave (or how current feed processing assigns an item url based on what the publisher gave).
i don't think it matters much for the feed system - they are used as messageIds and keys into feeditems.rdf. the critical thing is to handle this (sort of bogus) error and mush on cleanly.
(In reply to alta88 from comment #8)
> i don't think it matters much for the feed system - they are used as
> messageIds and keys into feeditems.rdf.
Then I don't see the point of trying to mash them into a standard URL...
If you are going to fall back on just using the value as is, if it can't be parsed as an url, why attempt to parse it as an url at all? why not just use the value?
At least for the feed, that I have problems with, firefox (specifically the code that shows the code when you browse to the url of the feed) seems to parse it correctly, so it might be helpful to compare the feed handling from the two applications. It turns out that the feed have an error in that the isPermaLink-attribute on the guid-element is set to "true" even though the guid-element does not contain a valid url.
If I interpret the code correctly and am looking in the right place, it seems firefox first look at the link-element of the feed entry, and then only if it is emty, look that the guid-element. Also it compares isPermaLink-attribute case-insensitively with "false". Therefor it is not tripped by the error as the link-element in my case contains a url and so it do not use the guid for the url.
The current thunderbird-code on the other hand, seems to use the value of the guid-element if the isPermaLink-attribute is missing or (case-sensitively) "true" and only check the format if attrbiute is missing, and else use the link-element.
So if the value of the guid-element, with the isPermaLink-attribute set to true, should continue to be preferred over the link-element, perhaps line 145 of feed-parser.js could just be changed from "if (!guidNode.hasAttribute("isPermaLink"))" to "if (isPermaLink)" to have the check be preformed in all cases where the guid-element is used. That just leaves the value unchecked if the value of the link-element is used.
Created attachment 579064 [details] [diff] [review]
(In reply to andershol from comment #10)
a truly noble endeavor would be bug 450543. but it's not clear the toolkit code is issue free, even per its author:
to your specific guid/permalink question - i believe the way Tb handles isPermaLink is correct according to the spec:
the solution of what to do with a guid that must be a valid url but isn't, will be the same - ensure no halt in processing, do a best effort to verify the url, but in the end if the publisher gave you non spec junk what can you do but store it and not assume anything.
so the new patch stores as is, though i lean to the fact that there is a non zero positive in making a standard url if it was meant to be a url.
(In reply to alta88 from comment #12)
> so the new patch stores as is, though i lean to the fact that there is a non
> zero positive in making a standard url if it was meant to be a url.
But I still think running URLs through the nsIOService is better than directly creating a nsStandardURL object.
Created attachment 579349 [details] [diff] [review]
i completely agree, and mistook your prior comment to be 'save as is'.
Comment on attachment 579349 [details] [diff] [review]
>- var uri = uri.QueryInterface(Components.interfaces.nsIURI);
>+ url = ioService.newURI(aVal, null, null)
I don't know why you thought you had to query to nsIURL; nsIURI was used above to get access to .spec but newURI already returns an nsIURI so you can access the spec straight away.
>+ this.mURL = url ? url.spec : aVal;
This could probably be simplified by using separate assignment statements inside the try and catch blocks respectively.
Created attachment 579718 [details] [diff] [review]
Comment on attachment 579718 [details] [diff] [review]
>+ var url;
>+ url = ioService.newURI(aVal, null, null);
>+ this.mURL = url.spec;
(It's probably not worth having a separate variable for this.)
Created attachment 580424 [details] [diff] [review]
Comment on attachment 580424 [details] [diff] [review]
a=Standard8 for aurora. Given where we are for beta, I don't really want to take this this late - we'll pick it up in 10.
Checked into trunk and aurora: