Last Comment Bug 521861 - After creating news account through clicking news URL Account Wizard is broken (skips account type selection, bound to invalid news account)
: After creating news account through clicking news URL Account Wizard is broke...
Status: NEW
: relnote
Product: SeaMonkey
Classification: Client Software
Component: MailNews: Account Configuration (show other bugs)
: Trunk
: All All
: -- major with 6 votes (vote)
: ---
Assigned To: Nobody; OK to take it and work on it
:
Mentors:
http://enigmail.mozdev.org/support/ne...
: 521627 636622 680003 1179680 1209211 (view as bug list)
Depends on:
Blocks: 538414
  Show dependency treegraph
 
Reported: 2009-10-12 12:26 PDT by Jens Hatlak (:InvisibleSmiley)
Modified: 2016-05-27 18:21 PDT (History)
23 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
affected


Attachments
Old, new prefs.js and diff, all anonymized (Father-prefs.js is the affected by this bug) (26.54 KB, application/octet-stream)
2010-01-02 12:01 PST, [:rickiees] Ricardo Palomares
no flags Details
Clipboard.jpg (109.84 KB, image/jpeg)
2016-01-14 07:27 PST, Frank-Rainer Grahl
no flags Details
accountwizard.patch (1.10 KB, patch)
2016-01-14 07:32 PST, Frank-Rainer Grahl
no flags Details | Diff | Review
521861-Account_Creation.patch (3.13 KB, patch)
2016-05-17 14:32 PDT, Frank-Rainer Grahl
philip.chee: feedback-
mkmelin+mozilla: feedback+
Details | Diff | Review

Description Jens Hatlak (:InvisibleSmiley) 2009-10-12 12:26:23 PDT
As Susanne Jaeger reported in the German MailNews newsgroup, after following a news:// URL you'll be unable to select the account type (mail, feeds, news) when opening the Account Wizard (either from the File menu or Account Settings). Instead you'll be taken directly to the Identity wizard page and from there to the Server Information wizard page. At least the Server Information page will show the account data matching the newsgroup subscribed to through the news:// URL. You will not be able to finish the wizard without changing the Newsgroup Server name. However, even finishing the wizard won't solve the problem.

Problem source: The account is declared invalid, i.e. the mail.server.serverX.valid pref is set to false (where X is the matching account).

Root cause: unknown, yet to be determined. Accounts created in the way described above are either missing a validation step or the corresponding function is broken.

Workarounds: Set mail.server.serverX.valid to true or delete invalid news account.

Backtrace:
1. AccountWizard.js: checkForInvalidAccounts() calls GetPageData() which sets gPageData
2. aw-accounttype.js: also calls GetPageData() which returns the created news server's data because gPageData is still pointing to that.
Comment 1 :Hb 2009-10-12 15:44:05 PDT
The faulty behaviour in the Account Wizard is similar to bug 521627.
Comment 2 Jens Hatlak (:InvisibleSmiley) 2009-10-12 23:46:08 PDT
*** Bug 521627 has been marked as a duplicate of this bug. ***
Comment 3 [:rickiees] Ricardo Palomares 2010-01-01 13:07:18 PST
Found this problem myself on SeaMonkey 2.0.1. It is somewhat nasty, because there is no clue about the reason why suddenly the account manager stops working right.

Is there something we (users) could do to help fixing this problem (providing a fixed set of steps, activating mail debugging logs, etc.)?

BTW, this bug affects SeaMonkey 2.0 branch, so maybe the version field should be changed?
Comment 4 [:rickiees] Ricardo Palomares 2010-01-01 13:40:33 PST
(In reply to comment #3)
> Found this problem myself on SeaMonkey 2.0.1.


BTW, in my case clicking in a news:// link didn't CREATE a fake news account; instead, MODIFIED one of my newsgroup accounts, renaming it and changing the From: e-mail address associated to it.
Comment 5 :Hb 2010-01-01 14:15:59 PST
Do you have the prefs.js files before and after this step? I.e. could you please post an anonymized diff?
Comment 6 [:rickiees] Ricardo Palomares 2010-01-01 15:01:37 PST
(In reply to comment #5)
> Do you have the prefs.js files before and after this step? I.e. could you
> please post an anonymized diff?


I'll take a look (I have two synchronized machines through an external hard disk, and it could be that one of them still have the previous version).
Comment 7 [:rickiees] Ricardo Palomares 2010-01-02 12:01:57 PST
Created attachment 419791 [details]
Old, new prefs.js and diff, all anonymized (Father-prefs.js is the affected by this bug)
Comment 8 Jens Hatlak (:InvisibleSmiley) 2010-10-25 10:09:08 PDT
Today I took the time to find a regression window for this. Result:
Last known good: 2009-01-09
Broken since: 2009-01-10
Note that "good" here means that "New > Account" fails the first time after trying to subscribe to a newsgroup and canceling in the following dialog, but afterward the account wizard works again, whereas "broken" means that the account wizard stays broken even if you try "New > Account" again.

Probable causing changeset:
http://hg.mozilla.org/comm-central/rev/b78eb7642489 (bug 538414)

My test case was:
1. create a new profile
2. create a Feed account (or any other, but it's fast to do it this way)
3. subscribe to a newsgroup, but cancel the dialog
4. try to create a different account (check the first screen)
5. cancel the dialog and repeat step 4.
Comment 9 Philip Chee 2011-02-24 20:09:49 PST
*** Bug 636622 has been marked as a duplicate of this bug. ***
Comment 10 Jens Hatlak (:InvisibleSmiley) 2011-05-29 11:11:03 PDT
Relnoted, see bug 656719 comment 25.
Comment 11 alan@danburyonline.net 2012-07-27 11:47:26 PDT
This could be a security issue.

I tried to create a new email account, but could not because it was defaulting to a news account, so I gave up and switched to a standalone copy of Thunderbird to set up the mail. However, I now find that when replying to a Newsgroup item, it has picked up the private email address that I was trying to set up. This has exposed a personal email address that I do not use for newsgroups ( to avoid spam etc).

Since this problem has been known about for nearly three years, it should have been fixed by now.
Comment 12 chicagolovr 2012-09-02 14:39:21 PDT
I'm having this problem with adding news servers, but not through an URL, through the settings in SM 2.8.  I can't add but one news server now, because it's really badly broken now.  Sometimes what I add doesn't show up at all in SM.  Other times it will, but it has changed the original account that I had.  It has given me too many invalid responses to list.  Does no one care that they have rendered a core function of SM unusable?
Comment 13 :Hb 2013-10-20 06:00:30 PDT
*** Bug 680003 has been marked as a duplicate of this bug. ***
Comment 14 Trane Francks 2013-11-28 23:34:02 PST
I've just run into this issue in SM 2.22.1 on OS X 10.7.5. Nasty!
Comment 15 Rainer Bielefeld 2015-07-02 09:38:44 PDT
REPRODUCIBLE with  SeaMonkey 2.39a1 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0 from official download area)  Gecko/20100101  Firefox/42.0 (Classic Theme) on German WIN7 64bit

1. Launch SeaMonkey via Profile manager from command line
2. Create blank new profile "Newsgroupstest2" → Launch SeaMonkey →
    confirm default client for all applications → confirm changes requiring 
    WIN admin permissions
3. on <http://www.seamonkey-project.org/dev/get-involved> in left selection pane
   click "Communitiy and support" for <http://www.seamonkey-project.org/community>
4. Click Link Newsgroup: mozilla.support.seamonkey → confirm confirm 'would
   you like to subscribe' with [ok]
   » "Identity" dialog appears instead of account type selection dialog
5. [Cancel] will create account and subscribe "mozilla.support.seamonkey" with 
   invalid identity entries, but postings will be downloaded.

I changed "Version" to ease recognition of appearance

therube asked me to test this one in "Bug 1179680 - Lost ability to add new mail/news accounts". Effects here look similar, but my problem there did not appear in 2009
Comment 16 Rainer Bielefeld 2015-07-03 02:51:18 PDT
@Hb: May be you can explain your Version modification?
Comment 17 Jens Hatlak (:InvisibleSmiley) 2015-07-03 03:25:24 PDT
While I'm not :Hb, here's the reasoning:
* The 2.0 branch is long gone, it has been discontinued years ago. "SeaMonkey 2.0 Branch" is not including all 2.x versions as you might think, but only the 2.0.x ones.
* Version should show the latest affected version. Until this bug gets fixed, all versions are affected, i.e. Trunk is the correct value for the field for the time being. For the same reason, setting any of the status tracking flags is pointless, too, since you might then as well set those for all past and future versions, too.
Comment 18 Rainer Bielefeld 2015-08-16 23:09:15 PDT
*** Bug 1179680 has been marked as a duplicate of this bug. ***
Comment 19 Rainer Bielefeld 2015-09-29 05:03:59 PDT
*** Bug 1209211 has been marked as a duplicate of this bug. ***
Comment 20 Frank-Rainer Grahl 2016-01-14 07:27:50 PST
Created attachment 8707885 [details]
Clipboard.jpg

I stumbled again over this bug when I tried to add a mail account today. 

I think the problem is the leftover valid setting for the news server. Usually you can see the pref for a short time when you create an account manually. It's the string pref in the lower half of the picture. 

When you cancel the subscription in the way Rainer did it in Comment 15 you end up with a mail.server.serverX.valid boolean pref set to false. This seems to confuse the heck out of the account wizard. The simple solution seems just to delete the pref. The not so simple solution would be to make sure that every mail.server.serverX has such a valid pref regardless of it's setting. This seems to work too.
Comment 21 Frank-Rainer Grahl 2016-01-14 07:32:14 PST
Created attachment 8707887 [details] [diff] [review]
accountwizard.patch

Simple patch which just deletes the pref during startup. 

I do not think it's reviewable yet because I am not sure it should be in the migrate js. Was just convenient there because all servers are read here. Any comments where it should be or if this is even the right approach?

Works fine btw. Tested it with a private build.

FRG
Comment 22 Philip Chee 2016-03-27 10:37:02 PDT
(In reply to Frank-Rainer Grahl from comment #20)
> Created attachment 8707885 [details]
> Clipboard.jpg
> 
> I stumbled again over this bug when I tried to add a mail account today. 
> 
> I think the problem is the leftover valid setting for the news server.
> Usually you can see the pref for a short time when you create an account
> manually. It's the string pref in the lower half of the picture. 
> 
> When you cancel the subscription in the way Rainer did it in Comment 15 you
> end up with a mail.server.serverX.valid boolean pref set to false. This
> seems to confuse the heck out of the account wizard. The simple solution
> seems just to delete the pref. The not so simple solution would be to make
> sure that every mail.server.serverX has such a valid pref regardless of it's
> setting. This seems to work too.

Let's ask mkmelin

I think one possible place would be nsMsgAccountManager::LoadAccounts()

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp?rev=1cb65d027ed4&mark=1188-1192#1188
Comment 23 Magnus Melin 2016-04-03 11:30:42 PDT
Yeah that looks like a good spot to me.
Comment 24 Philip Chee 2016-04-03 12:23:09 PDT
(In reply to Magnus Melin from comment #23)
> Yeah that looks like a good spot to me.
Frg: do you want to try moving your patch to?:

http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgAccountManager.cpp?rev=1cb65d027ed4&mark=1209-1210#1207
Comment 25 Frank-Rainer Grahl 2016-04-03 13:23:49 PDT
Ratty,

sure can do. Not this week but with a bug from 2009 it can eventually wait a little longer :)

But not sure if it fixes all the instances. This one might have been a different one:

http://forums.mozillazine.org/viewtopic.php?f=40&t=2995769

Did anyone else who did run into the problem checked if it's caused by the .valid prefs?
Comment 26 Frank-Rainer Grahl 2016-05-17 14:32:54 PDT
Created attachment 8753546 [details] [diff] [review]
521861-Account_Creation.patch
Comment 27 Frank-Rainer Grahl 2016-05-17 14:43:09 PDT
Pressed Enter too fast. I moved the half baked patch to nsMsgAccountManager. 

It works but not sure if I am happy with it so a two questions. 

There seems to be no way to check which mail.server prefs do exists. You can get the accounts list via mail.accountmanager.accounts and the servers via mail.account.accountx.server but if there is a stale server you will not get it. I implemented it by just going over the first 99 server entries which seems to be a bit excessive but sure cleans stuff out. Should this be changed back to go via the accounts?

This is not javascript. Should the pref be checked that it exists before it is to be cleared.

If the valid pref is cleared might this cause problems with invalid server entries?

The root cause why the accounts manager is stuck will not be fixed by this. It's just a workaround so I think the bug should not be closed if the patch is to be accepted.
Comment 28 Magnus Melin 2016-05-26 04:13:18 PDT
Comment on attachment 8753546 [details] [diff] [review]
521861-Account_Creation.patch

Review of attachment 8753546 [details] [diff] [review]:
-----------------------------------------------------------------

::: mailnews/base/src/nsMsgAccountManager.cpp
@@ +367,5 @@
>    }
>  }
>  
> +void
> +nsMsgAccountManager::CleanServerValidState()

nsresult is usually used even if we might not care too much about the return value atm.

@@ +376,5 @@
> +  if (NS_FAILED(rv))
> +    return;
> +
> +  nsCOMPtr<nsIPrefBranch> prefServer;
> +  if (prefService)

you already know you have a prefService since rv succeeded. Or if you're paranoid, check it where you check the rv code

@@ +385,5 @@
> +  }
> +
> +  nsAutoCString serverKey;
> +
> +  // Loop over first 99 mail.server.server* entried and delete .valid entries.

maybe mail.account.lastKey + some?
Comment 29 Philip Chee 2016-05-27 18:21:29 PDT
Comment on attachment 8753546 [details] [diff] [review]
521861-Account_Creation.patch

> +  nsresult rv;
> +  nsCOMPtr<nsIPrefService> prefService(do_GetService(NS_PREFSERVICE_CONTRACTID,
> +                                       &rv));
If you want to wrap at 80cols:

nsCOMPtr<nsIPrefService> prefService =
  do_GetService(NS_PREFSERVICE_CONTRACTID, &rv);

> +  if (NS_FAILED(rv))
> +    return;
> +
> +  nsCOMPtr<nsIPrefBranch> prefServer;
nsCOMPtr<nsIPrefBranch> prefBranch;

> +  if (prefService)
You don't need to check for prefService if you have already done NS_FAILED

> +  nsAutoCString serverKey;
> +
> +  // Loop over first 99 mail.server.server* entried and delete .valid entries.
prefBranch->GetChildList(...)

+    serverKey.AppendInt(lastKey);
+    serverKey.AppendLiteral(".valid");
+    // Just delete the 'valid' pref. Don't check if it exists.
+    prefServer->ClearUserPref(serverKey.get());

StringEndsWith(.. ".valid")

Reference: http://mxr.mozilla.org/comm-central/source/mailnews/base/src/nsMsgTagService.cpp?rev=ff603e79f92a#160

Note You need to log in before you can comment on or make changes to this bug.