413915 - isValidFeed modifies its first parameter

Status: RESOLVED FIXED

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

Description

[[e-mail]]

...which IMHO is messy. Talking to sayrer on IRC the acceptable solution seems to have been that isValidFeed should return the assumed feed type (which defaults to application/rss+xml in the case of a <link rel="feed"> with no type) if the feed is valid, although only page info uses the specific value.

This also fixes a nit in the page info code in that it won't process <link rel="feed" type="application/atom+xml; charset=UTF-8"> correctly because the existing code doesn't canonicalise the feed type if the rel is already feed.

Comment 1

[[e-mail]]

Attachment: Proposed patch

Unfortunately I don't actually build Firefox, so this patch has been written by code inspection. Feel free to nominate someone who builds Firefox to test it.

Comment 2

[Robert Sayre]

This looks ok, but a unit test is required. If you can't build this, we'll need to find someone else to write it.

Comment 3

[Robert Sayre]

Bug 413891 looks like it has adequate test coverage. Can we dupe and merge the patches?

Comment 4

[Phil Ringnalda (:philor)]

The test in bug 413891 is just a copy of a test already in the tree, with added links to test expanding discovery to even more things that other browsers don't discover, no? Seems like if that test is adequate, then this is already tested.

Comment 5

[Phil Ringnalda (:philor)]

Attachment: tests

Neil's patch does pass the existing autodiscovery tests, but that's rather roundabout testing of it, so here's a testcase with a pancake on its head.

Comment 6

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/mozilla-central/rev/c989ea4a7119
http://hg.mozilla.org/mozilla-central/rev/41790e7f7ae2