If we encounter a document served as application/atom+xml, application/rss+xml, or linked via autodiscovery (and thus purported to be RSS/Atom), we should parse at all costs. Conversely, if we find something we sniffed as a feed, but don't turn up useful information, we should reload as we do when we miss-sniff RDF feeds. This should take care of sniffing bsmedberg's PHP Atom template and the like.

This should work out well. This detects clicks in the location bar and feed:// links, which never showed in the content area before. Both of those clicks are supposed to be to RSS/Atom feeds. It also assumes content served with feed-specific MIME types are supposed to be feeds. This also takes care of bsmedberg's Atom template, because it's shown to be sniffed, and doesn't turn up a useful result, so it's shown as text/plain after an unsuccessful parse (edge case).

Comment on attachment 246244 [details] [diff] [review] set a response header that tells us whether we sniffed the feed >? browser/components/feeds/src/347897.patch >Index: browser/base/content/browser.js >=================================================================== > */ > subscribeToFeed: function(href, event) { > // Just load the feed in the content area to either subscribe or show the > // preview UI > if (!href) > href = event.target.getAttribute("feed"); > urlSecurityCheck(href, gBrowser.currentURI.spec, > Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT_OR_DATA); >+ var feedURI = makeURI(href, document.characterSet); >+ if (feedURI.scheme == "http" || feedURI.scheme == "https") >+ href = "feed:" + href; Change this to |if (/^https?/.test(feedURI.scheme))| so we don't call GetScheme twice. >Index: browser/components/feeds/src/FeedConverter.js >=================================================================== >@@ -101,16 +101,21 @@ FeedConverter.prototype = { > */ > _data: null, > > /** > * This is the object listening to the conversion, which is ultimately the > * docshell for the load. > */ > _listener: null, >+ >+ /** >+ * Records if the feed was sniffed >+ */ >+ _sniffed: false, > > /** > * See nsIStreamConverter.idl > */ > canConvert: function FC_canConvert(sourceType, destinationType) { > // We only support one conversion. > return destinationType == TYPE_ANY && sourceType == TYPE_MAYBE_FEED; > }, >@@ -215,17 +220,22 @@ FeedConverter.prototype = { > } > } > } > > var ios = > Cc["@mozilla.org/network/io-service;1"]. > getService(Ci.nsIIOService); > var chromeChannel; >- if (result.doc) { >+ >+ // show the feed page if it wasn't sniffed and we have a doc, >+ // or we have a document, title, and link or id >+ if ((!this._sniffed && result.doc) || >+ (result.doc && (result.doc.title != null) && >+ (result.doc.link || result.doc.id))) { nit: make this: > if (result.doc && (!this._sniffed || > (result.doc.title && (result.doc.link || result.doc.id)))) { >@@ -256,27 +266,40 @@ FeedConverter.prototype = { > sourceOffset, count); > }, > > /** > * See nsIRequestObserver.idl > */ > onStartRequest: function FC_onStartRequest(request, context) { > var channel = request.QueryInterface(Ci.nsIChannel); >+ var httpChannel = channel.QueryInterface(Ci.nsIHttpChannel); >+ >+ // Check for a header that tells us there was no sniffing >+ if (httpChannel) { QueryInterface would throw if not. >+ try { >+ var noSniff = httpChannel.getResponseHeader("X-Moz-Is-Feed"); >+ if (noSniff != "1") >+ this._sniffed = true; Unlike the feedsniffer change, this takes care of |X-Moz-Is-Feed: 0|. Since this is an internal header either way is fine as long as you're consistent. > var channel = ios.newChannelFromURI(uri, null); >- channel.originalURI = uri; >- return channel; >+ var httpChannel = channel.QueryInterface(Ci.nsIHttpChannel); nit: use channel directly, i.e. var channel = ios.newChannelFromURI(uri, null) .QueryInterface(Ci.nsIHttpChannel); >Index: browser/components/feeds/src/nsFeedSniffer.cpp >=================================================================== >@@ -294,25 +294,41 @@ nsFeedSniffer::GetMIMETypeFromContent(ns > } > > // Check the Content-Type to see if it is set correctly. If it is set to > // something specific that we think is a reliable indication of a feed, don't > // bother sniffing since we assume the site maintainer knows what they're > // doing. > nsCAutoString contentType; > channel->GetContentType(contentType); >- if (contentType.EqualsLiteral(TYPE_RSS) || >- contentType.EqualsLiteral(TYPE_ATOM)) { >- >+ PRBool noSniff; >+ noSniff = contentType.EqualsLiteral(TYPE_RSS) || >+ contentType.EqualsLiteral(TYPE_ATOM); nit: declare and init on the same line here >+ // Check to see if this was a feed request from the location bar or from >+ // the feed: protocol. This is also a reliable indication. >+ if (!noSniff) { >+ nsCAutoString sniffHeader; >+ nsresult foundHeader = >+ channel->GetRequestHeader(NS_LITERAL_CSTRING("X-Moz-Is-Feed"), >+ sniffHeader); >+ noSniff = NS_SUCCEEDED(foundHeader); see above.

Comment on attachment 246344 [details] [diff] [review] fix Mano's comments er, wrong patch... hold on

Comment on attachment 246350 [details] [diff] [review] fix Mano's comments >Index: browser/base/content/browser.js >=================================================================== > */ > subscribeToFeed: function(href, event) { > // Just load the feed in the content area to either subscribe or show the > // preview UI > if (!href) > href = event.target.getAttribute("feed"); > urlSecurityCheck(href, gBrowser.currentURI.spec, > Ci.nsIScriptSecurityManager.DISALLOW_SCRIPT_OR_DATA); >+ var feedURI = makeURI(href, document.characterSet); >+ if (/^https?/.test(feedURI.scheme)) >+ href = "feed:" + href; I would comment on the location bar case here rather than in FeedSniffer. >Index: browser/components/feeds/src/FeedConverter.js >=================================================================== > > var ios = > Cc["@mozilla.org/network/io-service;1"]. > getService(Ci.nsIIOService); > var chromeChannel; >- if (result.doc) { >+ >+ // show the feed page if it wasn't sniffed and we have a doc, >+ // or we have a document, title, and link or id nit: s/doc/document. r=mano.

Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.739; previous revision: 1.738 done Checking in browser/components/feeds/src/FeedConverter.js; /cvsroot/mozilla/browser/components/feeds/src/FeedConverter.js,v <-- FeedConverter.js new revision: 1.23; previous revision: 1.22 done Checking in browser/components/feeds/src/nsFeedSniffer.cpp; /cvsroot/mozilla/browser/components/feeds/src/nsFeedSniffer.cpp,v <-- nsFeedSniffer.cpp new revision: 1.16; previous revision: 1.15 done

Comment on attachment 246359 [details] [diff] [review] patch to check in approved for 1.8 branch, a=dveditz for drivers

Checking in browser/components/feeds/src/FeedConverter.js; /cvsroot/mozilla/browser/components/feeds/src/FeedConverter.js,v <-- FeedConverter.js new revision: 1.1.2.22; previous revision: 1.1.2.21 done Checking in browser/components/feeds/src/nsFeedSniffer.cpp; /cvsroot/mozilla/browser/components/feeds/src/nsFeedSniffer.cpp,v <-- nsFeedSniffer.cpp new revision: 1.1.2.9; previous revision: 1.1.2.8 done Checking in browser/base/content/browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.479.2.210; previous revision: 1.479.2.209 done

*** Bug 362282 has been marked as a duplicate of this bug. ***

QA: http://svn.smedbergs.us/wordpress-atom10/tags/0.4/wp-atom10-comments.php should not display the feed preview.