Open Bug 216235 Opened 21 years ago Updated 2 years ago

Spell Check should ignore reply header text on check before send

Categories

(Thunderbird :: Message Compose Window, defect, P3)

Tracking

(Not tracked)

Thunderbird 3.0rc1

People

(Reporter: mscott, Unassigned)

References

Details

Attachments

(1 file)

The spell checker currently ignores quoted message bodies. But we don't ignore the reply header text we insert into the editor instance right before the quoted text. i.e. "John Doe wrote:" **insert quoted text which we dont spell check** Here's how we currently insert a quoted message into editor: 1) We call InsertHTML or InsertText to insert the reply header 2) Then we call InsertQuotationWithCitation to insert the actual reply. This reply gets marked with a block="cite" attribute on it so the spell checker knows to ignore it. In order to fix this bug I think I need to do the following: Extend nsIEditorMailSupport.idl to add reply text support. I could see this happening two ways: 1) add a new method, say InsertReplyText or InsertQuotationPrefix which creates a dom text fragment just like the existing InsertQuotedCitation does and sets an attribute on this dom fragment which the spell checker filter code could look for and then ignore the element. 2) Extend insertAsCitedQuotation to pass in a prefix string which gets created as a separate dom fragment from the quotation dom node (we don't want it being part of the block quote of course). Then set an attribute on this prefix dom node so the spell checker can ignore it. One problem with this approach is that insertAsCitedQuotation returns a DOMNode. I'd be changing it to create two dom nodes but only returning the second one (the actual quoted one). That's probably not going to cause a problem but could be considered inconsistent. cc'ing Kin in case he has a suggestion or a preference for one of these approaches.
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird0.2
I'll talk to Kin about this for thunderbird 0.3
Target Milestone: Thunderbird0.2 → Thunderbird0.3
I decided it was a lot easier to solve this without changing any of the mail editor APIs. I can just wrap the reply text with span that has a special class set on it in mail compose directly. The spell checker filter checks for this attribute on spans and skips the contents if appropriate. Patch coming up. This patch also includes a small change to support skipping signatures as well.
Attached patch the patch (deleted) — — Splinter Review
The patch looks a little more complicated than it is because I collapsed a couple nested if clauses in nsComposeTxtSrvFilter. In short it does the following: 1) the filter used to look for pre or span tags with _moz_quote=true on it. _moz_quote is used by editor to denote and style quoted text. We now also look for _moz_skip_spellcheck (is this name okay? do we want another name here?) which is an attribute mail compose sets on elements that are not part of the quoted text but which we want to ignore when it comes to spell checking. For HTML signatures, we also look for divs in addition to pre and span tags with this attribute set on it. 2) In mail compose, we wrap the reply text with a span (is that the right element to use? I think it is the most harmless), with an attribute of _moz_skip_spellcheck="true". 3) In mail compose, the div and the pre (for plain text) used for the signature now includes the attribute/value pair: _moz_skip_spellcheck="true".
Attachment #131299 - Flags: review?(kin)
Blocks: 219336
Attachment #131299 - Flags: superreview?(bienvenu)
Attachment #131299 - Flags: superreview?(bienvenu) → superreview+
i've noticed this on macOS X, too, but can't change the HW/OS fields since i'm not owner.
Yup, this is generic; changing to All.
OS: Windows XP → All
Hardware: PC → All
QA Contact: asa
This has been fixed on the 0.3 branch. Will wait for the review from Kin before landing on the trunk for all mozilla mail customers.
So your changes look fine to me ... but one concern I have is that reply header text isn't accounted for in the edit rules, so that means that if the user places the caret at the beginning of: |Kin wrote: and started typing, his content would end up in the span, and not be spellchecked. Same thing with the div used for the sig.
for historical purposes, I have never checked in this patch because I ran into exactly what Kin mentioned in his review comments. It is too easy for user text to get inserted inside the reply header span and suddenly that text gets missed by the spell checker. I don't know enough about editor yet to know if there is a way to keep stuff out of that span....
Blocks: 119232
I would like to see this issue picked back up again. The issue, at least what I am having in 0.8, is that when I reply, the text is quoted with > and the spell checker ignores it, but it checks the name of the previous author. This is very irriatating when that is the only word spelled wrong. I think it should just add the previous author to the quoted text. This would solve another problem that arises in long threads, because the person who wrote something is quoted on a different level. For example: Hi am Peter >Paul Smith Wrote > :I am Paul Smith > : > : John Does wrote > : >I am John Doe. It can start to become unclear who wrote what as you go further through several posts in a newsgroup. Thanks, Peter
What about this comment 2 at bug 245897 ? Seems like a much simpler solution to me (and shouldn't someone consolidate these two bugs?) https://bugzilla.mozilla.org/show_bug.cgi?id=245897#c2
It might work every now and then if it is a frequent contact, but when replying to newgroups and heavily used help lists, it doens't really work. Thanks for checking though. I just thought they could put a little > in front of that part too.
(In reply to comment #11) > It might work every now and then if it is a frequent contact, but when replying > to newgroups and heavily used help lists, it doens't really work. Presumably, when TB composes the reply and inserts the header text, it knows the text it is inserting (e.g. "Foo Bar wrote:). It's just a question of holding onto that text, and when the spellchecker is initialized, putting that in the "ignore all" list.
See also Bug 190660 for a related issue.
*** Bug 309602 has been marked as a duplicate of this bug. ***
Comment on attachment 131299 [details] [diff] [review] the patch Scott, does Kin still do reviews? This patch has been open for 3,5 years.
QA Contact: message-compose
I don't think so...
Comment on attachment 131299 [details] [diff] [review] the patch See comment 7 and comment 8 - Kin's review was "do you really want to have an invisible area where people might type with no spellcheck?" and the answer was no, we don't. Canceling the request to avoid waves of stale review request triagers.
Attachment #131299 - Flags: review?(kinmoz)
I have a possible design for resolving this spellcheck bug. Presently spellcheck uses two dictionaries, the master and personal dictionaries. I suggest creation of a third Scratch pad dictionary in RAM that would be populated with the identity of the person replied to and the content of the Sig block. This gives spellcheck a third source of known good words. Then when the reply is sent, the scratch pad is flushed to be ready for the next reply to be composed.
Wasn't this fixed by Bug 280826 a long time ago? When I reply to a message, the person's name in the citation field isn't spell checked.
Sorry Scott but I still see the original behavior that you generated this bug for. Tb 2.0.0.6 release. I do not know about trunk, as I have not used one of them since the 0.4 release. OS Win ME. Saw this was still open when searching spellcheck bugs because I may have a new one in this build. Still testing.
I see this In current trunk also, the name in the citation field is spell checked but only if you have the option ticked to "spellcheck before you send"
Thanks for pointing me to the right bug Phil. Bug still exists in 2.0.0.9, both in Win and OSX.
Went and looked at Bug 280826 referenced in Comment #19. That bug was patched by generating an 'ignore list' for inline spell check. That patch does not make an ignore list that is also usable by 'spell check on send' if the 'Inline' option is not also a selected preference. This bug is still unresolved for the case of only the 'check on Send' being the users preference. This bug needs an ignore list that is auto populated with the %s variable for name of original sender within the reply header. Whether this will need to be expanded to ignore message ID depends on how that new variable gets added to reply headers in Tb 3.
Nominating wanted-thunderbird3 to keep it on the radar.
Flags: wanted-thunderbird3?
Flags: wanted-thunderbird3? → wanted-thunderbird3+
changing summary to reflect remaining open issue.
Summary: Spell Check should ignore reply header text → Spell Check should ignore reply header text on check before send
Priority: -- → P3
Target Milestone: Thunderbird0.3 → Thunderbird 3.0rc1
Assignee: mscott → nobody
Status: ASSIGNED → NEW
This bug is still active, i.e. replies typically have the "On xx/xx/xx Yyy Zzz wrote: and the Zzz usually triggers a spelling error. I also notice that "Forward" is tested differently, i.e. the body is also checked for errors. I think it should be treated the same - or is that a different bug?
Hi there, Thanks for trying to improve the spell-check! As an additional request, may I request to also exclude the signature from spell-check? Maybe through an optional checkbox or about:config entry? Robert
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: