Closed Bug 465258 Opened 16 years ago Closed 16 years ago

Connect Feed Discovery with MailNews feed subscribe

Categories

(SeaMonkey :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Callek, Assigned: Callek)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Make it work (obsolete) — Splinter Review
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]
Flags: blocking-seamonkey2.0a2?
Attachment #348504 - Flags: superreview?(neil)
Attachment #348504 - Flags: review?(neil)
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.
Flags: blocking-seamonkey2.0a2? → blocking-seamonkey2.0a2+
Attachment #348504 - Flags: superreview?(neil)
Attachment #348504 - Flags: superreview+
Attachment #348504 - Flags: review?(neil)
Attachment #348504 - Flags: review+
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
Attachment #348504 - Attachment is obsolete: true
Attachment #348888 - Flags: superreview+
Attachment #348888 - Flags: review+
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.