491034 - guid with isPermaLink="False" (rather than "false") used as item link

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Grant Claypack]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en; rv:1.9.0.8pre) Gecko/2009022317 Camino/2.0b2 (like Firefox/3.0.8pre)
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9.1b5pre) Gecko/20090430 Shredder/3.0b3pre

Website URLs in certain RSS items are being truncated. For example: "http://www.bcjobs.ca/job.cfm?JobID=91767" becomes "91767", so the link can't be clicked through.

Noticed in: 3.0b2    (20090223)
Also seen:  3.0b3pre (20090429)
NOT seen:   2.0.0.21 (20090302)

Happens on Mac OS X 10.4.11 and Win 2000 SP4.

Reproducible: Always

Steps to Reproduce:
1. Go to www.bcjobs.ca
2. At bottom of page, subscribe to 'RSS Latest Jobs' feed.
3. Load RSS items.
4. Select any item, and notice its 'website' URL is truncated.
Actual Results:  
URL is truncated, thus un-followable.

Expected Results:  
URL should be properly formatted. It appears to be a parsing issue, but I haven't narrowed it down yet.

Comment 1

[Joe Sabash [:JoeS1]]

Yes, the message does not render at all for me using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b5pre) Gecko/20090501 Lightning/1.0pre Shredder/3.0b3pre ID:20090501031425

In branch the "web page" displays nicely with all relevant data.
This is not due to the removal of javascript support, as an older JS enabled build
also shows the problem.

Here is the source in branch:
Content-Base: http://www.bcjobs.ca/job.cfm?JobID=91715

And in trunk:
Content-Base: 91715/

This could be the result of security changes on trunk?
Confirming.

Here is the actual feed url for testing:
http://www.bcjobs.ca/rss/index.cfm?nam=Latest%20Job%20Postings

Comment 2

[Phil Ringnalda (:philor)]

Attachment: Fix v.1

There really are no words that can adequately express my hatred for the RSS 2.0 spec.

The feed is wrong, there's no question about that: it uses <guid isPermaLink="False"> when the only two values mentioned in the spec for that attribute are "true" and "false".

However, sifting through the pile of sheep entrails I use to interpret the spec, I think our current interpretation, |isPermaLink = guidNode.getAttribute('isPermaLink') == 'false' ? false : true;|, is not correct.

Of course the spec can't be bothered to talk about error correction, and it's easy to fall into the trap of thinking that "If its value is false, the guid may not be assumed to be a url" and "isPermaLink is optional, its default value is true" add up to "any value other than "false", including no value, means true", but in fact the way it says "has an attribute named "isPermaLink" with a value of true" and "default value is true" much better supports an interpretation of "only an absent attribute or "true" are permalinks."

Comment 3

[Phil Ringnalda (:philor)]

Attachment: Ad hoc testcase

Comment 4

[Phil Ringnalda (:philor)]

Notes from the field:

Firefox and Yahoo both ignore guid completely.

Bloglines agrees with me on everything except that they treat isPermaLink="True" as true.

NetNewsWire and Google Reader agree completely.

Comment 5

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 375441 [details] [diff] [review]
Fix v.1

I think you're exactly right about what the spec says, and this patch brings us in conformance with it (to the extent possible given its vagueness).  I suppose it's possible that ignoring case, as Bloglines does, would make us slightly more interoperable, but given that other major readers don't do so, we're in good company. r=myk

Comment 6

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/b3e346a40491