Closed Bug 136937 Opened 22 years ago Closed 22 years ago

Mail composition window becomes ineditable after send failure

Categories

(MailNews Core :: Security: S/MIME, defect, P2)

Product:
MailNews Core
Component:
Security: S/MIME
Version:
1.0 Branch
Platform:
x86
Windows 2000
Type:
defect

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: carosendahl, Assigned: vparthas)

References

Details

(Keywords: regression, Whiteboard: [adt1], checked into trunk.)

Attachments

(1 file)

Changes to MsgComposeCommands.js
704 bytes, patch
jud
: review+
mscott
: superreview+
jud
: approval+
Details | Diff | Splinter Review 
If there are any problems sending mail messages (bad signature cert, missing
cert, no recipient encryption cert, etc) after the failure message is displayed
to the user, focus is lost in the composition window and returned to the mail
client or the browser.

Refocusing the composition window yields an ineditable message composition
frame.  Further, rather sporadically, any attempts to alter the current security
mode (remove encryption, require encryption & remove signature, etc) causes
mozilla to crash (separate issue - for now the mail client or PSM should fix the
above issue)
Is this on the 1.0 branch or on the trunk?
I can't reproduce using 2002041011 1.0 branch build.
I open a compose window, type fasldkf@asdflksjdf.com in the address,
Then select encryption and try to send. I do get the dialog telling me it can't
be sent. The focus is indeed not set correctly, but if I bring the compose
window back to the front, it's editable, and I can play with the security settings.

I haven't  tested with the other s/mime error conditions.

The focus is an issue, but I'm pretty positive that we just make use of the
sendReport functionality in Mail/News, and we only set the text of the error, so
I believe they would have to take a look at it.  cc ducarroz on this.
Charles, you may want to have a javascript console opened when you reproduce
this.  There may be a javascript error (in moz, a javascript error will stop
execution of the current script, which often results in undefined behavior).

If you don't see any errors, you may want to set the following in your prefs.js
to catch more js issues:
user_pref("javascript.options.strict", true);
This is the trunk build 2002041003.  No JS Errors are generated.

I have tried it using multiple and new profiles on the win2k platform.  The test
case you describe actually causes the same behavior on my box.  I'l add in the
strict js messages.
Steps to reproduce (on two separate win2k boxes):

1.  Create a new profile and set up mail account settings.
2.  Import a dual-use certificate and the CA that signed it
3.  Make sure the CA Trust bits are set
4.  Set the dual-use cert as the signing and encryption cert in mail settings
5.  Compose a message to foo@netscape.com
6.  Choose to digitally sign and encrypt the message (you should not have a cert
for foo)
7.  Click send

You should see a failure message (no encryption cert).  Click ok, the focus is
lost and the composition frame becomes immutable.

there are no javascript errors in the console with/without strict option set.

I would have guessed this is a general mail composer problem, not especially
related to S/Mime.

We should try what happens when you are not using S/Mime, but maybe disturb your
mail settings (wrong server/port), and see what happens when sending fails
because of that.

Kai, can you reproduce this. I can't works like a charm for me.
Altering regular mail settings to generate errors does yield a loss of window
focus, but the composition frame remains editable.

If you can't reproduce it, I will keep this on hold until it reappears on
another system.  I am using a clean installation of the latest commercial 1.0
builds.
Nominating for beta.  the immutability of the message composition frame needs to
be fixed.
Keywords: nsbeta1
John, can you reproduce. Nobody seems to be able to reproduce this.
Kai, please try to reproduce.
Assignee: ssaux → kaie
If the window does not re-enabled its widgets, that means
nsMsgComposeSendListener::OnStopSending never get called!
I have also reproduced this on the Mac 9.2 platform.
Reproducible on 4/15 Win2000 trunk build.
Severity: normal → major
Priority: -- → P2
I can reproduce the bug, and it happens on the branch, too.
But I don't have a solution yet.

JF, your guess is correct, that method is indeed not reached, I traced that.

When the crypto code is unable to prepare for sending, an exception in
MsgComposeSend is triggered:

failed to SendMsg: [Exception... "Component returned failure code: 0x80004005 (N
S_ERROR_FAILURE) [nsIMsgCompose.SendMsg]"  nsresult: "0x80004005 (NS_ERROR_FAILU
RE)"  location: "JS frame :: chrome://messenger/content/messengercompose/MsgComp
oseCommands.js :: GenericSendMessage :: line 1694"  data: no]

This is different from what happens, when sending fails for other reasons. For
example, if sending fails because the server is unreachable, this exception does
not get triggered.
4/10 build doesn't exhibit this problem. nsMsgComposeSecure::MimeCryptoHackCerts
has always returned ns_error_failure when something went wrong. We also set the
error message to present to the user.
We have not changed code in the extension.

If this is indeed a regression, then s/mime is effectively disabled.
Keywords: nsbeta1 → nsbeta1+, regression
Whiteboard: [adt1]
Well, actually I changed something recently, landing of bug 131087. It changed
some code nearby.

But I tested backing this change out locally, and I still see the same
behaviour, so it can't be the cause.

I think I found a behaviour that proves that this bug is not related to S/Mime,
but only to the message compose window.

Please try the following:
- compose a new message
- do not type a recipient
- do not type a subject
- do not enter something in the body
- click the send button.
- when Mozilla promps you for a subject, accept the default and click ok
- a warning shows up, confirm it

Now the body field doesn't get focus, and can be edited, too.

Jean-Francois, ff you agree, please assign bug to Mailnews.
Thanks.


reassign to myself for further investigation...
Assignee: kaie → ducarroz
I can reproduce the problem following steps described in comment #16 But in that
case, nsMsgComposeSendListener::OnStopSending must have been called as we
re-eable correctly others field and button in the window. Do we have 2 issue
here? one about smime not always returning an error and one about the body
getting locked.

Varada, do you have time to take care of the body issue?
taking bug from the much overloaded Jean-Francois :)
Assignee: ducarroz → varada
Note, that the S/Mime message failure shows the same behaviour, i.e. header
editable, body not, so both scenarios should have the same cause.

I think this was happening because we were enabling the editable fields twice -
and the editor body which was enabled using manipulation of flag bits was
getting disabled by the second call. I have changed the manipulation a bit and
it works for the few conditions that I tried.
Whiteboard: [adt1] → [adt1], fix in hand - waiting on reviews.
Comment on attachment 79531 [details] [diff] [review]
Changes to MsgComposeCommands.js

looks good
r=srilatha
varada, did your test cases include the s/mime code paths that also demonstrated
this problem?
Scott, I just tested, and I can confirm that the fix also fixes the failure on
sending a S/Mime message.

Scott, I just saw you were not cc'ed on the bug. Please see my answer to your
question in the previous comment.

nominating for checking into branch.
Keywords: adt1.0.0
Whiteboard: [adt1], fix in hand - waiting on reviews. → [adt1], checked into trunk.
If it's checked into the trunk, don't forget to change the status to resolved.
Status: NEW → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Blocks: 138000
Focus is still lost (minor), but indeed the composition window remains editable.

Verified.  Please upgrade to adt1.0.0+, and add the fixed1.0.0 keyword when
moved into the branch.
Status: RESOLVED → VERIFIED
adt1.0.0+ (on ADT's behalf) approval for checking into the 1.0 branch. Pls check
this in today, then add the fixed1.0.0 keyword.
Keywords: adt1.0.0 → adt1.0.0+
Comment on attachment 79531 [details] [diff] [review]
Changes to MsgComposeCommands.js

sr=mscott
Attachment #79531 - Flags: superreview+
Comment on attachment 79531 [details] [diff] [review]
Changes to MsgComposeCommands.js

carrying over srilatha's r= from bug comments, and approving.
Attachment #79531 - Flags: review+
Attachment #79531 - Flags: approval+
Checked into Branch.
Keywords: fixed1.0.0
Keywords: fixed1.0.0 → verified1.0.0
Product: PSM → Core
Version: psm2.3 → 1.0 Branch
Product: Core → MailNews Core
QA Contact: carosendahl → s.mime
You need to log in before you can comment on or make changes to this bug.