173267 - Add a method to get the NCProfileName from the registry

Status: VERIFIED FIXED

(Core Graveyard :: Profile: BackEnd, defect)

Description

[Srilatha Moturi]

Add a method to nsIProfileInternal.idl to get the NCProfileName from the 
registry. This is needed for bugscape bug 19946
http://bugscape.netscape.com/show_bug.cgi?id=19946

Comment 1

[Srilatha Moturi]

Attachment: patch v1

Comment 2

[Conrad Carlen (not reading bugmail)]

Comment on attachment 102164 [details] [diff] [review]
patch v1

What I'd rather do is to provide a "getRegStrings" to match "setRegStrings"

Saying that NCProfileName is "the" reg string is misleading since there are
quite a few strings stored in a profile registry entry.

Better yet - since most callers may only want 1 string, make both the getter
and the setter take a param to specify which string, like:
 AString getRegString(in AString profileName, in AString stringID);

Same thing for setRegString. Then it's flexible and uses nsAString types.

Comment 3

[Srilatha Moturi]

I have changed the method to getRegStrings. Since i am going to check this into 
the branch, and if we change SetRegStrings we will have to change the calling 
code too, and i am not comfortable changing it now. 
I will file another bug to change these to getRegString and setRegString which 
takes a parameter for the stringId.

Comment 4

[Srilatha Moturi]

Attachment: patch v2

Comment 5

[Conrad Carlen (not reading bugmail)]

Don't make GetRegStrings() return a ';' delimited concatenation of strings. That
would require making ';' an illegal char in any string stored there, plus
putting the burden of parsing on any caller - not good. If you don't want to
take the original suggestion (getting/setting one named string at a time), make
getRegStrings() return the same 4 separate strings as  setRegStrings().

Comment 6

[Srilatha Moturi]

Attachment: patch v3

Comment 7

[Conrad Carlen (not reading bugmail)]

Comment on attachment 102244 [details] [diff] [review]
patch v3

>+
>+    nsresult rv = NS_OK;
>+    ProfileStruct*    profileVal;
>+
>+    rv = gProfileDataAccess->GetValue(aProfileName, &profileVal);
>+    NS_ENSURE_SUCCESS(rv,rv);
>+    NS_ENSURE_TRUE(profileVal, NS_ERROR_FAILURE);

I think just the NS_ENSURE_TRUE is enough. I'd drop the rv test.

>+    
>+    if (!profileVal->NCHavePregInfo.IsEmpty()) {
>+      *aRegString = ToNewUnicode(profileVal->NCHavePregInfo);
>+      if ( !*aRegString)
>+        return NS_ERROR_OUT_OF_MEMORY;
>+    }
>+    else
>+      *aRegString = nsnull;
>+

Nit: the indentation and blank lines get inconsistent at this point. Just make
the next 3 blocks match the spacing of the above block.

>+    if (!profileVal->NCProfileName.IsEmpty()) {
>+      *aRegName= ToNewUnicode(profileVal->NCProfileName);
>+      if ( !*aRegName)
>+        return NS_ERROR_OUT_OF_MEMORY;
>+     }
>+     else
>+       *aRegName = nsnull;
>+     if (!profileVal->NCEmailAddress.IsEmpty()) {

r=ccarlen

Comment 8

[(not reading, please use [e-mail] instead)]

Comment on attachment 102244 [details] [diff] [review]
patch v3

1)

+interface nsISupportsArray;

is that needed?

2)

style nit:

instead of:

+    nsresult rv = NS_OK;
+    ProfileStruct*    profileVal;
+
+    rv = gProfileDataAccess->GetValue(aProfileName, &profileVal);

do:

+    ProfileStruct*    profileVal;
+
+    nsresult rv = gProfileDataAccess->GetValue(aProfileName, &profileVal);

address those nits, and ccarlen's nits, and then sr=sspitzer

Comment 9

[Judson Valeski]

please checkin to the MOZILLA_1_0_BRANCH branch. once there, remove the
"mozilla1.0.1+" keyword and add the "fixed1.0.1" keyword.

Comment 10

[Srilatha Moturi]

Fix checked in

Comment 11

[Srilatha Moturi]

setting the right keyword fixed1.0.2

Comment 12

[K'Trina Medina]

Does anyone have steps to verify this?

Comment 13

[Srilatha Moturi]

kTrina, this a backend change. One way you can verify this.
Create a profile using 7.0 Rtm
Activate a screen name(for ex: test).
run the daily branch build with that profile
The activation dialog should comeup and the screeen name field should be 
pre-filled with the screen name (test in our ex).
I think Grace has already tested this scenario.

Comment 14

[Grace Bush]

I test this daily and it is ok 
will mark verified1.0.2

Comment 15

[Michael Buckland]

Removing bbird metabug blockage as bbird has shipped.

Comment 16

[racham]

Related changes landed on the trunk. 

Marking Fixed.

Comment 17

[Grace Bush]

verified trunk 2003032904