Spell Check should also check the subject line.

VERIFIED FIXED

Status

P3
normal
VERIFIED FIXED
21 years ago
3 years ago

People

(Reporter: bfox, Assigned: mscott)

Tracking

(Depends on: 1 bug, {verified1.8.1.2})

Trunk
verified1.8.1.2
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

21 years ago
(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

20 years ago
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

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

Comment 3

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

Updated

20 years ago
QA Contact: 4495

Comment 4

20 years ago
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.

Updated

20 years ago
QA Contact: 4495 → 4080

Updated

20 years ago
Assignee: sol → ducarroz

Comment 5

20 years ago
per phil, reassign to ducarroz.

Updated

20 years ago
Status: NEW → ASSIGNED
Target Milestone: M10

Comment 6

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

Updated

19 years ago
Summary: RFE: Spell Check should also check the subject line. → [HELP WANTED] Spell Check should also check the subject line.
Target Milestone: M10 → M15

Comment 7

19 years ago
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

19 years ago
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

19 years ago
Reopen mail/news HELP WANTED bugs and reassign to nobody@mozilla.org

Updated

19 years ago
Whiteboard: HELP WANTED

Comment 10

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

Updated

19 years ago
QA Contact: lchiang → pmock

Comment 11

19 years ago
Changing qa assigned to pmock@netscape.com

Updated

19 years ago
Keywords: helpwanted

Updated

19 years ago
Summary: [HELP WANTED] Spell Check should also check the subject line. → Spell Check should also check the subject line.
Whiteboard: HELP WANTED

Updated

19 years ago
Target Milestone: M15

Updated

18 years ago
Keywords: mozilla1.0

Updated

17 years ago
Blocks: 119232

Comment 12

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

Comment 13

16 years ago
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

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

Updated

15 years ago
Flags: blocking1.6b+

Updated

15 years ago
Flags: blocking1.6b+

Comment 15

15 years ago
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

15 years ago
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

15 years ago
*** Bug 229577 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 18

15 years ago
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
Keywords: helpwanted, mozilla1.0
(Assignee)

Comment 19

15 years ago
Bug #16409 may also shed some light
(Assignee)

Comment 20

15 years ago
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

Comment 21

15 years ago
> 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

15 years ago
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.
(Assignee)

Comment 23

15 years ago
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.

Updated

15 years ago
QA Contact: esther → core.spelling-checker

Comment 24

15 years ago
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.
(Assignee)

Comment 25

15 years ago
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.

Comment 27

15 years ago
> 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

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

Comment 29

14 years ago
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.
(Assignee)

Comment 30

14 years ago
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. ***

Comment 33

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

Comment 34

14 years ago
*** Bug 268322 has been marked as a duplicate of this bug. ***
Product: MailNews → Core

Comment 35

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

Comment 36

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

Comment 37

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

Comment 38

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

Comment 39

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

Comment 40

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

Comment 41

13 years ago
*** Bug 307863 has been marked as a duplicate of this bug. ***
*** Bug 309515 has been marked as a duplicate of this bug. ***

Comment 43

13 years ago
Is anyone actually working on this?

Comment 44

13 years ago
*** 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?
(Assignee)

Comment 47

12 years ago
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

12 years ago
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

Comment 49

12 years ago
even microsoft outlook has this feature

Comment 50

12 years ago
bug 346787 is now marked fixed.  does someone want to fix this 9-year-old bug?
(Assignee)

Comment 51

12 years ago
Created attachment 253131 [details] [diff] [review]
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.
Attachment #253131 - Flags: superreview?(bienvenu)

Updated

12 years ago
Attachment #253131 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 52

12 years ago
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. 
(Assignee)

Comment 53

12 years ago
Created attachment 253221 [details] [diff] [review]
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.
Attachment #253221 - Flags: superreview?(bienvenu)

Updated

12 years ago
Attachment #253221 - Flags: superreview?(bienvenu) → superreview+
(Assignee)

Comment 54

12 years ago
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
Last Resolved: 12 years ago
Keywords: fixed1.8.1.2
Resolution: --- → FIXED

Updated

12 years ago
Depends on: 368915

Comment 55

12 years ago
(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?

Updated

12 years ago
Blocks: 360488
Duplicate of this bug: 369251
(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

12 years ago
(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
Keywords: fixed1.8.1.2 → verified1.8.1.2

Updated

12 years ago
Blocks: 378434

Updated

11 years ago
No longer blocks: 360488
Product: Core → MailNews Core

Updated

8 years ago
No longer blocks: 378434
Depends on: 378434
You need to log in before you can comment on or make changes to this bug.