360702 - Look for ISP RDF Files in extension directories

Status: RESOLVED FIXED

(Thunderbird :: Account Manager, defect)

Description

[Scott MacGregor]

Currently our tool for ISPs to get themselves listed in the account manager require them to copy their ISP file into <exe dir>\defaults\isp. Of course an extension can't write to this location.

We should do what we did for dictionary extensions, and have the account manager walk through the list of extensions looking for RDF files under:

profile\extensions\<ext id>\isp\*.rdf

Comment 1

[Scott MacGregor]

Attachment: the fix

1) For MOZ_XUL_APPs (thunderbird & seamonkey using toolkit), implement a dir service provider which provides a list of extension directories that contain isp sub directories. 

2) The ISP Service provider now looks in <path to exe>/isp for any RDF files and loads those into the service provider data source. It then walks through the list of directories returned from our directory service provider for each extension with an isp directory and loads any .rdf files it find in extensins\<name of extension\isp\.

Comment 2

[Scott MacGregor]

There's one thing I'm worried about, but I think it's ok and the right thing to do. cc'ing Axel for a sanity check. 

Currently, our ISP RDF files (like rss.rdf, movemail.rdf, dotmac.rdf) get exported to <path to exe>\defaults\isp\AB-CD\ where AB-CD would be en-US, etc. When running Thunderbird with a particular locale, we only pay attention to the RDF files in the defaults directory for that locale.

With this change, these ISP files are treated more like the changes made for dictionaries...looking for ISP RDF files in:
<path to exe>\isp\rss.rdf.

If you are running Thunderbird with multiple locales, you would see all of the ISP files in \isp\ that your version of Thunderbird shipped with and not just the ones exported for a specific locale.

Comment 3

[Axel Hecht [:Pike]]

Why not check for ab-CD and the top-level dir?

Not sure why we have them in the bin dir twice right now.

Does the code honor enabled vs. disabled extensions?

Comment 4

[Scott MacGregor]

(In reply to comment #3)
> Why not check for ab-CD and the top-level dir?

Do you mean look in both:

<thunderbird.exe path>\isp\*.rdf
<thunderbird.exe path>\isp\ab-CD\*.rdf

and:

<profile extensions dir>\extension id\isp\*.rdf
<profile extensions dir>\{extension id}\ab-CD\isp\*.rdf


> 
> Not sure why we have them in the bin dir twice right now.

I don't understand why it's pushed twice either. 

Comment 5

[Axel Hecht [:Pike]]

(In reply to comment #4)
> (In reply to comment #3)
> > Why not check for ab-CD and the top-level dir?
> 
> Do you mean look in both:
> 
> <thunderbird.exe path>\isp\*.rdf
> <thunderbird.exe path>\isp\ab-CD\*.rdf
> 
> and:
> 
> <profile extensions dir>\extension id\isp\*.rdf
> <profile extensions dir>\{extension id}\ab-CD\isp\*.rdf

Yes

Comment 6

[Scott MacGregor]

(In reply to comment #5)
> (In reply to comment #4)
> > (In reply to comment #3)
> > > Why not check for ab-CD and the top-level dir?
> > 
> > Do you mean look in both:
> > 
> > <thunderbird.exe path>\isp\*.rdf
> > <thunderbird.exe path>\isp\ab-CD\*.rdf
> > 
> > and:
> > 
> > <profile extensions dir>\extension id\isp\*.rdf
> > <profile extensions dir>\{extension id}\ab-CD\isp\*.rdf
> 
> Yes
> 

Axel, do you know what the best way to extract the current locale string? I didn't see it as an attribute on nsIXULAppInfo, but I suspect you'll know.

I'm still trying to figure out if we need l10n versions of these ISP rdf files. I guess it comes down to if we think there would be localizeable strings in the rdf files.

Comment 7

[Axel Hecht [:Pike]]

... or if an ISP wants to offer different connection data for different locales (not sure if that'd be the best way to do it, but still).

http://lxr.mozilla.org/seamonkey/source/toolkit/components/urlformatter/src/nsURLFormatter.js#82 has code on how we get the selected locale for the urlformatter.

Comment 8

[Scott MacGregor]

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

Comment 9

[Scott MacGregor]

Attachment: the fix including looking in isp\ab-cd

Ok, this patch adds support for the current locale as well. So we look in

<thunderbird.exe>\isp\*.rdf
<thunderbird.exe>\isp\ab-cd\*.rdf

and for every enabled extension:
<extension path>\isp\*.rdf
<extension path>\isp\ab-cd\*.rdf

* We now have a directory provider for XUL_APP enabled mail builds (thunderbird + toolkit based seamonkey). Right now the only directory type it supports is isp directory lists, but it could support other directories in the future. 

* The directory provider for our isp directory list returns an enumerator for all of the extension\isp directories (for enabled extensions)

* nsMsgServiceProvider now walks through the fore mentioned directories for moz xul app enabled builds.

*For rss.rdf and dotmac.rdf, we now export those to dist\bin\isp. For non xul app enabled seamonkey builds, I left the export targets alone so they still go to defaults/isp/AB-CD

* installer changes to remove the now obsolete files in defaults\isp

Comment 10

[Scott MacGregor]

cc'ing Kohei. This patch is going to make some of the ISP extension work he is doing for mozilla japan a lot easier.

Comment 11

[David :Bienvenu]

Comment on attachment 247607 [details] [diff] [review]
the fix including looking in isp\ab-cd

couple nits:

+  rv = catMan->AddCategoryEntry(XPCOM_DIRECTORY_PROVIDER_CATEGORY,
+				"mail-directory-provider",
+				NS_MAILDIRPROVIDER_CONTRACTID, PR_TRUE, PR_TRUE, nsnull);
+  return rv;


you can just return the result of AddCategoryEntry here - no need for rv.

same thing with unregister.

Unless, you want to return the rv from do_GetService, instead of NS_ERROR_FAILURE:

+  nsCOMPtr<nsICategoryManager> catMan =
+    do_GetService(NS_CATEGORYMANAGER_CONTRACTID);
+  if (!catMan)
+    return NS_ERROR_FAILURE;
+

in which case, you will need rv :-)

Comment 12

[Scott MacGregor]

Comment on attachment 247607 [details] [diff] [review]
the fix including looking in isp\ab-cd

fixed on the trunk, will let it bake there before merging to the branch.

Comment 13

[Scott MacGregor]

Attachment: replaces spaces with tabs in Makefile.in to fix linux bustage

the seamonkey linux build turned red I think because of spaces in Makefile.in. The spaces should be tabs. I just checked this in.

Comment 14

[Scott MacGregor]

Attachment: don't forget to package up rss.rdf into the new location

In today's trunk builds, you can't create a new RSS account on Windows because we  are forgetting to package up rss.rdf in its new location.

Comment 15

[Scott MacGregor]

Attachment: the fix ported to the 1.8 branch

Comment 16

[Scott MacGregor]

I just landed this on the branch.

Comment 17

[Scott MacGregor]

FYI, this hasn't been working in the branch builds because I forgot to land the factory changes in nsMailModule.cpp. I just checked that in tonight.