592235 - Alert on active folder does not identify Mail Account or Folder for "This folder is being processed. Please wait until processing is complete to get messages."

Status: RESOLVED FIXED

(Thunderbird :: Mail Window Front End, defect)

Description

[john]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.8) Gecko/20100722 Firefox/3.6.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.8) Gecko/20100802 Lightning/1.0b2 Thunderbird/3.1.2

On some occasions when TBird GetMail is busy, undertaking various steps can force an Alert popup box showing:
     "ALERT"  "This folder is being processed. Please wait until processing is complete to get messages."

This message is not useful, in that it does not identify the problem mail account.

This occurs in these various examples of times:

-- getting mail when a prior request is delayed or slowed due to network issues.   While this occurs with GetAllMail, or GetMail on a particular account, the confusion is most obvious when Getting ALL mail, which is most common for me [using the default TBird configured GetAllMail, or using an AddOn GetAllMail button]

     ---- this occurs when getting mail following start up, and the automatic pickup of all mail, delayed by a network slow down.   This is most typical for me.   Other times: uploads of large attachments on slow networks.

          ------ I am not sure what happens if, at that time, the mail is a scheduled pickup rather than a manual request [I don't know if an alert box is issued]

-- getting mail while an indexing is occurring ?
-- other times ?

This bug is distinct from the failure to show relevant messaging on the status bar.   See bug 584181 which has been marked as a dupe of bug 66860.

In this instant bug's case, an Alert PopUp Box is generated; in each of the other referenced bug[s], there is no popup.

I distinguish this bug's Alert pop up, centered on the screen, from the floating popup which slides in at the lower right.   My thought is that -that- sliding popup -does- display the account that generated the error.   That behavior, for the sliding popup, should be duplicated for the alert warning popup dialog box, and for the status bar messaging.  All variations of warnings should identify the associated mail account.

Thanks....

Reproducible: Always



Expected Results:  
Any alert box [Popup Box] generated on a GetMail failure should identify the account which generated the alert box.

Comment 1

[john]

In my use, I have multiple POP accounts.  TBird has an AddOn for a GetAllMail icon/button.  My accounts' POP servers are automatically polled at different intervals.

Comment 2

[Ludovic Hirlimann [:Usul]]

This is fixed in upcoming 3.1.3 release , if not please reopen.

Comment 3

[john]

Thank you, Ludovic, for reviewing my report.  In response to your comment 2, above, though close, I don't think it's a duplicate....

There, the problem reported is that a modal alert -appears-.   Mine is that the message appears without identifying the involved account.

My appreciation of the reported fix, there, is that it will remove the alerts[?], and that your comment 2 reflects the fix, there.   I don't notice, there, that their fix includes an alternative message on the status bar, akin to "Request for *account* already in progress".

My report of the error, here, is that the Alert box does not report the account which is being processed.   With a manual GetMail request, following an existing [auto or manual] request, I would want to be notified that some action was already in progress [not a silent return to the existing state w/o notice].   Given that programs freeze and other problems occur with connections and more, silence does not inform.

As such, in any case, when the modal alert -is- provided, i think it wise to identify the account on which the report is founded.   Just like the slide in notice that slides up from the lower right on connection failures.   And -not- like the status bar messages which do not identify problem connections' accounts.

----

For the next planned release, may I also suggest a broader fix which contemplates, generally, that all user messaging should provide the account?  This would include all status bar messages and all alert boxes?   [Maybe an exception for where there is only ONE account on the program....?]

Comment 4

[David :Bienvenu]

I don't think this is fixed in 3.1.3 - this alert is very different from the alerts fixed in the 3.1.3.

Comment 5

[Wayne Mery (:wsmwk)]

alerts = popups as far as I know, so bug 66860 should fixed this. You can test whether it is so, by using trunk build at ftp://ftp.mozilla.org/pub/thunderbird/nightly/latest-comm-central-trunk/

Comment 6

[David :Bienvenu]

this alert is not fixed - the translated text doesn't have %S in it, where the folder name would go, so I'm pretty sure we still don't include the folder name.

Comment 7

[Wayne Mery (:wsmwk)]

making NEW, so it's crystal clear this is confirmed.  Though I'm surprised this isn't a duplicate of something on our books - perhaps they all got duped to bug 66860 :(

Comment 8

[john]

see Bug 288896

... which addresses modality of the alert box, when a non-modal alert/dialog would be more appropriate.   Probably should be addressed concurrently with whatever happens, here...

Comment 9

[john]

Here's another example of an Alert box [not sure if modal or not] that does not display the involved account:

     "error occurred while sending mail. The mail server responded:  Message refused.. Please check the message and try again."

I would suggest account identification on variations of similar popup alert boxes in this case, or when the mail server can't be contacted, or the connection breaks.

Comment 11

[Magnus Melin [:mkmelin]]

The message seems to be pop3MessageFolderBusy
This may end up being fixed in bug 221592.

Comment 12

[Suyash Agarwal (:sshagarwal)]

I think this bug has been fixed with bug 221592. So, shall we close this?

Comment 13

[Magnus Melin [:mkmelin]]

I don't know how to reproduce it on the spot, but code wise it looks like it would still exist
http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Service.cpp#458

Comment 14

[brainstuff John Scheb]

Ludovic,

Please remove all kind of ALERTS with an ok button ... there is no need for this stupid confirmation.  


Replace this by a warning message and warning message list without any OK button. This can be placed on the left of the "not read" message on the right food line of thunderbird. ... warnings: 9999  à click on warnings shows a pop-up list with all warnings and with 2 buttons OK and "delete warning list". 

There is not any need for a "OK button" to confirm thease stupid ALERT- Messages. 

John

Comment 15

[:aceman]

Yes, this seems not fixed.
The nsPop3Service::AlertServerBusy opencodes another variant of alert displaying and does not use the new nsPop3Protocol::Error from bug 221592. Can we make it call that one? If not, just update it in similar way as nsPop3Protocol::Error.

There is another usage of this message at http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Protocol.cpp#2368 but that seems already converted.

But the message "This folder is being processed" does not say which folder it is. But it seems to be only used for pop3. Can we assume that is always Inbox? If not, can we determine which folder it is? We should refine the message in both cases.

Comment 16

[:aceman]

John, reworking the alert infrastructure is not part of this bug. I am sure I have already seen a bug for that. Please join that one.

Comment 17

[john]

Just now, I got the same error as originally reported (talk about timing....).   This time, generated by double clicking on some icon in Win8 and starting up two instances somehow (even though I went through a profile dlg box....).  At least I think that is how it happened.   I have no idea what TBird was thinking about in the first instance.

In any case, I agree with aceman: the error should:

a) identify both the account -and- b) the folder which is at issue.    

Something like:  "...this folder (Inbox-->SubFolder) on account FooBar.Com ... "    While the folder tree might be useful, I can believe that recovering it would be a programming pain, compared to merely the subfolder. 

Do I correctly recall other bugs which have similar incomplete identification (sorting large folders?) (indexing?) (archiving?) (etc.?)

Comment 18

[Suyash Agarwal (:sshagarwal)]

Attachment: Possible fix

Possible fix.

Comment 19

[:aceman]

Comment on attachment 790858 [details] [diff] [review]
Possible fix

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

So if it is not possible to reuse the Error() then this seems fine. If Error it can't be called directly, can we have some neutral (outside of a class) helper function somewhere? Maybe Neil will know.

Anyway, when forcing the dialog to appear even if server is not busy, I get a crash at line 471. Can you look at it?
#5  mozalloc_abort (msg=0xbfe34cd4 "###!!! ABORT: You can't dereference a NULL nsCOMPtr with operator->().: 'mRawPtr != 0', file ../../../mozilla/dist/include/nsCOMPtr.h, line 819")
    at /var/SSD/TB-hg/mozilla/memory/mozalloc/mozalloc_abort.cpp:30
#6  0xb577d48d in Abort (aMsg=<optimized out>) at /var/SSD/TB-hg/mozilla/xpcom/base/nsDebugImpl.cpp:430
#7  NS_DebugBreak (aSeverity=3, aStr=0xb5ac6728 "You can't dereference a NULL nsCOMPtr with operator->().", aExpr=0xb5ac671b "mRawPtr != 0",
    aFile=0xb5c5d8d4 "../../../mozilla/dist/include/nsCOMPtr.h", aLine=819) at /var/SSD/TB-hg/mozilla/xpcom/base/nsDebugImpl.cpp:417
#8  0xb5076148 in nsCOMPtr<nsIMsgFolder>::operator-> (this=0xbfe35148) at ../../../mozilla/dist/include/nsCOMPtr.h:819
#9  0xb518a1b2 in nsPop3Service::AlertServerBusy (this=0xa9f52220, url=0xa4d92c04) at /var/SSD/TB-hg/mailnews/local/src/nsPop3Service.cpp:471
#10 0xb518a4dd in nsPop3Service::RunPopUrl (this=0xa9f52220, aServer=0xaa108c00, aUrlToRun=0xa4d92c04) at /var/SSD/TB-hg/mailnews/local/src/nsPop3Service.cpp:268
#11 0xb518a7d8 in nsPop3Service::GetMail (this=0xa9f52220, downloadNewMail=true, aMsgWindow=0xa65bca60, aUrlListener=0xa4df9ec0, aInbox=0xa65470e4, aPopServer=0xaa108c84, aURL=0xbfe3532c)

::: mail/locales/en-US/chrome/messenger/localMsgs.properties
@@ +98,5 @@
>  
>  # Status - write error occurred
> +#LOCALIZATION NOTE (pop3MessageFolderBusy): Do not translate the word "%S"
> +# below. Place the word %S where the folder name should appear.
> +pop3MessageFolderBusy=%S is being processed. Please wait until processing is complete to get messages.

Don't we need to change the string ID if the value changed?

::: mailnews/local/src/nsPop3Service.cpp
@@ +253,1 @@
>    if (!serverBusy)

I think we should check NS_SUCCEEDED(rv) here (otherwise why is it set?).

@@ +449,5 @@
>    if (NS_FAILED(rv) || !msgWindow)
>      return;
>  
>    rv = msgWindow->GetPromptDialog(getter_AddRefs(dialog));
>    NS_ENSURE_SUCCESS(rv, void(0));

Try NS_ENSURE_SUCCESS_VOID(rv);

@@ +455,5 @@
>    nsString alertString;
> +  nsCOMPtr<nsIMsgIncomingServer> server;
> +  url->GetServer(getter_AddRefs(server));
> +  nsCOMPtr<nsIMsgFolder> folder;
> +  url->GetFolder(getter_AddRefs(folder));

Do we need some error checking here? rv = url-> ...

@@ +459,5 @@
> +  url->GetFolder(getter_AddRefs(folder));
> +  nsCOMPtr<nsIMsgIncomingServer> m_pop3Server = do_QueryInterface(server);
> +  nsString accountName;
> +  rv = m_pop3Server->GetPrettyName(accountName);
> +  NS_ENSURE_SUCCESS(rv, void(0));

S_ENSURE_SUCCESS_VOID(rv);

Comment 20

[Suyash Agarwal (:sshagarwal)]

Can you please give your views on comment 15 and help us decide? 
I am able to access the root folder but that happens to be always the account name itself. Accessing current folder myself gives me segmentation fault.
So, can we assume that the folder generating this error is always Inbox?

Comment 21

[[e-mail]]

(In reply to aceman from comment #15)
> The nsPop3Service::AlertServerBusy opencodes another variant of alert
> displaying and does not use the new nsPop3Protocol::Error from bug 221592.
> Can we make it call that one? If not, just update it in similar way as
> nsPop3Protocol::Error.
I don't think we can, that function is used when the server returns an error code, but we never talk to the server at this point.

> There is another usage of this message at
> http://mxr.mozilla.org/comm-central/source/mailnews/local/src/nsPop3Protocol.cpp#2368
> but that seems already converted.
I don't know whether that would actually work in that case.

> But the message "This folder is being processed" does not say which folder
> it is. But it seems to be only used for pop3. Can we assume that is always
> Inbox? If not, can we determine which folder it is? We should refine the
> message in both cases.
The message is slightly misleading because we're actually checking whether the server is busy, not any specific folder. (From the point of view of the server there are no folders.) I don't know whether POP3 URLs actually specify a folder; I suspect not. (IMAP URLs do have to specify a folder, of course.)

Comment 22

[Suyash Agarwal (:sshagarwal)]

So, because this message is for POP3 and POP3 URLs don't specify a folder (as said above), shall we change the message itself?

This folder is being processed. Please wait until processing is complete to get messages. -> The account is being processed. Please wait until processing is complete to get messages. (with account name in the title bar of the popup box displayed).

Comment 23

[:aceman]

Yes, if the situation is as Neil says, we can change the messages.

Comment 24

[[e-mail]]

(In reply to Suyash Agarwal away for a week from comment #22)
> So, because this message is for POP3 and POP3 URLs don't specify a folder
> (as said above), shall we change the message itself?
Yes, it seems reasonable to use account as it is a more understandable term than server (which is what's actually busy) and the terms are interchangable as we use the server's name as the account name anyway.

Comment 25

[john]

(In reply to [e-mail] from comment #24)
> Yes, it seems reasonable to use account as it is a more understandable term
> than server (which is what's actually busy) and the terms are interchangable
> as we use the server's name as the account name anyway.

I recall a bug I filed a while back, where the fix replaced the server name with the account name.   I think that while we default to the server name, on an account creation, the focus here should be on the user's account.   The server name is often repeated for POP servers on domains with multiple addresses, and normal users might amend the account name to something shorter, or sensible.

Comment 26

[Suyash Agarwal (:sshagarwal)]

Attachment: Possible fix v2

Okay, so here is the possible fix, with the decided message being displayed in the popup with the account name in the title bar.

Do we need a ui-r too?

Comment 27

[[e-mail]]

Comment on attachment 792119 [details] [diff] [review]
Possible fix v2

>+  NS_SUCCEEDED(rv);
???

>-  nsString alertString;
>+  nsString alertString, accountName;
Just declare the two strings separately, they're unrelated.

>+  nsCOMPtr<nsIMsgIncomingServer> m_pop3Server = do_QueryInterface(server);
Don't need to do this, server is already an nsIMsgIncomingServer.

Comment 28

[Suyash Agarwal (:sshagarwal)]

(In reply to [e-mail] from comment #27)
> Comment on attachment 792119 [details] [diff] [review]
> Possible fix v2
> 
> >+  NS_SUCCEEDED(rv);
> ???

In the function RunPopUrl(), this rv = aServer->GetServerBusy(&serverBusy); was not being checked, so I put NS_SUCCEEDED(rv);. Should I remove it or rather replace it with NS_ENSURE_SUCCESS(rv, rv); ?

Comment 29

[Suyash Agarwal (:sshagarwal)]

Attachment: Possible fix v2 with nits

I put that NS_ENSURE_SUCCESS(rv, rv) in because I was using that method to trigger false Alert. And there I found that rv set at 
rv = aServer->GetServerBusy(&serverBusy); wasn't being checked.

If you think this isn't needed, or since this bug doesn't revolve around this so I can't make this change, please let me know, I'll remove this.


Thanks.

Comment 30

[:aceman]

NS_ENSURE_SUCCESS_VOID(rv) ?

Comment 31

[:aceman]

Anyway, it seems Neil is looking at an old version of the patch. The newest one does not contain the bogus NS_SUCCEEDED(rv) line.

Comment 32

[:aceman]

Or the patches are flowing in too quickly and I am looking at a fixed patch already :)

Comment 33

[[e-mail]]

Comment on attachment 792132 [details] [diff] [review]
Possible fix v2 with nits

Code review only, I don't do POP3 accounts.

>   nsString alertString;
>+  nsString accountName;
>+  nsCOMPtr<nsIMsgIncomingServer> server;
>+  rv = url->GetServer(getter_AddRefs(server));
>+  NS_ENSURE_SUCCESS_VOID(rv);
>+  rv = server->GetPrettyName(accountName);
>+  NS_ENSURE_SUCCESS_VOID(rv);
>+
One final nit: move this addition above alertString so it looks like this:

nsString accountName;
...
rv = server->GetPrettyName(accountName);
NS_ENSURE_SUCCESS_VOID(rv);

nsString alertString;
bundle->GetStringFromName(
  NS_LITERAL_STRING("pop3ServerBusy").get(),
  getter_Copies(alertString));

Comment 34

[:aceman]

Comment on attachment 792132 [details] [diff] [review]
Possible fix v2 with nits

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

::: mailnews/local/src/nsPop3Service.cpp
@@ +254,1 @@
>  

I would remove the blank line here.

@@ +439,5 @@
>    nsresult rv;
>    nsCOMPtr<nsIStringBundleService> bundleService =
>      mozilla::services::GetStringBundleService();
>    if (!bundleService)
>      return void(0);

Just "return" ?

@@ +467,3 @@
>      getter_Copies(alertString));
>    if (!alertString.IsEmpty())
> +    dialog->Alert(accountName.get(), alertString.get());

Could you use the nice dialog title used in Error() implemented in bug 221592 ?
+    nsresult rv = server->GetPrettyName(accountName);
+    const PRUnichar *titleParams[] = { accountName.get() };
+    nsString dialogTitle;
+    mLocalBundle->FormatStringFromName(
+      NS_LITERAL_STRING("pop3ErrorDialogTitle").get(),
+      titleParams, 1, getter_Copies(dialogTitle));
+                dialog->Alert(dialogTitle.get(), alertString.get());

::: suite/locales/en-US/chrome/mailnews/localMsgs.properties
@@ +71,5 @@
>  # Status - write error occurred
>  pop3MessageWriteError=Unable to write the email to the mailbox. Make sure the file system allows you write privileges, and you have enough disk space to copy the mailbox.
>  
>  # Status - write error occurred
> +pop3ServerBusy=This folder is being processed. Please wait until processing is complete to get messages.

Update this SM string to be the same as in TB :)

Comment 35

[Suyash Agarwal (:sshagarwal)]

(In reply to :aceman from comment #34)
> Comment on attachment 792132 [details] [diff] [review]
> Possible fix v2 with nits
> 
> Review of attachment 792132 [details] [diff] [review]:
> -----------------------------------------------------------------
> @@ +467,3 @@
> >      getter_Copies(alertString));
> >    if (!alertString.IsEmpty())
> > +    dialog->Alert(accountName.get(), alertString.get());
> 
> Could you use the nice dialog title used in Error() implemented in bug
> 221592 ?
> +    nsresult rv = server->GetPrettyName(accountName);
> +    const PRUnichar *titleParams[] = { accountName.get() };
> +    nsString dialogTitle;
> +    mLocalBundle->FormatStringFromName(
> +      NS_LITERAL_STRING("pop3ErrorDialogTitle").get(),
> +      titleParams, 1, getter_Copies(dialogTitle));
> +                dialog->Alert(dialogTitle.get(), alertString.get());
> 

That displays "Error with <accountname>" & I think this is just an Alert so I haven't used it. But if you think that can be used here, please let me know.

Comment 36

[:aceman]

So in the current form you display an alert like this:

+---<account name>----+
|
| The account is being processed .... |
+-------------------------------------+

Depends whether the user will notice the message is talking about the account in the title bar (hopefully no window manager hides menu bars).

So I would like to have the "Error with <accountname>" string in the title bar (to be consistent with the other dialog from bug 221592). But if you think this dialog is of a lower severity then the POP3 errors (which I do not think it is), then propose some better wording.
You could also change the message to be "The account <accountname> is being processed..." . We will need UI help and review here.

Comment 37

[Suyash Agarwal (:sshagarwal)]

Proposal 1:

+----Error with <account name>-----+
|
| The account <account name> is being processed... |
+--------------------------------------------------+


Proposal 2:

+----<account name>-----+
|
| The account is being processed... |
+--------------------------------------------------+


Proposal 3:

+----Error with <account name>-----+
|
| The account is being processed... |
+--------------------------------------------------+


Can you please help me decide which proposal to continue with?

Comment 38

[Blake Winton (:bwinton) (:☕️)]

So, I'm leaning towards Proposal 1, if the text isn't too wordy.
Also, is it actually an error for the account to be in the middle of being processed?  Could we change that to "Warning for <account name>" instead?

(A screenshot of the dialog with the new wording would help me tell if the text is too wordy or not.)

Thanks,
Blake.

Comment 39

[Suyash Agarwal (:sshagarwal)]

Attachment: Screenshot from 2013-08-20 02:43:40.png

Screen shot for Proposal #1.

Is this accepted?

Comment 40

[Suyash Agarwal (:sshagarwal)]

Attachment: Possible fix v3

Possible fix resulting in the Screen shot.
Carrying over review from Neil. Not sure, if I can carry over that now, significant changes made.

Comment 41

[:aceman]

Comment on attachment 792423 [details] [diff] [review]
Possible fix v3

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

I haven't run with this yet, but it looks good now:)

Comment 42

[john]

Watching the flurry of focus and effort recently, I suspect that some errors (the original ones I reported) have sufficient distinctive qualities that:
         "Account ABC is being processed..." 
might not convey full information?

I distinguish between: a getmail request which is delayed, versus a getmail during a background process (such as indexing or resorting).

In the first case, the account is relevant -- for example, where network connection issues slow down access to a server, or a second getmail request is triggered on top of an existing getmail operation. 

However, in the second case, -both- the account and the underlying folder are important (ie, the folder being indexed or sorted or whatever...)

I realize that a POP request, pulling to an InBox, is different from an IMAP case, where any folder might be at issue.

Comment 43

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 792416 [details]
Screenshot from 2013-08-20 02:43:40.png

Yeah, that seems fine to me.

If you'ld like, you can get rid of the word "account" in the title, since we say "account" just beneath that…

Thanks,
Blake.

Comment 44

[Suyash Agarwal (:sshagarwal)]

(In reply to john ruskin from comment #42)
> Watching the flurry of focus and effort recently, I suspect that some errors
> (the original ones I reported) have sufficient distinctive qualities that:
>          "Account ABC is being processed..." 
> might not convey full information?
> 
> I distinguish between: a getmail request which is delayed, versus a getmail
> during a background process (such as indexing or resorting).
> 
> In the first case, the account is relevant -- for example, where network
> connection issues slow down access to a server, or a second getmail request
> is triggered on top of an existing getmail operation. 
> 
> However, in the second case, -both- the account and the underlying folder
> are important (ie, the folder being indexed or sorted or whatever...)
> 
> I realize that a POP request, pulling to an InBox, is different from an IMAP
> case, where any folder might be at issue.

So, can you please make it a bug so that we can work on that?
It will require to suggest some of the errors that you think need the folder, or even just the idea will be enough.

Thanks.

Comment 45

[:aceman]

(In reply to Blake Winton (:bwinton) from comment #43)
> If you'ld like, you can get rid of the word "account" in the title, since we
> say "account" just beneath that…
Yeah, but we use this same string for other messages that may not contain the word "account", so we need to keep it in the title. I don't think it is worth to create a new string just for this single alert.

Comment 46

[Magnus Melin [:mkmelin]]

Comment on attachment 792423 [details] [diff] [review]
Possible fix v3

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

Looks good to me. r=mkmelin

Comment 47

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/8e9531bfd45a

Comment 48

[[e-mail]]

Comment on attachment 792423 [details] [diff] [review]
Possible fix v3

>-        return Error(rv == NS_MSG_FOLDER_BUSY ? "pop3MessageFolderBusy" :
>+        return Error(rv == NS_MSG_FOLDER_BUSY ? "pop3ServerBusy" :
>                                                 "pop3MessageWriteError");
Error() can't handle errors with paramters...

Comment 49

[Suyash Agarwal (:sshagarwal)]

(In reply to [e-mail] from comment #48)
> Comment on attachment 792423 [details] [diff] [review]
> Possible fix v3
> 
> >-        return Error(rv == NS_MSG_FOLDER_BUSY ? "pop3MessageFolderBusy" :
> >+        return Error(rv == NS_MSG_FOLDER_BUSY ? "pop3ServerBusy" :
> >                                                 "pop3MessageWriteError");
> Error() can't handle errors with paramters...

Oops...
Will be fixing this soon after the dinner.

Thanks for telling.

Comment 50

[Suyash Agarwal (:sshagarwal)]

Attachment: Regression Fix

Possible fix for the regression caused.

Comment 51

[[e-mail]]

Comment on attachment 797350 [details] [diff] [review]
Regression Fix

>+          nsCOMPtr<nsIMsgIncomingServer> server;
>+          mailnewsUrl->GetServer(getter_AddRefs(server));
>+          NS_ENSURE_SUCCESS(rv, -1);
Patch looks good but file style seems to be to QI the nsIMsgIncomingServer from m_pop3Server.

Comment 52

[Suyash Agarwal (:sshagarwal)]

Attachment: Regression Fix using queryInterface

Comment 53

[:aceman]

Good catch with the missing argument to Error(), Neil.

Comment 54

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/2288ad515204