413891 - RSS discovery doesn't handle the general XML types case well

Status: VERIFIED WONTFIX

(Firefox Graveyard :: RSS Discovery and Preview, defect)

Description

[(no longer active)]

Attachment: Test case

This is a major inconsistency which I found out while testing a page which my friend sent me.  I'm attached a simplified test case.

We should be able to detect both feeds on this page.

The solution is a simple modification to the regular expression.  A patch is under way.

Comment 1

[(no longer active)]

Attachment: Patch (v1)

The patch includes an automated test to make sure this won't happen again.

Asking mano for review on the utilityOverlay.js related change.
Asking sayrer for review on the automated test.

Comment 2

[(no longer active)]

sayrer: Note that the test is based on that of bug 377611, with the <link> elements changed to test for this bug.

Comment 3

[Phil Ringnalda (:philor)]

Because I know sayrer will be asking this question, "no, IE7 doesn't discover either one of those feeds." And even though he won't be asking this question, "no, Opera 9.25 doesn't discover either one of those two feeds." So doing this will encourage people to use autodiscovery links that only work with Safari and Fx3+, but not Fx2 or Fx1.5 or IE7 or any Opera.

The current really slimy regex was designed to do exactly two things: give RDF fanatics who were so proud of having their own mimetype that they just had to use it in autodiscovery a way out, a need which is now met by "alternate feed", and to catch the default link to WordPress' RSS 0.92 feed, so we wouldn't have to bother sucking down their whole RSS 2.0 feed just to get titles and links, a need which no longer exists since the default WP templates now only link to the RSS 2.0 feed.

Before b1, I would have applauded dropping sniffing for title characters entirely; now, I think being even more incompatible with other browsers than we're already being while adding obscure false positives from 平rss仮 or érss doesn't sound like a good idea.

Comment 4

[Asaf Romano (gone)]

Comment on attachment 299007 [details] [diff] [review]
Patch (v1)

Robert is the right reviewer for this change.

Comment 5

[(no longer active)]

sayrer: ping.

Comment 6

[(no longer active)]

sayrer: re-ping :-)

Comment 7

[Phil Ringnalda (:philor)]

IE8b1? Doesn't discover either one.

Comment 8

[(no longer active)]

Hmm, should this be resolved as INVALID, or does my patch still apply here?

Comment 9

[Phil Ringnalda (:philor)]

I'd vote for wontfix.

Comment 10

[(no longer active)]

-> WONTFIX.  If you feel this needs to be fixed for some reason and considering what comment 3 suggests, please re-open.

Comment 11

[Tanner M. Young [:tmyoung]]

Verified Wontfix.