3459 - Spell Check should also check the subject line.

Status: VERIFIED FIXED

(MailNews Core :: Composition, defect, P3)

Description

[bfox]

(This bug imported from BugSplat, Netscape's internal bugsystem.  It
was known there as bug #104347
http://scopus.netscape.com/bugsplat/show_bug.cgi?id=104347
Imported into Bugzilla on 03/05/99 14:45)

Comment 1

[Marek Z. Jeziorek]

mass-setting TFV for enhancements to 5.0 (we will not deal with them in 4.x
tree, unless the resources to fix it and test it come from the 5.0 team)

Comment 2

[yoel]

Marek,
Please assign this to someone who exists even if it never get's fixed. Thanks
Yoel

Comment 3

[Marek Z. Jeziorek]

assigning to sol (please strongly consider this in 5.0 messanger)

Comment 4

[lchiang]

enhancement request brought over from bugsplat.  Assigned to sol for processing
of enhancement - if this is a feature for 5.0, then pls add to PRD and assign to
engineer.

Comment 5

[lchiang]

per phil, reassign to ducarroz.

Comment 6

[sol]

I don't think we need to do this RFE for PR1.

Comment 7

[Phil Peterson]

Add [help wanted] to summary, so this RFE will be on the radar for mozilla
contributors. Moved to M15, with the rest of the RFEs.

Comment 8

[Phil Peterson]

Bulk-resolving requests for enhancement as "later" to get them off the Seamonkey
bug tracking radar. Even though these bugs are not "open" in bugzilla, we
welcome fixes and improvements in these areas at any time. Mail/news RFEs
continue to be tracked on http://www.mozilla.org/mailnews/jobs.html

Comment 9

[Phil Peterson]

Reopen mail/news HELP WANTED bugs and reassign to [e-mail]

Comment 10

[Phil Peterson]

Put HELP WANTED in the Status Whiteboard so the mozilla.org bug query finds them

Comment 11

[pmock]

Changing qa assigned to [e-mail]

Comment 12

[Philip Nemec]

Checking to see what needs to be done to get this fixed for Mozilla 1.0 or so...

Comment 13

[u32858]

Hi,

I just found this bug.. is anyone interested in fixing it?  I think it must be
fairly trivial to change it to accept spell check of the subject right?

Regards

JG

Comment 14

[Scott]

I agree that this needs to be fixed.  Shouldn't it be easy?

Comment 15

[Peter Kroll]

requested in 1998, spellchecker has been added to mozilla now. so this feature
should complete the spellchecker functionality. can it be completed for 1.6 final?

Comment 16

[Mikael Parknert]

A comment in code says spellcheck is disabled on subject etc,
se:
http://lxr.mozilla.org/seamonkey/source/mailnews/compose/resources/content/MsgComposeCommands.js#1561

Maybe some reason for that?

Comment 17

[Oliver Klee]

*** Bug 229577 has been marked as a duplicate of this bug. ***

Comment 18

[Scott MacGregor]

taking.

the hard part is that the spell checker api expects a nsIEditor instance to
spell check. THe subject field is a text box. I dont' know if I have the ability
to get the nsIEditor out of a textbox in JS.

Comment 19

[Scott MacGregor]

Bug #16409 may also shed some light

Comment 20

[Scott MacGregor]

I think this is going to be extremely hard. I just added a bunch of code to try
to extract the nsIEditor instance that gets created with the xul:textbox for the
subject field. I then tried to pass that into nsIEditorSpellcheck which runs the
spell checker on a nsIEditor object.

Unfortunately, nsEditorSpellCheck::InitSpellChecker gets the document associated
with the editor instance and then walks the tree looking for text nodes to spell
check. For a xul:textbox, the document is messengercompose.xul, so we start
walking through our XUL file, spell checking all of our xul text nodes! For
instance I just got prompted for misspelling #ifdef XP_OSX..zoinks.

Fixing this may mean major re-design of the spell checker to support individual
text box elements instead of requiring an editor instance. Or at the very least
a change to how we walkt the whole document looking for text nodes. Neither of
these approaches is going to be a bug fix level change.

Comment 21

[Aleksey Nogin]

> Fixing this may mean major re-design of the spell checker to support individual
> text box elements instead of requiring an editor instance.

If this is the case, you might want to also keep in mind the bugs that ask for
an ability to spellcheck a form (bug 23421 and bug 16409) - may be at least that
big re-design can be done in a way that would make it easier to fix all of those.

Comment 22

[Tal Cohen]

Scott, you might wish to consider having the spellcheck function accept an 
array of elements to check rather than just a single element. This way you can 
manually feed it the edit box and the subject input box, rendering irrelevant 
the internal search for text-elements.

The important thing would be to make this a single spellcheck session, rather 
than two individual sessions (or more, depending on the array size). For 
example, if I mark a word to ignore in the subject, it should also be ignored 
in the message body.

Comment 23

[Scott MacGregor]

that's exactly what I did. I made it accept an array of editor instances. But
the core nsEditorSpellCheck stuff makes rampant uses of nsIEditor which is why
it will be a big undertaking to pass in an array of arbitrary elements that may
not be nsIEditor instances.

Comment 24

[rbs]

1) ...or you can manufacture a prior selection that covers what you want and set
it in nsEditorSpellCheck::InitSpellChecker(aEditor,
aEnableSelectionChecking=TRUE) (while toggling the selection display flag off so
that the user doesn't see any apparent effect as to what is going on under the
hood... nsISelectionController::SELECTION_OFF)

2) ...or you can add another API that init() with a certain range (in the
aEditor's document), if you don't want to piggy back on
aEnableSelectionChecking=TRUE.

Option 1) seems fine to me, and should build on what you did in comment 20, and
shouldn't require much re-design. You can clear/reset the artificial selection
just after doing the init since it is only needed there.

Comment 25

[Scott MacGregor]

rbs, could you elaborate more on option (1)? My editor knowledge isn't up to
snuff enough to fully grok what you were suggesting :)

Comment 26

[Michiel van Leeuwen (email: mvl+moz@)]

Or you could call mozISpellcheckingEngine directly. What that needs is something
that can split a string into words. mozISpellI18NUtil.idl can help with that.

Comment 27

[rbs]

> rbs, could you elaborate more on option (1)

You mentioned in comment 20 that "nsEditorSpellCheck::InitSpellChecker gets the
document associated with the editor instance and then walks the tree looking for
text nodes to spell check. For a xul:textbox, the document is
messengercompose.xul, so we start walking through our XUL file, spell checking
all of our xul text nodes!"

Assuming you managed to get hold of the editor inside the textbox as you said.
What I am suggesting is:

1. create a range to delimit the textbox
   nsCOMPtr<nsIDOMRange> spellCheckRange(do_CreateInstance(kRangeCID));
   spellCheckRange->SelectNodeContents(textBoxNode);

2. get the selection controller from the editor and hide the display
   nsCOMPtr<nsISelectionController> selCon;
   aEditor->GetSelectionController(getter_AddRefs(selCon));
   selCon->SetDisplaySelection(nsISelectionController::SELECTION_HIDDEN);

3. select the the textbox (selection is there, but won't show up due to 2.)
   nsCOMPtr<nsISelection> sel;
   selCon->GetSelection(nsISelectionController::SELECTION_NORMAL,
     getter_AddRefs(sel));
   sel->RemoveAllRanges();
   sel->AddRange(spellCheckRange);

4. Init the spell checker as you die=d, but with aEnableSelectionChecking=TRUE
   (see the code for the effect)
   InitSpellChecker(aEditor, PR_TRUE/*aEnableSelectionChecking*/)

5. restore the display so that the user sees the selections of the spell checker
   sel->RemoveAllRanges();
   selCon->SetDisplaySelection(nsISelectionController::SELECTION_ON);

6. Do the spell checking as you normally do...
----
This will only lookup what is in |spellCheckRange|. So _in general_, depending
on what you put spellCheckRange, you can restrict to whatever you want without
much re-design.

Comment 28

[jesse]

*** Bug 246485 has been marked as a duplicate of this bug. ***

Comment 29

[rbs]

mscott, can you attach what you had here? This way, it would be clear where you
are up, and who knows, perhaps others could chime in and help from there.

Comment 30

[Scott MacGregor]

sorry those changes are long gone in a hard drive crash.

I won't be bouncing back to this until after 1.0...

Comment 31

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 253485 has been marked as a duplicate of this bug. ***

Comment 32

[Stephen Donner [:stephend] Not actively reading bugmail]

*** Bug 253484 has been marked as a duplicate of this bug. ***

Comment 33

[Jo Hermans]

*** Bug 261149 has been marked as a duplicate of this bug. ***

Comment 34

[Mike Cowperthwaite]

*** Bug 268322 has been marked as a duplicate of this bug. ***

Comment 35

[Oliver Klee]

*** Bug 271568 has been marked as a duplicate of this bug. ***

Comment 36

[Mike Cowperthwaite]

*** Bug 272442 has been marked as a duplicate of this bug. ***

Comment 37

[Mike Cowperthwaite]

*** Bug 283702 has been marked as a duplicate of this bug. ***

Comment 38

[Jo Hermans]

*** Bug 293796 has been marked as a duplicate of this bug. ***

Comment 39

[Jim Booth]

*** Bug 296106 has been marked as a duplicate of this bug. ***

Comment 40

[Adam Guthrie]

*** Bug 301515 has been marked as a duplicate of this bug. ***

Comment 41

[Adam Guthrie]

*** Bug 307863 has been marked as a duplicate of this bug. ***

Comment 42

[Magnus Melin [:mkmelin]]

*** Bug 309515 has been marked as a duplicate of this bug. ***

Comment 43

[vhsgg70bvulosu7]

Is anyone actually working on this?

Comment 44

[Jo Hermans]

*** Bug 335969 has been marked as a duplicate of this bug. ***

Comment 45

[Magnus Melin [:mkmelin]]

*** Bug 340190 has been marked as a duplicate of this bug. ***

Comment 46

[(not reading, please use [e-mail] instead)]

is this easier now that brettw's work for spell checking text areas has landed?

Comment 47

[Scott MacGregor]

With the recent work to expose inline spell checking to html input fields, this bug has become much easier to implement.

I think all we have to do is:

1) Turn on spell checking for the msg subject editor instance in MsgComposeCommands.js:

GetMsgSubjectElement().editor.setSpellcheckUserOverride(true);

2) Add the right hooks to contentAreaContextMenu to actually build the list of mispelled items in the content area context menu. This context menu currently lives here: http://lxr.mozilla.org/mozilla/source/xpfe/communicator/resources/content/contentAreaContextOverlay.xul#61
but it looks like we (thunderbird) are the only remaining consumers. The suite has forked the file into mozilla/suite. I think it's time we forked and built our own contentAreaContextMenu in mail\base\content here, adding the extra items for dictionary support.

Comment 48

[Adam Guthrie]

We could either do what Scott mentioned in comment 47, or wait for bug 346787 to get fixed, which would make this a one-line fix.

Comment 49

[andy]

even microsoft outlook has this feature

Comment 50

[Ryan Foley]

bug 346787 is now marked fixed.  does someone want to fix this 9-year-old bug?

Comment 51

[Scott MacGregor]

Attachment: one line fix to enable spell checking in the subject!

our toolkit team rocks. This became a one line change.

The only downside is that the strings the toolkit context menu uses are slightly different than the strings the app content menus use.

i.e. in the subject field you see: (No Spelling Suggestions)

where in the message body you would instead see: No Suggestions Found

Also ignore word isn't showing up in the text box case either. 

But those are small prices to pay for having spell check support in the subject I think.

Comment 52

[Scott MacGregor]

I've checked this patch into the trunk. 

I'm going to leave the bug open though, as I think we should propagate user events to turn off inline spell checking in the compose window out to the subject box so it stops spell checking too. 

Comment 53

[Scott MacGregor]

Attachment: subject inline spell check should honor compose window setting

This patch keeps subject inline spell check in sync with the global setting for the compose window. If you toggle inline spell check on/off in the compose window, the subject field honors the setting. Likewise for the global pref.

Comment 54

[Scott MacGregor]

This is now fixed on the branch and the trunk for Thunderbird 2. Woo Hoo.

There are issues which I doubt we address for tb 2. In particular, the subject field textbox is really a 2nd instance of the spell checker and it's not tied into the inline spell checker for the mail body. i.e. If you ignore a word in one, it doesn't automatically get ignored in the other unless force spell check to run on the word again. Those can be addressed in follow up bugs.

Comment 55

[Mike Cowperthwaite]

(In reply to comment #54)
> If you ignore a word in one, it doesn't automatically get ignored in the
> other unless force spell check to run on the word again.

I don't see an Ignore option at all in the Subject.  


Spellchecking of the subject requires that the Inline Spell Check option is turned on; if you turn that off, performing a regular spell check (manual or automatic-before-send) still ignores the subject.

There is a menu item, "Spell check this field" which always initializes to checked -- unchecking it turns off the inline spellchecking in the subject.  I'm not sure how useful this menu item is -- what's the purpose of turning it off dynamically, just to hide the red squiggles?

Comment 57

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #55)
> Spellchecking of the subject requires that the Inline Spell Check option is
> turned on; if you turn that off, performing a regular spell check (manual or
> automatic-before-send) still ignores the subject.

Mike, is a bug out there which handles this issue? Due to this bug is marked as fixed, we should file a new one?

Comment 58

[Mike Cowperthwaite]

(In reply to comment #57)
> (In reply to comment #55)
> > Spellchecking of the subject requires that the Inline Spell Check option is
> > turned on; if you turn that off, performing a regular spell check (manual or
> > automatic-before-send) still ignores the subject.
> 
> Mike, is a bug out there which handles this issue?

Bug 368915 looks like it's about this.

Comment 59

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks. I have overseen the depends field.

Verified for trunk and 1.8 branch.