97228 - [Regression] AutoConfig- Multiple appends when using append_email_adress pref

Status: VERIFIED FIXED

(Core :: Preferences: Backend, defect)

Description

[lrg]

When using autoadmin.refresh_interval in conjunction with 
autoadmin.append_emailaddr, the email address sent to the server is not 
refreshed, and is appended to, the logfile would look like this:

Launch: [e-mail]
1st Refresh [e-mail]?[e-mail]
2nd Refresh [e-mail]?[e-mail]?[e-mail]

and so on ad infinitum.  
This problem seems to have come in with the patch to bug 87661.

Comment 1

[Mitesh Shah]

I moved ConfigURL to be a member variable instead local so it keeps the old
value and adds an email address/ profile name again to the URL. I need to move
the appending of the email address under the firstTime check so it does only once. 

Comment 2

[Mitesh Shah]

Attachment: suggested fix, also moved cleaning of mBuf together

Comment 3

[Mike Shaver (:shaver emeritus)]

+    // Removing the emailAddress appended at the end of the ConfigURL if present
+    // So the URL won't keep growing with the arguments when the emailAddress 
+    // will be added again. 

I don't understand that comment, or what it's describing about the need to
remove the email address.  Can you try to reword it for my little brain?

Other than that, sr=shaver.

Comment 4

[Mitesh Shah]

Attachment: changed comments

Comment 5

[Brian Nesse (gone)]

I don't think adding that code to Notify() is going to be the best solution. If
Observe() gets called again (say from a profile switch) the reset code won't
happen. Is there any reason to not just put this code at the top of
downloadAutoConfig? This would cover all of the bases.

Comment 6

[Mitesh Shah]

Attachment: Moved the clean up code to beginning of DownLoadAutoCfg()

Comment 7

[Brian Nesse (gone)]

Thanks! r=bnesse.

Comment 8

[Mike Shaver (:shaver emeritus)]

Comment on attachment 48308 [details] [diff] [review]
Moved the clean up code to beginning of DownLoadAutoCfg()

Thanks for the improved comment.  sr=shaver.

Comment 9

[Mitesh Shah]

Code checked into the trunk. Waiting for drivers approval for branch check in.

Checking in modules/libpref/src/nsAutoConfig.cpp;
/cvsroot/mozilla/modules/libpref/src/nsAutoConfig.cpp,v  <--  nsAutoConfig.cpp
new revision: 3.13; previous revision: 3.12
done

Comment 10

[lrg]

Verified fixed in Windows 2000
Verified fixed in Linux
Verified in MacOS

Comment 11

[Mitesh Shah]

Requesting PDT approval for branch check in

Comment 12

[Mitesh Shah]

Reopening so that we can land it on the branch

Comment 13

[Jaime Rodriguez, Jr.]

U need an R= for the latest patch in the patch status, before we can give it a PDT+

Comment 14

[Brian Nesse (gone)]

Comment on attachment 48308 [details] [diff] [review]
Moved the clean up code to beginning of DownLoadAutoCfg()

added r to patch status

Comment 15

[grega]

marking pdt+...lets get it on the branch asap.

Comment 16

[Mitesh Shah]

Fix checked into the branch.

Checking in nsAutoConfig.cpp;
/cvsroot/mozilla/modules/libpref/src/nsAutoConfig.cpp,v  <--  nsAutoConfig.cpp
new revision: 3.12.2.1; previous revision: 3.12
done                        

Comment 17

[Mitesh Shah]

Tinderbox seems fine

Comment 18

[lrg]

Verified in MacOS, Windows and Linux, closing as verified