Connect Feed Discovery with MailNews feed subscribe

RESOLVED FIXED

Status

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Callek, Assigned: Callek)

Tracking

Dependency tree / graph
Bug Flags:
blocking-seamonkey2.0a2 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

11 years ago
Posted 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)
(Assignee)

Comment 1

11 years ago
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)
(Assignee)

Comment 2

11 years ago
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... :/

Comment 3

11 years ago
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+

Updated

11 years ago
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 :-(
(Assignee)

Comment 5

11 years ago
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+
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.