This is a quick and dirty hack for the mailnews subscribe being tied into browser. It only links discovery and does *not* affect feed links anywhere else in browser. I had to apply the dirtiest hack to actually prevent the link toolbar from attempting to launch the url as well. The second thing of note is that attempting to subscribe to a feed before MailNews has first been setup is problematic at best, but if you then choose to setup RSS it "just works". I'm not very happy with this, but happy enough for a2. (Feed Preview would clean this up a lot) Of note is taking this makes external application launches via feed urls from our discovery impossible, I think we can live with that [feed preview will fix that as well]

Comment on attachment 348504 [details] [diff] [review] Make it work >+ <menupopup id="link-feed-popup" oncommand="subscribeToFeed('feed:' + event.originalTarget.attributes['href'].value);return false;" onclick="return false;"/> Self review here: onclick removed, and the return false from oncommand as well (I found a way that works, see the hunk above this)

Comment on attachment 348504 [details] [diff] [review] Make it work >- * isValidFeed: checks whether the given data represents a valid feed. > >+ * Feed: checks whether the given data represents a valid feed. Gyah this was obviously a mistake... :/

I think we need a solution for this for the upcoming alpha, and I also agree we can live with some hackery for this as a temporary measure until feed preview fills the gap correctly. Of course, this means we need feed preview for final, preferably already for the beta phase.

Comment on attachment 348504 [details] [diff] [review] Make it work > var destURL = event.target.getAttribute("href"); >+ <menupopup id="link-feed-popup" oncommand="subscribeToFeed('feed:' + event.originalTarget.attributes['href'].value);return false;" onclick="return false;"/> 1. Use event.stopPropagation() to stop the event (return false not needed) 2. Just use 'feed:' + event.target.getAttribute('href') [see destURL above] >+ oncommand="subscribeToFeed('feed:' + event.target.statusText /*, gBrowser.currentURI*/);"/> >+ oncommand="subscribeToFeed('feed:' + event.target.statusText, gBrowser.currentURI);"/> 3. This is inconsistent. Make your mind up ;-) r+sr=me with those fixed. >+ Components.classes["@mozilla.org/newsblog-feed-downloader;1"] >+ .getService(Components.interfaces.nsINewsBlogFeedDownloader) >+ .subscribeToFeed(uri, null, null); newsblog.js notes that this fails when you don't have an RSS account yet :-(

I'm being hit by Bug 464080 yet again :/ so I am unable to test this atm, but I'm pushing my updated patch anyway... $ hg push pushing to ssh://hg.mozilla.org/comm-central/ searching for changes remote: adding changesets remote: adding manifests remote: adding file changes remote: added 1 changesets with 5 changes to 5 files $ hg tip -v changeset: 1145:f78ce96ba4d6 tag: tip user: Justin Wood <Callek@gmail.com> date: Tue Nov 18 20:12:35 2008 -0500 files: suite/browser/linkToolbarOverlay.xul suite/browser/navigator.xul suite/browser/navigatorOverlay.xul suite/browser/pageinfo/feeds.js suite/common/utilityOverlay.js description: Bug 465258, attach feed discovery to MailNews. r+sr=NeilAway