474709 - Subscribing to a feed (triggered by an external application) while no RSS account is present, does nothing

Status: RESOLVED FIXED

(MailNews Core :: Feed Reader, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b3pre) Gecko/20090102 Shredder/3.0b2pre

If you choose to use Thunderbird as your RSS reader from within Firefox and the user subscribes to a new RSS feed, Thunderbird is started but nothing happens. It only works if a RSS account was already created.

Instead of bringing up no UI the user should be asked, if he wants to create a new RSS account.

Steps:
1. Make sure no RSS account is listed in your profile
2. Open Firefox and open http://www.nytimes.com/services/xml/rss/nyt/HomePage.xml
3. Select Thunderbird from the drop down via "Choose Application"
4. Click on "Subscribe Now"

With step 4 Thunderbird will be opened but doesn't show anything. Users will be irritated why it doesn't work.

Comment 1

[Phil Ringnalda (:philor)]

You'd think this would have already been filed, since there's been a comment in the code forever saying something like "// should create an account if we don't have one; just fail for now," but I don't see it if it is filed.

Comment 3

[Frank Wein [:mcsmurf]]

This needs to be fixed at http://hg.mozilla.org/comm-central/annotate/6bcbb9a88f88/mailnews/extensions/newsblog/js/newsblog.js#l166

Comment 4

[Frank Wein [:mcsmurf]]

Attachment: Patch

Patch seems to work fine, still want to test a few testcases though. Unfortunately that way I had to duplicate some logic of the account manager here, but I saw no better way as the account wizard code requires the data for account creation stored in a special format and the code would also need to call a few JS functions (see FinishAccount() in AccountWizard.js).
This patch also fixes the URL that the feed preview page passes to MailNews, now it prefixes the URL with "feed:" so that MailNews actually detects the feed (currently this does not work when MailNews is not open).

Comment 5

[Frank Wein [:mcsmurf]]

Attachment: Patch

Also changes the TB l10n files.
Note that I had to move the detection of MailNews/TB window below the account creation part as the code first needs to create the account before it opens a mail window as otherwise the mail window would open the Account Wizard because it does not see any accounts yet (but the RSS account and the Local Folders already exist shortly after the mail window has opened).

Comment 6

[Frank Wein [:mcsmurf]]

Comment on attachment 396099 [details] [diff] [review]
Patch

>+      var server = acctMgr.createIncomingServer("nobody", "Feeds", "rss");
>+      var bundle = Components.classes["@mozilla.org/intl/stringbundle;1"]
>+                             .getService(Components.interfaces.nsIStringBundleService)
>+                             .createBundle("chrome://messenger-newsblog/locale/newsblog.properties");
>+
[...]
>+      server.prettyName = bundle.GetStringFromName("feeds-accountname");

replace this code with
server.prettyName = GetNewsBlogStringBundle().GetStringFromName("feeds-accountname");

Comment 7

[Phil Ringnalda (:philor)]

(In reply to comment #4)
> This patch also fixes the URL that the feed preview page passes to MailNews,
> now it prefixes the URL with "feed:" so that MailNews actually detects the feed

You probably want to either do that conditionally, or do it in your FeedConverter.js - I didn't try the patch, but I assume that since Fx always passes feed: URIs to external apps, and Tb's commandline handler only treats it as a feed subscription attempt if it's a feed: URI, that you'll then be turning all of ours into feed:feed://example.org and feed:feed:https://example.org.

Comment 8

[Frank Wein [:mcsmurf]]

Comment on attachment 396099 [details] [diff] [review]
Patch

When Thunderbird is already open, it would not take that code path. Anyway, it's probably better to fix this in the suite version of FeedConverter.js. The case you described could for example occur (with this patch) when using SeaMonkey as feed reader within Firefox.

Comment 9

[Frank Wein [:mcsmurf]]

(In reply to comment #8)
> (From update of attachment 396099 [details] [diff] [review])
> When Thunderbird is already open, it would not take that code path. Anyway,
> it's probably better to fix this in the suite version of FeedConverter.js. The
> case you described could for example occur (with this patch) when using
> SeaMonkey as feed reader within Firefox.

Actually, I'm not sure, maybe can occur in TB, too. Will create a new patch for sure.

Comment 10

[Frank Wein [:mcsmurf]]

Attachment: New patch

Ok, this one changes FeedConverter.js in suite/ to simply prefix the URL with feed:. Normally the code would (according to the spec) need to replace http:// with feed:// and for other URLs use feed: as prefix, but the mailnews feed handler can handle both cases just fine so I used the simpler approach for all URLs.

Comment 11

[Mark Banner (:standard8)]

Comment on attachment 396156 [details] [diff] [review]
New patch

>+      var accountManager = Components.classes["@mozilla.org/messenger/account-manager;1"]
>+                           .getService(Components.interfaces.nsIMsgAccountManager);

nit: align the dots, or do something like:

var accountManager =
  Components.classes["@mozilla.org/messenger/account-manager;1"]
            .getService(Components.interfaces.nsIMsgAccountManager);


>+      for (var i=0; i< allServers.Count() && !aFolder; i++)

nit: space either side of = and space before < please.

>+        var currentServer = allServers.GetElementAt(i).QueryInterface(Components.interfaces.nsIMsgIncomingServer);

Please replace this by

var currentServer = allServers.QueryElementAt(i, Components.interfaces.nsIMsgIncomingServer);

its shorter and clearer.

r=Standard8 with those fixed.

Comment 12

[Frank Wein [:mcsmurf]]

Attachment: Patch

Fixed review comments.

Comment 13

[Frank Wein [:mcsmurf]]

Comment on attachment 396810 [details] [diff] [review]
Patch

Got sr+ from Standard8 via IRC.

Comment 14

[Frank Wein [:mcsmurf]]

Pushed http://hg.mozilla.org/comm-central/rev/220ad101680c