Supply "Stop" and "Send" buttons in Spell Checking dialog to support automatic spell checking when sending a mail message

VERIFIED FIXED in mozilla1.0

Status

MailNews Core
Composition
VERIFIED FIXED
16 years ago
9 years ago

People

(Reporter: Charles Manske, Assigned: varada)

Tracking

Trunk
mozilla1.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

905 bytes, patch
Details | Diff | Splinter Review
(Reporter)

Description

16 years ago
This was originally bugscape bug 3581.
1. Launch N6 [ or mozilla with spellchecker installed]
2. Click on the Mail icon from the Task Bar to launch Netscape Mail
3. Select Edit | Preferences
4. From the Prefrences window, select Mail and Newsgroups | Message Composition
5. From Message Composition, check the "Check spelling before sending" box
6. Click OK in the Prefrences window
7. From Mail window, click on New Message button
8. Compose a message with some incorrect words
9. Click on SEND

Results: You receive a Spell Check window, however, if you exit the dialogue by 
clicking on CLOSE or the X in the upper right corner, the message is sent.  

Note: This is not how 4.x worked. 4.x had a STOP button, and allowed the user to 
elect to STOP the Spell Check, and go back into the message to edit (Note:This 
is how Spell Check performs in 6.0 when you click on the Spell button).
(Reporter)

Updated

16 years ago
Status: NEW → ASSIGNED
Keywords: nsbeta1+
Summary: Can't stop a SEND, if Spell Check is enabled → Supply "Stop" and "Send" buttons in Spell Checking dialog to support automatic spell checking when sending a mail message
Whiteboard: FIX IN HAND
Target Milestone: --- → mozilla1.0
(Reporter)

Comment 1

16 years ago
Created attachment 75506 [details] [diff] [review]
Patch v3

This is identical to patch attached to bugscape 3581,
except Composer launches spelling dialog with the param
that signifies "spell check during send mail" action set false:
      window.openDialog("chrome://editor/content/EdSpellCheck.xul", "_blank",
	      "chrome,close,titlebar,modal", false);
Mail application should use this:
      window.openDialog("chrome://editor/content/EdSpellCheck.xul", "_blank",
	      "chrome,close,titlebar,modal", sendMode);
where "sendMode" is true when spell checking during send action.
(Reporter)

Comment 2

16 years ago
Comment on attachment 75506 [details] [diff] [review]
Patch v3

sr=hewitt
Attachment #75506 - Flags: superreview+

Comment 3

16 years ago
Comment on attachment 75506 [details] [diff] [review]
Patch v3

r=andreww - looks ok.
Attachment #75506 - Flags: review+
(Reporter)

Comment 4

16 years ago
*** Bug 83933 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 5

16 years ago
*** Bug 132701 has been marked as a duplicate of this bug. ***

Updated

16 years ago
Blocks: 119232

Comment 6

16 years ago
*** Bug 86296 has been marked as a duplicate of this bug. ***

Comment 7

16 years ago
Quick question - will "Send Later" work correctly with this patch? I haven't
tested this patch yet (I will), but when I tried the experimental patch from big
83933, "Send Later" would cause message to be sent right away after spellchecker...
OS: Windows 2000 → All
(Reporter)

Comment 8

16 years ago
Aleksey: That would be totally dependent on what Varada does with the 
"window.cancelSendMessage" var the spell checker is closed.
So that is part of work that needs to be done in messagecompose.js and is not 
included in "Patch v3".
(Assignee)

Comment 9

16 years ago
I shall test the mailcompose part and attach a patch.

Comment 10

16 years ago
Comment on attachment 75506 [details] [diff] [review]
Patch v3

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #75506 - Flags: approval+
(Reporter)

Comment 11

16 years ago
Comment on attachment 75506 [details] [diff] [review]
Patch v3

This was checked in.
Message composer code now needs to be fixed.
Attachment #75506 - Attachment is obsolete: true
(Reporter)

Comment 12

16 years ago
Composer side is fixed. Message Composer needs to be changed as described in
comment #1.
Assignee: cmanske → varada
Status: ASSIGNED → NEW
Component: Editor: Composer → Composition
Product: Browser → MailNews
Whiteboard: FIX IN HAND
(Assignee)

Comment 13

16 years ago
Created attachment 75730 [details] [diff] [review]
MailCompose Portion of fix.

I tested SendLater and the spellchecker comes up and can be dismissed the same
way as for SendNow - either stop and edit or send. If send is chosen then it is
stored in the Unsent Messages folder. When we choose to send unsent messages it
is sent without any dialog - but we always have an option of going to that
folder and editing the message.
(Reporter)

Comment 14

16 years ago
Comment on attachment 75730 [details] [diff] [review]
MailCompose Portion of fix.

Since sendMode is always
true, you don't need to bother having a separate var, just use "true" in
openDialog with a comment to explain it.
I suggested that originally because I thought this was used by both "normal"
spell checking and just before send, but I now realize the former routes
through ComposerCommands.js.
r=cmanske with that change.
Attachment #75730 - Flags: review+

Comment 15

16 years ago
Comment on attachment 75730 [details] [diff] [review]
MailCompose Portion of fix.

sr=bienvenu, IFF you make the change Charlie pointed out.
Attachment #75730 - Flags: superreview+
(Assignee)

Comment 16

16 years ago
Created attachment 76105 [details] [diff] [review]
Patch V2

removed the var sendMode and invoked the function with just true.
Attachment #75730 - Attachment is obsolete: true

Comment 17

16 years ago
Comment on attachment 76105 [details] [diff] [review]
Patch V2

a=asa (on behalf of drivers) for checkin to the 1.0 trunk
Attachment #76105 - Flags: approval+
(Assignee)

Comment 18

16 years ago
Marking Fixed.
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Comment 19

16 years ago
Olga, please verify...thanks
QA Contact: sujay → olgam

Comment 20

16 years ago
Tested with BuildID 2002032603 on RedHat Linux 7.2, works great!

Updated

16 years ago
QA Contact: olgam → esther

Comment 21

16 years ago
Verified on trunk recent builds, Win2K, Linux, MacOSX.
Status: RESOLVED → VERIFIED
QA Contact: esther → olgam
Product: MailNews → Core
Product: Core → MailNews Core
You need to log in before you can comment on or make changes to this bug.