Too many alert dialogs when a news posting fails

VERIFIED FIXED in mozilla0.9.5

Status

MailNews Core
Composition
VERIFIED FIXED
17 years ago
10 years ago

People

(Reporter: Scott MacGregor, Assigned: Jean-Francois Ducarroz)

Tracking

Trunk
mozilla0.9.5
x86
Windows 2000

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: PDT+)

Attachments

(2 attachments)

(Reporter)

Description

17 years ago
This came out of Bug #85825:

You get too many alert dialogs when we fail to send a message via send later.
you get an alert from the news server (I was posting on my bad message), 
an alert from compose saying "Send Message failed. SendMessage Failed." Yes the
text is repeated. Then you get a 3rd dialog saying "Send Message Failed."
(Reporter)

Comment 1

17 years ago
I need to investiage this for .9.4. It has enterprise ramifications.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.4
(Reporter)

Comment 2

17 years ago
the work for this bug will happen during .9.5 but we intend to land it on the
branch.
Keywords: nsbranch+
Target Milestone: mozilla0.9.4 → mozilla0.9.5
(Reporter)

Comment 3

17 years ago
now that JF is back from vacation, load balancing back to him.
Assignee: mscott → ducarroz
Status: ASSIGNED → NEW
(Assignee)

Comment 4

17 years ago
Created attachment 49378 [details] [diff] [review]
Proposed fix, v1
(Assignee)

Updated

17 years ago
Status: NEW → ASSIGNED
Whiteboard: Have fix

Comment 5

17 years ago
Comment on attachment 49378 [details] [diff] [review]
Proposed fix, v1

r=varada
Attachment #49378 - Flags: review+
(Reporter)

Comment 6

17 years ago
Comment on attachment 49378 [details] [diff] [review]
Proposed fix, v1

>Index: resources/locale/en-US/composeMsgs.properties
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/compose/resources/locale/en-US/composeMsgs.properties,v
>retrieving revision 1.45
>diff -w -u -2 -r1.45 composeMsgs.properties
>--- composeMsgs.properties	2001/09/06 03:40:58	1.45
>+++ composeMsgs.properties	2001/09/14 19:25:02
>@@ -62,5 +62,5 @@
> 
> ## @name NS_ERROR_COULD_NOT_LOGIN_TO_SMTP_SERVER
>-12513=An error occurred sending mail: Unable to connect to the SMTP server. The server may be down or may be incorrectly configured. Please verify that your Mail/News account settings are correct and try again.
>+12513=An error occurred while sending mail: Unable to connect to the SMTP server. The server may be down or may be incorrectly configured. Please verify that your Mail/News account settings are correct and try again.
> 
This wording change seems extraneous from the bug. Let's not check that into the branch since we
are after the localization freeze for the branch. 

> ## @name NS_ERROR_SENDING_FROM_COMMAND
>@@ -75,4 +75,7 @@
> ## @name NS_ERROR_SENDING_MESSAGE
> 12517=An error occurred while sending mail. The mail server responded:  %s. Please check the message and try again.
>+
>+## @name NS_ERROR_POST_FAILED
>+12518=An error occurred while posting message. The news server (NNTP) may be down or may be incorrectly configured. Please verify that your Mail/News account settings are correct and try again.
> 
while posting message doesn't sound like good english =). If we really need to add this new string we need to as Robin to run over the gramar of this sentence and
make sure it checks out. 
> ## @name NS_ERROR_QUEUED_DELIVERY_FAILED

>Index: src/nsMsgSendReport.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/mailnews/compose/src/nsMsgSendReport.cpp,v
>retrieving revision 1.1
>diff -w -u -2 -r1.1 nsMsgSendReport.cpp
>--- nsMsgSendReport.cpp	2001/08/16 00:27:29	1.1
>+++ nsMsgSendReport.cpp	2001/09/14 19:25:45
>@@ -343,8 +343,13 @@
>     {
>       nsAutoString temp((const PRUnichar *)dialogMessage);  // Because of bug 74726, we cannot use directly an XPIDLString
>+
>+      //Don't need to repeat ourself!
>+      if (currMessage != temp)

for nsString is != going to do the right thing here? Or do you really want to use .Equals( )
Is != just going to compare the addresses of temp and currMessage or does nsString over ride it and call .Equals?

Comment 7

17 years ago
Get the SR= ASAP.
(Assignee)

Comment 8

17 years ago
Ok, I'll not check in the UI changes, I can live without it. Also, in the
benefit of the doubt, I will use EqualsIgnoreCase to compare strings instead of !=.
(Assignee)

Comment 9

17 years ago
Created attachment 49849 [details] [diff] [review]
Proposed fix, V2
(Assignee)

Comment 10

17 years ago
Comment on attachment 49849 [details] [diff] [review]
Proposed fix, V2

This patch take care of mscott comments.
Attachment #49849 - Flags: review+
(Reporter)

Comment 11

17 years ago
Comment on attachment 49849 [details] [diff] [review]
Proposed fix, V2

sr=mscott
Attachment #49849 - Flags: superreview+
(Assignee)

Comment 12

17 years ago
Fixed in the trunk
Whiteboard: Have fix → Fixed in the trunk
(Reporter)

Updated

17 years ago
Blocks: 99508

Comment 13

17 years ago
check it in - PDT+
Whiteboard: Fixed in the trunk → PDT+,Fixed in the trunk
(Assignee)

Comment 14

17 years ago
Fixed in the branch too.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Whiteboard: PDT+,Fixed in the trunk → PDT+

Comment 15

17 years ago
I verified this bug with the same test scenario as bug 85825.  I did not see any 
sending of message failed text twice in the dialog and too many dlgs with NNTP 
failures

Branch build:  2001-09-20-11 win98
Still waiting for linux and mac branch builds from today. Will verify when they 
become available. 

Comment 16

17 years ago
verified on mac and linux with following builds:
094-2001092104-linux
094-2001092103-mac
adding keyword vtrunk and needs to be verified on trunk
Keywords: vtrunk

Comment 17

17 years ago
verified on trunk builds:
2001-09-25-05 win98, mac 9.1, linux RH 6.2
Status: RESOLVED → VERIFIED

Comment 18

17 years ago
remove keyword
Keywords: vtrunk
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.