38932 - Allow Mail/News Account Wizard branding through CCK tool

Status: VERIFIED FIXED

(CCK Graveyard :: CCK-Wizard, defect, P3)

Description

[robinf]

Include a screen in the CCK tool that allows the ISP to brand the Mail Account 
Wizard (username/pwd dialog). We can take the old "Internet Setup - Part 2" 
screen and use it for this purpose.

Comment 1

[robinf]

Accepting, but there's no agreement yet on whether we will allow mail wizard 
customizations through the CCK tool. We should discuss this at our next team 
meeting. I'm planning to document this as a manual customization.

Comment 2

[bmartin]

re-assigning to Bijal

Comment 3

[msanz]

changing QA contact to blee

Comment 4

[bmartin]

removing bmartin from cc list

Comment 5

[msanz]

changing assigned to shrutiv

Comment 6

[Shruti Vasudevan]

In the Netscape commercial release, mail account username/password dialog will
be handled by Account Setup (separately from CCK). However some of the mail/news 
account customizations like server type, incoming server, outgoing server, 
domain name etc. for ISP will be done through the CCK tool.

Comment 7

[Shruti Vasudevan]

Attachment: Patch file for mail/news screens in CCK

Comment 8

[Shruti Vasudevan]

Attachment: Ini file for mail settings screen in CCK wizard

Comment 9

[Shruti Vasudevan]

Attachment: Ini file for News settings in CCK wizard

Comment 10

[Shruti Vasudevan]

Attachment: Patch file to suppport Mail/News Account customization for ISP through CCK

Comment 11

[Shruti Vasudevan]

Attachment: Patch file to suppport Mail/News Account customization for ISP through CCK

Comment 12

[Shruti Vasudevan]

Attachment: Patch file to suppport Mail/News Account customization for ISP through CCK

Comment 13

[Shruti Vasudevan]

Attachment: Patch file to suppport Mail/News Account customization for ISP through CCK

Comment 14

[bobj]

Varada,
   Tao is out, and we'd like one person familiar with CCK to review the
   changes.  Will you please review all of these changes?

Bhuvan,
  Will you please review the specific changes related to creating the rdf files?
     http://bugzilla.mozilla.org/showattachment.cgi?attach_id=36205
     http://bugzilla.mozilla.org/showattachment.cgi?attach_id=36206

Thanks.

Comment 15

[racham]

Here are some comments, suggestions and questions (looking at rdf file related
changes):

* (Patch ID : 36205) Please add comments to the code segments that you have
added in this patch. Also, add some example paths or command structures that are
processing in those routines. Like, what is a rootPath and how does it's value
look like. Similarly, xpiArcDest, xpiList and another vars that need explaination. 

Also, as to what is happening in block "else if (strcmp(cmdname, "addrdfFile")
== 0) {". This will help you when you look back at this code couple of months
from now and will be easy for me to expedite the review.

* So, looking at the other patches, I noticed that you are using "isp.rdf" to
represent mail accounts and "news.rdf" to represent news accounts (probaly you
should have named them as mailaccount.rdf and newsaccount.rdf
respectively..anyway that's not a major concern). This setup is per
configuration right...? Will we ever have a scenario where single configuration
will have multiple mail related rdf files (say one for POP and one for IMAP)..?
If yes, we will have problems with the current naming scheme as it allows ISP to
have only one mail type account. Also, only one news account. If we are going
explain ISPs how to generate these files manually & where to place them, if
needed additionally, then we are at least covered. Is there a plan for that ?

I noticed that some of the values that go into rdf files are all hardcoded in
the cpp file. 
For example, 

# <NC:showServerDetailsOnWizardSummary>false</NC:showServerDetailsOnWizardSummary>
#  <NC:wizardSkipPanels>true</NC:wizardSkipPanels>
# <NC:deleteMailLeftOnServer>false</NC:deleteMailLeftOnServer>
& all sample email related fields.

I hope that you all discussed about these options and decided that they remain
that way for a generic rdf file we create via CCK. But remember to update the
help notes as to how ISP can go modify these files if they really need to change
some of those values.

* So, we are putting the generated rdf files in -> root + "\\Configs\\" + config
+ "\\Temp\\";

Is ib.cpp file picking up these files and putting them in the right place...?
Looks like script.ib has command addrdfFile providing required input info as to
where these rdf files have to go..Is that right ?

And finally, please create some sample customizations and rdf files and test the
account creation process for the correctness.

Please update the patches with comments. 

thanks
bhuvan

Comment 16

[Shruti Vasudevan]

Attachment: Updating patch for ib.cpp, helpmenu.cpp and script.ib.

Comment 17

[Shruti Vasudevan]

Bhuvan,
Resubmitting patch for ib.cpp, helpmenu.cpp and script.ib.
I made some changes based on your suggestions. Added a few comments to specify 
the functionality of some routines and variables. Renamed isp.rdf and news.rdf 
as "mailaccount.rdf" and "newsaccount.rdf" respectively.

Script.ib has command addrdfFile providing required input info as to where these 
rdf files have to go. I had attached the patch for this (id=36208). Anyway, I 
have included the patch file for this again.

All other things like multiple accounts, default values, etc. will probably be 
handled by documentation after we discuss about it in our meeting. I created 
some sample customizations and rdf files and tested the account creation 
process. It worked fine. 

Thanks,
Shruti

Comment 18

[racham]

You said

// mailaccount.rdf file is created 

in one of the comments inside void CreateNewsMenu(void). Change that to
"newsaccount.rdf" before you check in.

Also, for all your future changes remember that it will be easy for you manage
code later, if you can confine the line length of the each statement to 80 chars
and add comments right above the statement you would like to comment on. Go to
VC++'s Tools->Options->Tabs and select "Insert Spaces" for tab/indent, so that
you will have proper indentation when you check in.

Thanks for adding comments. It's better now.

r=bhuvan on rdf generation related patches.

Comment 19

[Shruti Vasudevan]

Thanks. I'll change the mailaccount.rdf comment, I probably overlooked it.

Comment 20

[bobj]

Bhuvan,  Thanks for the detailed review!

Comment 21

[varada]

The patches seem to be good.
r=varada

Comment 22

[Shruti Vasudevan]

Bhuvan and Varada,
Thanks for the review.

Comment 23

[Shruti Vasudevan]

Checked in patch files. Marking this as fixed.

Comment 24

[blee]

Verified in 6/7 bld. 

FYI, The original description seems to suggest somewhat different from what's 
been done to address this.