Closed Bug 3459 Opened 27 years ago Closed 17 years ago

Spell Check should also check the subject line.

Categories

(MailNews Core :: Composition, defect, P3)

defect

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: bfox, Assigned: mscott)

References

(Depends on 1 open bug)

Details

(Keywords: verified1.8.1.2)

Attachments

(2 files)

(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)
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)
Marek,
Please assign this to someone who exists even if it never get's fixed. Thanks
Yoel
assigning to sol (please strongly consider this in 5.0 messanger)
QA Contact: 4495
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.
QA Contact: 4495 → 4080
Assignee: sol → ducarroz
per phil, reassign to ducarroz.
Status: NEW → ASSIGNED
Target Milestone: M10
I don't think we need to do this RFE for PR1.
Summary: RFE: Spell Check should also check the subject line. → [HELP WANTED] Spell Check should also check the subject line.
Target Milestone: M10 → M15
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.
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
Reopen mail/news HELP WANTED bugs and reassign to nobody@mozilla.org
Whiteboard: HELP WANTED
Put HELP WANTED in the Status Whiteboard so the mozilla.org bug query finds them
QA Contact: lchiang → pmock
Changing qa assigned to pmock@netscape.com
Keywords: helpwanted
Summary: [HELP WANTED] Spell Check should also check the subject line. → Spell Check should also check the subject line.
Whiteboard: HELP WANTED
Target Milestone: M15
Keywords: mozilla1.0
Blocks: 119232
Checking to see what needs to be done to get this fixed for Mozilla 1.0 or so...
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
I agree that this needs to be fixed.  Shouldn't it be easy?
Flags: blocking1.6b+
Flags: blocking1.6b+
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?
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?

*** Bug 229577 has been marked as a duplicate of this bug. ***
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.

Assignee: nobody → mscott
Blocks: 229740
Severity: enhancement → normal
Bug #16409 may also shed some light
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.
Status: NEW → ASSIGNED
> 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.
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.
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.
QA Contact: esther → core.spelling-checker
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.
rbs, could you elaborate more on option (1)? My editor knowledge isn't up to
snuff enough to fully grok what you were suggesting :)
Or you could call mozISpellcheckingEngine directly. What that needs is something
that can split a string into words. mozISpellI18NUtil.idl can help with that.
> 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.
*** Bug 246485 has been marked as a duplicate of this bug. ***
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.
sorry those changes are long gone in a hard drive crash.

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

*** Bug 253485 has been marked as a duplicate of this bug. ***
*** Bug 253484 has been marked as a duplicate of this bug. ***
*** Bug 261149 has been marked as a duplicate of this bug. ***
*** Bug 268322 has been marked as a duplicate of this bug. ***
Product: MailNews → Core
*** Bug 271568 has been marked as a duplicate of this bug. ***
*** Bug 272442 has been marked as a duplicate of this bug. ***
*** Bug 283702 has been marked as a duplicate of this bug. ***
*** Bug 293796 has been marked as a duplicate of this bug. ***
*** Bug 296106 has been marked as a duplicate of this bug. ***
*** Bug 301515 has been marked as a duplicate of this bug. ***
*** Bug 307863 has been marked as a duplicate of this bug. ***
*** Bug 309515 has been marked as a duplicate of this bug. ***
Is anyone actually working on this?
*** Bug 335969 has been marked as a duplicate of this bug. ***
*** Bug 340190 has been marked as a duplicate of this bug. ***
is this easier now that brettw's work for spell checking text areas has landed?
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.
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.
Depends on: 346787
even microsoft outlook has this feature
bug 346787 is now marked fixed.  does someone want to fix this 9-year-old bug?
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.
Attachment #253131 - Flags: superreview?(bienvenu)
Attachment #253131 - Flags: superreview?(bienvenu) → superreview+
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. 
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.
Attachment #253221 - Flags: superreview?(bienvenu)
Attachment #253221 - Flags: superreview?(bienvenu) → superreview+
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.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: fixed1.8.1.2
Resolution: --- → FIXED
Depends on: 368915
(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?
Blocks: TB2SM
(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?
(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.
Thanks. I have overseen the depends field.

Verified for trunk and 1.8 branch.
Status: RESOLVED → VERIFIED
Blocks: 378434
No longer blocks: TB2SM
Product: Core → MailNews Core
Depends on: 433181
No longer blocks: 378434
Depends on: 378434
Depends on: 717292
Depends on: 1176716
You need to log in before you can comment on or make changes to this bug.