Closed Bug 278312 Opened 20 years ago Closed 20 years ago

Inline Spell Checker Back End

Categories

(Thunderbird :: Message Compose Window, defect)

x86
Windows XP
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird1.1

People

(Reporter: mscott, Assigned: mscott)

References

Details

Attachments

(3 files, 13 obsolete files)

10.87 KB, patch
mscott
: superreview+
Details | Diff | Splinter Review
41.39 KB, patch
mscott
: review+
mscott
: superreview+
Details | Diff | Splinter Review
51.20 KB, patch
mscott
: superreview+
Details | Diff | Splinter Review
The changes to layout to support inline spell checking. Broken out from Bug #58612
Status: NEW → ASSIGNED
Target Milestone: --- → Thunderbird1.1
Some of this came from Neil's original patch in 58612.
Attached patch extensions\spellcheck patch (obsolete) — Splinter Review
Attached patch layout changes (obsolete) — Splinter Review
This part is pretty much unchanged from Neil's original patch with the exception of changes I made to nsTextFrame.cpp I also re-merged his layout changes back to the trunk where files have been moved and changed since mozilla 1.6.
Attached patch more layout changes (obsolete) — Splinter Review
adjust the underline color for misspelled words to be a red dotted line
Attachment #171216 - Attachment is obsolete: true
Attached patch more spell checker changes (obsolete) — Splinter Review
1) re-write the inline spell checker to work on end of word delimiters instead of on every character you type. 2) When you backspace back to a mispelled word, clear out the underline. Add the underline back in after a word delimter is added. 3) line breaks should cause us to spell check the last word before the br element is added even if there isn't a word delimter. 4) add argument bullet proofing 5) use error macros and avoid some early returns
Attachment #171215 - Attachment is obsolete: true
Attached patch editor update (obsolete) — Splinter Review
revamp the APIs and JS interface for accessing the inline spell checker. Simplify some of the editor changes.
Attachment #171214 - Attachment is obsolete: true
Note to self: 1) Use the I18N word breaker routine to figure out when we've reached the end of the word instead of calling IsEndOfWordChar. 2) apostrophes don't work correctly (doonnnn't) because nsTextServicesDocument::FindWordBounds treats ' as a end of word terminator
Comment on attachment 171310 [details] [diff] [review] more layout changes David, are you the right person to review thes changes to nsFrame and nsSelection or should I find someone else? Most of these changes were actually made my Neil Deakin for some work he did for NVU, I'm just picking up that part of his patch and using it for inline spell checking.
Attachment #171310 - Flags: superreview?(dbaron)
Comment on attachment 171312 [details] [diff] [review] editor update Hi Daniel, these are the core editor changes required to support inline spell checking. Most of this should look familiar as it is based on Neil's patch for for NVU.
Attachment #171312 - Flags: review?(daniel)
Comment on attachment 171312 [details] [diff] [review] editor update Nothing major but one clarification (quotation marks) and a double-check (leaks) needed. >Index: composer/src/nsComposeTxtSrvFilter.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/composer/src/nsComposeTxtSrvFilter.cpp,v >retrieving revision 1.9 >diff -u -w -r1.9 nsComposeTxtSrvFilter.cpp >--- composer/src/nsComposeTxtSrvFilter.cpp 17 Jun 2004 00:12:35 -0000 1.9 >+++ composer/src/nsComposeTxtSrvFilter.cpp 15 Jan 2005 01:38:58 -0000 >@@ -49,6 +49,7 @@ > mPreAtom = do_GetAtom("pre"); > mSpanAtom = do_GetAtom("span"); > mMozQuoteAtom = do_GetAtom("_moz_quote"); >+ mClassAtom = do_GetAtom("class"); > mTypeAtom = do_GetAtom("type"); > mScriptAtom = do_GetAtom("script"); > mTextAreaAtom = do_GetAtom("textarea"); >@@ -82,6 +83,11 @@ > if (NS_SUCCEEDED(content->GetAttr(kNameSpaceID_None, mMozQuoteAtom, mozQuote))) { > *_retval = mozQuote.LowerCaseEqualsLiteral("true"); > } >+ >+ nsAutoString className; >+ if (NS_SUCCEEDED(content->GetAttr(kNameSpaceID_None, mClassAtom, className))) { >+ *_retval = className.EqualsIgnoreCase("moz-signature"); You're sure about that ? A class name _is_ case sensitive. I understand you consider that "moz-signature" value can only come from inside Mozilla but is it really worth the time spent to ignore case ? >Index: composer/src/nsEditorSpellCheck.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/composer/src/nsEditorSpellCheck.cpp,v >retrieving revision 1.17 >diff -u -w -r1.17 nsEditorSpellCheck.cpp >--- composer/src/nsEditorSpellCheck.cpp 24 Nov 2004 22:48:35 -0000 1.17 >+++ composer/src/nsEditorSpellCheck.cpp 15 Jan 2005 01:38:58 -0000 >@@ -23,6 +23,7 @@ > * Kin Blas <kin@netscape.com> > * Akkana Peck <akkana@netscape.com> > * Charley Manske <cmanske@netscape.com> >+ * Neil Deakin <neil@mozdevgroup.com> (realtime spellchecking) We usually don't mention the reason why a contributor is added. >Index: libeditor/base/nsEditor.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/base/nsEditor.cpp,v >retrieving revision 1.434 >diff -u -w -r1.434 nsEditor.cpp >--- libeditor/base/nsEditor.cpp 24 Nov 2004 22:48:35 -0000 1.434 >+++ libeditor/base/nsEditor.cpp 15 Jan 2005 01:38:59 -0000 >@@ -101,6 +101,7 @@ > #include "nsEditor.h" > #include "nsEditorUtils.h" > #include "nsISelectionDisplay.h" >+#include "nsIInlineSpellChecker.h" > #include "nsINameSpaceManager.h" > #include "nsIHTMLDocument.h" > >@@ -215,6 +216,9 @@ > delete mEditorObservers; // no need to release observers; we didn't addref them > mEditorObservers = 0; > >+ if (mInlineSpellChecker) >+ mInlineSpellChecker->Destroy(); >+ > if (mActionListeners) > { > PRInt32 i; >@@ -1141,6 +1145,23 @@ > return NS_OK; > } > >+NS_IMETHODIMP nsEditor::GetInlineSpellChecker(nsIInlineSpellChecker ** aInlineSpellChecker) >+{ >+ NS_ENSURE_ARG_POINTER(aInlineSpellChecker); >+ nsresult rv; >+ >+ if (!mInlineSpellChecker) { >+ mInlineSpellChecker = do_CreateInstance(MOZ_INLINESPELLCHECKER_CONTRACTID, &rv); >+ NS_ENSURE_SUCCESS(rv, rv); >+ >+ rv = mInlineSpellChecker->Init(this); >+ NS_ENSURE_SUCCESS(rv, rv); >+ } >+ >+ NS_IF_ADDREF(*aInlineSpellChecker = mInlineSpellChecker); >+ Remove spaces on blank line. >+ return NS_OK; >+} > > #ifdef XP_MAC > #pragma mark - >@@ -5203,6 +5224,27 @@ > { > return RemoveAttribute(aElement, aAttribute); > } >+ >+nsresult >+nsEditor::HandleRealTimeSpellCheck(PRInt32 action, >+ nsISelection *aSelection, >+ nsIDOMNode *previousSelectedNode, >+ PRInt32 previousSelectedOffset, >+ nsIDOMNode *aStartNode, >+ PRInt32 aStartOffset, >+ nsIDOMNode *aEndNode, >+ PRInt32 aEndOffset) >+{ >+ return mInlineSpellChecker ? mInlineSpellChecker->SpellCheckAfterEditorChange(action, >+ aSelection, >+ previousSelectedNode, >+ previousSelectedOffset, >+ aStartNode, >+ aStartOffset, >+ aEndNode, >+ aEndOffset) : NS_OK; Hard to read because of the line's length... >Index: libeditor/base/nsEditor.h >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/base/nsEditor.h,v >retrieving revision 1.158 >diff -u -w -r1.158 nsEditor.h >--- libeditor/base/nsEditor.h 12 Sep 2004 05:00:57 -0000 1.158 >+++ libeditor/base/nsEditor.h 15 Jan 2005 01:38:59 -0000 >@@ -59,6 +59,8 @@ > #include "nsIDTD.h" > #include "nsIDOMElement.h" > #include "nsSelectionState.h" >+#include "nsIEditorSpellCheck.h" >+#include "nsIInlineSpellChecker.h" > > class nsIEditActionListener; > class nsIDocumentStateListener; >@@ -547,6 +549,15 @@ > > PRBool GetShouldTxnSetSelection(); > >+ nsresult HandleRealTimeSpellCheck(PRInt32 action, >+ nsISelection *aSelection, >+ nsIDOMNode *previousSelectedNode, >+ PRInt32 previousSelectedOffset, >+ nsIDOMNode *aStartNode, >+ PRInt32 aStartOffset, >+ nsIDOMNode *aEndNode, >+ PRInt32 aEndOffset); >+ > public: > // Argh! These transaction names are used by PlaceholderTxn and > // nsPlaintextEditor. They should be localized to those classes. >@@ -563,6 +574,7 @@ > nsWeakPtr mSelConWeak; // weak reference to the nsISelectionController > nsIViewManager *mViewManager; > PRInt32 mUpdateCount; >+ nsCOMPtr<nsIInlineSpellChecker> mInlineSpellChecker; // used for real-time spellchecking > nsCOMPtr<nsITransactionManager> mTxnMgr; > nsWeakPtr mPlaceHolderTxn; // weak reference to placeholder for begin/end batch purposes > nsIAtom *mPlaceHolderName; // name of placeholder transaction >Index: libeditor/html/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/html/Makefile.in,v >retrieving revision 1.17 >diff -u -w -r1.17 Makefile.in >--- libeditor/html/Makefile.in 15 Dec 2004 04:06:57 -0000 1.17 >+++ libeditor/html/Makefile.in 15 Jan 2005 01:38:59 -0000 >@@ -54,6 +54,7 @@ > unicharutil \ > content \ > txmgr \ >+ txtsvc \ > htmlparser \ > necko \ > pref \ >Index: libeditor/html/nsHTMLEditRules.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp,v >retrieving revision 1.317 >diff -u -w -r1.317 nsHTMLEditRules.cpp >--- libeditor/html/nsHTMLEditRules.cpp 12 Jan 2005 19:11:47 -0000 1.317 >+++ libeditor/html/nsHTMLEditRules.cpp 15 Jan 2005 01:39:01 -0000 >@@ -22,6 +22,7 @@ > * Contributor(s): > * Pierre Phaneuf <pp@ludusdesign.com> > * Daniel Glazman <glazman@netscape.com> >+ * Neil Deakin <neil@mozdevgroup.com> > * > * Alternatively, the contents of this file may be used under the terms of > * either of the GNU General Public License Version 2 or later (the "GPL"), >@@ -425,13 +426,17 @@ > nsresult res = mHTMLEditor->GetSelection(getter_AddRefs(selection)); > if (NS_FAILED(res)) return res; > >+ nsCOMPtr<nsIDOMNode> rangeStartParent, rangeEndParent; >+ PRInt32 rangeStartOffset = 0, rangeEndOffset = 0; >+ Spaces on blank line. > // do we have a real range to act on? > PRBool bDamagedRange = PR_FALSE; > if (mDocChangeRange) > { >- nsCOMPtr<nsIDOMNode> rangeStartParent, rangeEndParent; > mDocChangeRange->GetStartContainer(getter_AddRefs(rangeStartParent)); > mDocChangeRange->GetEndContainer(getter_AddRefs(rangeEndParent)); >+ mDocChangeRange->GetStartOffset(&rangeStartOffset); >+ mDocChangeRange->GetEndOffset(&rangeEndOffset); > if (rangeStartParent && rangeEndParent) > bDamagedRange = PR_TRUE; > } >@@ -540,6 +545,12 @@ > } > } > >+ res = mHTMLEditor->HandleRealTimeSpellCheck(action,selection, >+ mRangeItem.startNode,mRangeItem.startOffset, >+ rangeStartParent,rangeStartOffset, >+ rangeEndParent,rangeEndOffset); Please add a whitespace after commas for readability. >+ if (NS_FAILED(res)) return res; Two lines preferred. >+ > // detect empty doc > res = CreateBogusNodeIfNeeded(selection); > >Index: libeditor/html/nsHTMLEditor.h >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/html/nsHTMLEditor.h,v >retrieving revision 1.218 >diff -u -w -r1.218 nsHTMLEditor.h >--- libeditor/html/nsHTMLEditor.h 12 Jan 2005 19:11:47 -0000 1.218 >+++ libeditor/html/nsHTMLEditor.h 15 Jan 2005 01:39:01 -0000 >@@ -48,6 +48,7 @@ > #include "nsITableEditor.h" > #include "nsIEditorMailSupport.h" > #include "nsIEditorStyleSheets.h" >+#include "nsITextServicesDocument.h" > > #include "nsEditor.h" > #include "nsIDOMElement.h" >@@ -788,6 +789,9 @@ > // an array for holding default style settings > nsVoidArray mDefaultStyles; > >+ // for real-time spelling >+ nsCOMPtr<nsITextServicesDocument> mTextServices; >+ > // Maintain a static parser service ... > static nsIParserService* sParserService; > >Index: libeditor/text/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/text/Makefile.in,v >retrieving revision 1.14 >diff -u -w -r1.14 Makefile.in >--- libeditor/text/Makefile.in 15 Dec 2004 04:06:57 -0000 1.14 >+++ libeditor/text/Makefile.in 15 Jan 2005 01:39:01 -0000 >@@ -53,6 +53,7 @@ > layout \ > content \ > txmgr \ >+ txtsvc \ > htmlparser \ > necko \ > pref \ >Index: libeditor/text/nsTextEditRules.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/text/nsTextEditRules.cpp,v >retrieving revision 1.190 >diff -u -w -r1.190 nsTextEditRules.cpp >--- libeditor/text/nsTextEditRules.cpp 31 May 2004 12:15:21 -0000 1.190 >+++ libeditor/text/nsTextEditRules.cpp 15 Jan 2005 01:39:02 -0000 >@@ -49,7 +49,9 @@ > #include "nsIDOMNodeList.h" > #include "nsISelection.h" > #include "nsISelectionPrivate.h" >+#include "nsISelectionController.h" > #include "nsIDOMRange.h" >+#include "nsIDOMNSRange.h" > #include "nsIDOMCharacterData.h" > #include "nsIContent.h" > #include "nsIContentIterator.h" >@@ -58,6 +60,8 @@ > #include "nsIPrefBranch.h" > #include "nsIPrefService.h" > #include "nsUnicharUtils.h" >+#include "nsIWordBreakerFactory.h" >+#include "nsLWBrkCIID.h" > > // for IBMBIDI > #include "nsIPresShell.h" >@@ -196,6 +200,14 @@ > nsAutoLockRulesSniffing lockIt(this); > mDidExplicitlySetInterline = PR_FALSE; > >+ // get the selection and cache the position before editing >+ nsCOMPtr<nsISelection> selection; >+ nsresult res = mEditor->GetSelection(getter_AddRefs(selection)); >+ if (NS_FAILED(res)) return res; 2 lines. >+ >+ selection->GetAnchorNode(getter_AddRefs(mCachedSelectionNode)); >+ selection->GetAnchorOffset(&mCachedSelectionOffset); >+ Spaces on blank line; > if (!mActionNesting) > { > // let rules remember the top level action >@@ -221,6 +233,11 @@ > res = mEditor->GetSelection(getter_AddRefs(selection)); > if (NS_FAILED(res)) return res; > >+ res = mEditor->HandleRealTimeSpellCheck(action,selection, >+ mCachedSelectionNode,mCachedSelectionOffset, >+ nsnull,0,nsnull,0); Whitespace after comma. >+ if (NS_FAILED(res)) return res; 2 lines. >+ > // detect empty doc > res = CreateBogusNodeIfNeeded(selection); > if (NS_FAILED(res)) return res; >Index: libeditor/text/nsTextEditRules.h >=================================================================== >RCS file: /cvsroot/mozilla/editor/libeditor/text/nsTextEditRules.h,v >retrieving revision 1.75 >diff -u -w -r1.75 nsTextEditRules.h >--- libeditor/text/nsTextEditRules.h 18 Apr 2004 14:19:36 -0000 1.75 >+++ libeditor/text/nsTextEditRules.h 15 Jan 2005 01:39:02 -0000 >@@ -203,6 +203,8 @@ > PRInt32 mPasswordIMEIndex; > nsCOMPtr<nsIDOMNode> mBogusNode; // magic node acts as placeholder in empty doc > nsCOMPtr<nsIDOMNode> mBody; // cached root node >+ nsCOMPtr<nsIDOMNode> mCachedSelectionNode; // cached selected node >+ PRInt32 mCachedSelectionOffset; // cached selected offset > PRUint32 mFlags; > PRUint32 mActionNesting; > PRPackedBool mLockRulesSniffing; >Index: txtsvc/public/Makefile.in >=================================================================== >RCS file: /cvsroot/mozilla/editor/txtsvc/public/Makefile.in,v >retrieving revision 1.12 >diff -u -w -r1.12 Makefile.in >--- txtsvc/public/Makefile.in 18 Apr 2004 14:19:38 -0000 1.12 >+++ txtsvc/public/Makefile.in 15 Jan 2005 01:39:02 -0000 >@@ -54,6 +54,7 @@ > > XPIDLSRCS = \ > nsITextServicesFilter.idl \ >+ nsIInlineSpellChecker.idl \ > $(NULL) > > include $(topsrcdir)/config/rules.mk >Index: txtsvc/public/nsIInlineSpellChecker.idl >=================================================================== >RCS file: txtsvc/public/nsIInlineSpellChecker.idl >diff -N txtsvc/public/nsIInlineSpellChecker.idl >--- /dev/null 1 Jan 1970 00:00:00 -0000 >+++ txtsvc/public/nsIInlineSpellChecker.idl 15 Jan 2005 01:39:02 -0000 >@@ -0,0 +1,77 @@ >+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ >+/* ***** BEGIN LICENSE BLOCK ***** >+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1 >+ * >+ * The contents of this file are subject to the Mozilla Public License Version >+ * 1.1 (the "License"); you may not use this file except in compliance with >+ * the License. You may obtain a copy of the License at >+ * http://www.mozilla.org/MPL/ >+ * >+ * Software distributed under the License is distributed on an "AS IS" basis, >+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+ * for the specific language governing rights and limitations under the >+ * License. >+ * >+ * The Original Code is Real-time Spellchecking >+ * >+ * The Initial Developer of the Original Code is Neil Deakin. >+ * Portions created by the Initial Developer are Copyright (C) 2004 >+ * the Initial Developer. All Rights Reserved. >+ * >+ * Contributor(s): Neil Deakin (enndeakin@sympatico.ca) >+ * Scott MacGregor (mscott@mozilla.org) "Contributor(s):" should be alone on its line, contributors' list beginning at next line with 2 chars indentation. >+ * >+ * Alternatively, the contents of this file may be used under the terms of >+ * either the GNU General Public License Version 2 or later (the "GPL"), or >+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+ * in which case the provisions of the GPL or the LGPL are applicable instead >+ * of those above. If you wish to allow use of your version of this file only >+ * under the terms of either the GPL or the LGPL, and not to allow others to >+ * use your version of this file under the terms of the MPL, indicate your >+ * decision by deleting the provisions above and replace them with the notice >+ * and other provisions required by the GPL or the LGPL. If you do not delete >+ * the provisions above, a recipient may use your version of this file under >+ * the terms of any one of the MPL, the GPL or the LGPL. >+ * >+ * ***** END LICENSE BLOCK ***** */ >+ >+#include "nsISupports.idl" >+#include "domstubs.idl" >+ >+interface nsISelection; >+interface nsIEditor; >+interface nsIEditorSpellCheck; >+ >+[scriptable, uuid(f5d1ec9e-4d30-11d8-8053-da0cc7df1f20)] >+ >+interface nsIInlineSpellChecker : nsISupports >+{ >+ readonly attribute nsIEditorSpellCheck spellChecker; >+ >+ [noscript] void init(in nsIEditor editor); Worried me a bit because the inlinespellchecker keeps a ref to the editor that itself keeps a ref to the inlinespellchecker, but the former being a weak reference, there should be no leak. Please double-check. >+ [noscript] void destroy(); >+ >+ attribute boolean enableRealTimeSpell; >+ >+ void spellCheckAfterEditorChange(in PRInt32 action, >+ in nsISelection aSelection, >+ in nsIDOMNode previousSelectedNode, Some parameters follow the aSomething rule, some don't. Comment valid for all other IDLs in the patch. >+ in PRInt32 previousSelectedOffset, >+ in nsIDOMNode aStartNode, >+ in PRInt32 aStartOffset, >+ in nsIDOMNode aEndNode, >+ in PRInt32 aEndOffset); >+ >+ void spellCheckRange(in nsIDOMRange aSelection); >+ >+ nsIDOMRange getMispelledWord(in nsIDOMNode aNode, in PRInt32 aOffset); >+ void replaceWord(in nsIDOMNode aNode, in PRInt32 aOffset, in AString newword); >+ void addWordToDictionary(in AString word); >+ void ignoreWord(in AString word); >+}; >+ >+%{C++ >+ >+#define MOZ_INLINESPELLCHECKER_CONTRACTID "@mozilla.org/spellchecker-inline;1" >+ >+%} >Index: txtsvc/public/nsITextServicesDocument.h >=================================================================== >RCS file: /cvsroot/mozilla/editor/txtsvc/public/nsITextServicesDocument.h,v >retrieving revision 1.13 >diff -u -w -r1.13 nsITextServicesDocument.h >--- txtsvc/public/nsITextServicesDocument.h 18 Apr 2004 14:19:38 -0000 1.13 >+++ txtsvc/public/nsITextServicesDocument.h 15 Jan 2005 01:39:02 -0000 >@@ -266,6 +266,14 @@ > */ > > NS_IMETHOD SetDisplayStyle(TSDDisplayStyle aStyle) = 0; >+ >+ /** >+ * Returns the DOM range for a given offset and length >+ * @param aOffset offset into string returned by GetCurrentTextBlock(). >+ * @param aLength number characters selected. >+ * @param aDOMRange the DOM range that represents the offset and length >+ */ If you start adding javadoc comments, please don't forget the @return line. >+ NS_IMETHOD GetDOMRangeFor(PRInt32 aOffset, PRInt32 aLength, nsIDOMRange** aRange) = 0; > }; > > #endif // nsITextServicesDocument_h__ >Index: txtsvc/src/nsTextServicesDocument.cpp >=================================================================== >RCS file: /cvsroot/mozilla/editor/txtsvc/src/nsTextServicesDocument.cpp,v >retrieving revision 1.57 >diff -u -w -r1.57 nsTextServicesDocument.cpp >--- txtsvc/src/nsTextServicesDocument.cpp 1 Nov 2004 18:50:10 -0000 1.57 >+++ txtsvc/src/nsTextServicesDocument.cpp 15 Jan 2005 01:39:03 -0000 >@@ -21,6 +21,7 @@ > * > * Contributor(s): > * Pierre Phaneuf <pp@ludusdesign.com> >+ * Neil Deakin <neil@mozdevgroup.com> > * > * Alternatively, the contents of this file may be used under the terms of > * either of the GNU General Public License Version 2 or later (the "GPL"), >@@ -321,8 +322,6 @@ > { > NS_ENSURE_ARG_POINTER(aDOMRange); > NS_ENSURE_TRUE(mDOMDocument, NS_ERROR_FAILURE); >- NS_ENSURE_TRUE(mEditor, NS_ERROR_FAILURE); >- NS_ENSURE_TRUE(mNotifier, NS_ERROR_FAILURE); > > LOCK_DOC(this); > >@@ -548,10 +547,10 @@ > // Now adjust the range so that it uses our new > // end points. > >- result = aRange->SetStart(rngStartNode, rngStartOffset); >+ result = aRange->SetEnd(rngEndNode, rngEndOffset); > NS_ENSURE_SUCCESS(result, result); > >- return aRange->SetEnd(rngEndNode, rngEndOffset); >+ return aRange->SetStart(rngStartNode, rngStartOffset); > } > > NS_IMETHODIMP >@@ -2425,18 +2424,74 @@ > return NS_ERROR_NOT_IMPLEMENTED; > } > >+NS_IMETHODIMP >+nsTextServicesDocument::GetDOMRangeFor(PRInt32 aOffset, PRInt32 aLength, nsIDOMRange** aRange) >+{ >+ if (!mSelCon || aOffset < 0 || aLength < 0) >+ return NS_ERROR_FAILURE; >+ >+ nsIDOMNode *sNode = 0, *eNode = 0; >+ PRInt32 i, sOffset = 0, eOffset = 0; >+ OffsetEntry *entry; >+ >+ // Find the start >+ for (i = 0; !sNode && i < mOffsetTable.Count(); i++) >+ { >+ entry = (OffsetEntry *)mOffsetTable[i]; >+ if (entry->mIsValid) >+ { >+ if (entry->mIsInsertedText) >+ { >+ if (entry->mStrOffset == aOffset) >+ { >+ sNode = entry->mNode; >+ sOffset = entry->mNodeOffset + entry->mLength; >+ } >+ } >+ else if (aOffset >= entry->mStrOffset && aOffset <= entry->mStrOffset + entry->mLength) >+ { >+ sNode = entry->mNode; >+ sOffset = entry->mNodeOffset + aOffset - entry->mStrOffset; >+ } >+ } >+ } >+ >+ if (!sNode) >+ return NS_ERROR_FAILURE; >+ >+ // Now find the end >+ PRInt32 endOffset = aOffset + aLength; >+ >+ for (i = mOffsetTable.Count() - 1; !eNode && i >= 0; i--) >+ { >+ entry = (OffsetEntry *)mOffsetTable[i]; >+ >+ if (entry->mIsValid) >+ { >+ if (entry->mIsInsertedText) >+ { >+ if (entry->mStrOffset == eOffset) >+ { >+ eNode = entry->mNode; >+ eOffset = entry->mNodeOffset + entry->mLength; >+ } >+ } >+ else if (endOffset >= entry->mStrOffset && endOffset <= entry->mStrOffset + entry->mLength) >+ { >+ eNode = entry->mNode; >+ eOffset = entry->mNodeOffset + endOffset - entry->mStrOffset; >+ } >+ } >+ } >+ >+ return CreateRange(sNode, sOffset, eNode, eOffset, aRange); >+} >+ > nsresult > nsTextServicesDocument::InsertNode(nsIDOMNode *aNode, > nsIDOMNode *aParent, > PRInt32 aPosition) > { >- //**** KDEBUG **** >- // printf("** InsertNode: 0x%.8x 0x%.8x %d\n", aNode, aParent, aPosition); >- // fflush(stdout); >- //**** KDEBUG **** >- >- NS_ASSERTION(0, "InsertNode called, offset tables might be out of sync."); >- > return NS_OK; > } > >@@ -4579,6 +4634,13 @@ > return result; > } > >+// Spellchecker code has this. See bug 211343 >+#ifdef XP_MAC >+#define IS_NBSP_CHAR(c) (((unsigned char)0xca)==(c)) >+#else >+#define IS_NBSP_CHAR(c) (((unsigned char)0xa0)==(c)) >+#endif >+ > nsresult > nsTextServicesDocument::FindWordBounds(nsVoidArray *aOffsetTable, > nsString *aBlockStr, >@@ -4628,6 +4690,21 @@ > &beginWord, &endWord); > NS_ENSURE_SUCCESS(result, result); > >+ PRBool hasBeginQuote = PR_FALSE; >+ if (str[beginWord] == '\''){ Please add whitespace between ) and { or make a new line with the curly brace. Why do you test only single quote ' and not " or even « and other language-specific quotation marks ? >+ hasBeginQuote = PR_TRUE; >+ beginWord++; >+ } >+ if (hasBeginQuote && str[endWord - 1] == '\'') endWord--; 2 lines please. >+ >+ // Strip out the NBSPs at the ends >+ while ((beginWord <= endWord) && (IS_NBSP_CHAR(str[beginWord]))) beginWord++; 2 lines please. >+ if (str[endWord] == (unsigned char)0x20){ ) { >+ PRUint32 realEndWord = endWord - 1; >+ while ((realEndWord > beginWord) && (IS_NBSP_CHAR(str[realEndWord]))) realEndWord--; 2 lines please. >+ if (realEndWord < endWord - 1) endWord = realEndWord + 1; 2 lines please. >+ } >+ > // Now that we have the string offsets for the beginning > // and end of the word, run through the offset table and > // convert them back into dom points. >Index: txtsvc/src/nsTextServicesDocument.h >=================================================================== >RCS file: /cvsroot/mozilla/editor/txtsvc/src/nsTextServicesDocument.h,v >retrieving revision 1.19 >diff -u -w -r1.19 nsTextServicesDocument.h >--- txtsvc/src/nsTextServicesDocument.h 10 Oct 2004 03:30:42 -0000 1.19 >+++ txtsvc/src/nsTextServicesDocument.h 15 Jan 2005 01:39:03 -0000 >@@ -159,6 +159,7 @@ > NS_IMETHOD DeleteSelection(); > NS_IMETHOD InsertText(const nsString *aText); > NS_IMETHOD SetDisplayStyle(TSDDisplayStyle aStyle); >+ NS_IMETHOD GetDOMRangeFor(PRInt32 aOffset, PRInt32 aLength, nsIDOMRange** aRange); > > /* nsIEditActionListener method implementations. */ > nsresult InsertNode(nsIDOMNode * aNode, >Index: ui/jar.mn >=================================================================== >RCS file: /cvsroot/mozilla/editor/ui/jar.mn,v >retrieving revision 1.14 >diff -u -w -r1.14 jar.mn >--- ui/jar.mn 7 Jan 2005 01:31:43 -0000 1.14 >+++ ui/jar.mn 15 Jan 2005 01:39:03 -0000 >@@ -25,6 +25,7 @@ > content/editor/pref-composer.xul (composer/content/pref-composer.xul) > content/editor/pref-publish.xul (composer/content/pref-publish.xul) > content/editor/editorSmileyOverlay.xul (composer/content/editorSmileyOverlay.xul) >+ content/editor/editorInlineSpellCheck.js (composer/content/editorInlineSpellCheck.js) > content/editor/editorPrefsOverlay.xul (composer/content/editorPrefsOverlay.xul) > content/editor/editorNavigatorOverlay.xul (composer/content/editorNavigatorOverlay.xul) > content/editor/editorMailOverlay.xul (composer/content/editorMailOverlay.xul) >Index: ui/composer/content/editorInlineSpellCheck.js >=================================================================== >RCS file: ui/composer/content/editorInlineSpellCheck.js >diff -N ui/composer/content/editorInlineSpellCheck.js >--- /dev/null 1 Jan 1970 00:00:00 -0000 >+++ ui/composer/content/editorInlineSpellCheck.js 15 Jan 2005 01:39:03 -0000 >@@ -0,0 +1,174 @@ >+/* -*- Mode: Java; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4 -*- */ >+/* ***** BEGIN LICENSE BLOCK ***** >+ * Version: NPL 1.1/GPL 2.0/LGPL 2.1 >+ * >+ * The contents of this file are subject to the Netscape Public License >+ * Version 1.1 (the "License"); you may not use this file except in >+ * compliance with the License. You may obtain a copy of the License at >+ * http://www.mozilla.org/NPL/ >+ * >+ * Software distributed under the License is distributed on an "AS IS" basis, >+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License >+ * for the specific language governing rights and limitations under the >+ * License. >+ * >+ * The Original Code is Inline Spellchecker >+ * >+ * The Initial Developer of the Original Code is Mozdev Group, Inc. >+ * Portions created by the Initial Developer are Copyright (C) 2004 >+ * the Initial Developer. All Rights Reserved. >+ * >+ * Contributor(s): Neil Deakin (neil@mozdevgroup.com) >+ * Scott MacGregor (mscott@mozilla.org) See above. >+ * >+ * Alternatively, the contents of this file may be used under the terms of >+ * either the GNU General Public License Version 2 or later (the "GPL"), or >+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"), >+ * in which case the provisions of the GPL or the LGPL are applicable instead >+ * of those above. If you wish to allow use of your version of this file only >+ * under the terms of either the GPL or the LGPL, and not to allow others to >+ * use your version of this file under the terms of the NPL, indicate your >+ * decision by deleting the provisions above and replace them with the notice >+ * and other provisions required by the GPL or the LGPL. If you do not delete >+ * the provisions above, a recipient may use your version of this file under >+ * the terms of any one of the NPL, the GPL or the LGPL. >+ * >+ * ***** END LICENSE BLOCK ***** */ >+ >+const kSpellMaxNumSuggestions = 7; // Maximum number of suggested words to fill into the context menu. Most >+ // applications (like Open Office) set this value to 15. Could/Should this be a user pref ? >+ >+const kSpellNoMispelling = -1; >+const kSpellNoSuggestionsFound = 0; >+ >+var InlineSpellChecker = >+{ >+ editor : null, >+ inlineSpellChecker: null, >+ >+ Init : function (editor, enable) >+ { >+ this.editor = editor; >+ this.inlineSpellChecker = editor.inlineSpellChecker; >+ this.inlineSpellChecker.enableRealTimeSpell = enable; >+ }, >+ >+ checkDocument : function(doc) >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return null; 2 lines. >+ >+ var range = doc.createRange(); >+ range.selectNodeContents(doc.body); >+ this.inlineSpellChecker.spellCheckRange(range); >+ }, >+ >+ getMispelledWord : function() >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return null; 2 lines. >+ >+ var selection = this.editor.selection; >+ return this.inlineSpellChecker.getMispelledWord(selection.anchorNode,selection.anchorOffset); Whitespace after comma. >+ }, >+ >+ updateMenu : function(menuid, separatorid) >+ { >+ var word = this.getMispelledWord(); >+ >+ var suggestionsMenu = document.getElementById(menuid); >+ var suggestionsMenuSeparator = document.getElementById(separatorid); >+ if (word){ ) { >+ suggestionsMenu.removeAttribute("hidden"); >+ suggestionsMenuSeparator.removeAttribute("hidden"); >+ } else { Else on new line. >+ suggestionsMenu.setAttribute("hidden","true"); >+ suggestionsMenuSeparator.setAttribute("hidden","true"); Whitespace after comma. >+ } >+ }, >+ >+ // returns kSpellNoMispelling if there is no mispelling of the word >+ // For mispelled words, returns kSpellNoSuggestionsFound when there are no suggestions otherwise the >+ // number of suggestions is returned. >+ updateSuggestionsMenu : function (menupopup, firstNonWordMenuItem, word) >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return kSpellNoMispelling; 2 lines please. >+ >+ var child = menupopup.firstChild; >+ while (child != firstNonWordMenuItem) { >+ var next = child.nextSibling; >+ menupopup.removeChild(child); >+ child = next; >+ } >+ >+ if (!word){ ) { >+ word = this.getMispelledWord(); >+ if (!word) return kSpellNoMispelling; 2 lines please. >+ } >+ >+ var spellChecker = this.inlineSpellChecker.spellChecker; >+ if (!spellChecker) return kSpellNoMispelling; 2 lines please. >+ >+ var found = false; >+ var numSuggestedWords = 0; >+ >+ var isIncorrect = spellChecker.CheckCurrentWord(word.toString()); >+ if (isIncorrect){ ) { >+ do { >+ var suggestion = spellChecker.GetSuggestedWord(); >+ if (!suggestion) break; 2 lines please. >+ >+ found = true; This 'found' is apparently unused since you now rely only on the return value of |GetSuggestedWord()|. >+ >+ var item = document.createElement("menuitem"); >+ item.setAttribute("label",suggestion); >+ item.setAttribute("value",suggestion); Whitespace after comma. >+ menupopup.insertBefore(item, firstNonWordMenuItem); >+ numSuggestedWords++; >+ } while(numSuggestedWords < kSpellMaxNumSuggestions); Whitespace before ( please. >+ } >+ else >+ numSuggestedWords = kSpellNoMispelling; >+ >+ return numSuggestedWords; >+ }, >+ >+ selectSuggestion : function (newword, node, offset) >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return null; 2 lines please. >+ >+ if (!node){ ) { >+ var selection = this.editor.selection; >+ node = selection.anchorNode; >+ offset = selection.anchorOffset; >+ } >+ >+ this.inlineSpellChecker.replaceWord(node,offset,newword); >+ }, >+ >+ addToDictionary : function (node, offset) >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return null; 2 lines please. >+ >+ if (!node){ ) { >+ var selection = this.editor.selection; >+ node = selection.anchorNode; >+ offset = selection.anchorOffset; >+ } >+ >+ var word = this.inlineSpellChecker.getMispelledWord(node,offset); Whitespace after comma. >+ if (word) this.inlineSpellChecker.addWordToDictionary(word); 2 lines please. >+ }, >+ >+ ignoreWord : function (node, offset) >+ { >+ if (!this.inlineSpellChecker || !this.inlineSpellChecker.enableRealTimeSpell) return null; 2 lines please. >+ >+ if (!node){ ) { >+ var selection = this.editor.selection; >+ node = selection.anchorNode; >+ offset = selection.anchorOffset; >+ } >+ >+ var word = this.inlineSpellChecker.getMispelledWord(node,offset); Whitespace after comma. >+ if (word) this.inlineSpellChecker.ignoreWord(word); 2 lines please. >+ } >+}
Attachment #171312 - Flags: review?(daniel) → review-
new patch to address daniel's review comments. (Thanks for that quick turn around!) Daniel to answer two of your questions: 1) I confirmed there wasn't a cylce leak 2) I may end up making the # of words in the menu a configureable pref down the line.
Attachment #171312 - Attachment is obsolete: true
Comment on attachment 171670 [details] [diff] [review] updated patch based on review comments new patch to address daniel's review comments. (Thanks for that quick turn around!) Daniel to answer two of your questions: 1) I confirmed there wasn't a cylce leak 2) I may end up making the # of words in the menu a configureable pref down the line.
Attachment #171670 - Flags: review?(daniel)
(In reply to comment #12) > Daniel to answer two of your questions: > > 1) I confirmed there wasn't a cylce leak > 2) I may end up making the # of words in the menu a configureable pref down the > line. Scott, I still don't understand the quotes test. Why only single-quotes ?
fixes lots of edge cases and issues with deleting text in mispelled words.
Attachment #171311 - Attachment is obsolete: true
Attachment #171670 - Attachment is obsolete: true
Attachment #171670 - Flags: review?(daniel)
Comment on attachment 171811 [details] [diff] [review] updated editor patch, removes unnecessary code in nsTextServicesDocument. Thanks for making me revist the change in nsTextServicesDocument::FindWordBounds with the check for single quotes. That part of the patch was from Neil's original code. I think he was trying to work around a problem with the I18n word breaker later on in the spell check code when it trys to handle apostrophes. Where it does handle " correctly. I was able to safely remove this code altogether as I'm working on changes further down the pipeline to make the word breaker handle apostrophes.
Attachment #171811 - Flags: review?(daniel)
Comment on attachment 171811 [details] [diff] [review] updated editor patch, removes unnecessary code in nsTextServicesDocument. cool:-) r=daniel@glazman.org
Attachment #171811 - Flags: review?(daniel) → review+
Comment on attachment 171310 [details] [diff] [review] more layout changes >+ //ITS OK TO CAST HERE THE RESULT WE USE WILLNOT DO BAD CONVERSION Why are you yelling? And what does this mean, anyway?
This latest update to the spell check directory adds support for using the arrow keys to navigate to another word (we now spell check the old word). For using the mouse to move to another word (we now spell check the old word). And initial support for pasting a group of words into the compose window. We now spell check the new pasted text. This patch makes the inline spell checker instance register as a mouse and key dom even listener so we can track all of these events.
Attachment #171804 - Attachment is obsolete: true
Attached patch latest round of changes (obsolete) — Splinter Review
Fixes include: 1) proper support for copy and paste 2) several code optimizations 3) mispell two words in a row in a bullet item then hit return, before we would undo t spelling underline on the first word 4) we now properly recognize URLS of the form http://foo or mailto:foo. When pasting we never underling these words at all. When typing, we'll underline the scheme name due to a hard to fix bug. 5) mispellings for words with apostrophes like didnn't now get underlined instead of treating the apostrophe as a word separator. In general we are much smarter about guessing what a word is. 6) Add word to the dictionary issues should be fixed (with words that had multiple spaces between them.
Attachment #172154 - Attachment is obsolete: true
Comment on attachment 171310 [details] [diff] [review] more layout changes nsTextFrame.cpp: >@@ -3331,6 +3363,7 @@ > { > char *currenttext = iter.CurrentTextCStrPtr(); > PRUint32 currentlength= iter.CurrentLength(); >+ > //TextStyle &currentStyle = iter.CurrentStyle(); Seemed better the way it was... >@@ -3367,6 +3400,12 @@ > aRenderingContext.SetColor(nsCSSRendering::TransformColor(aTextStyle.mColor->mColor,canDarkenColor)); > aRenderingContext.DrawString(text, PRUint32(textLength), dx, dy + mAscent); > } >+ } >+ else if (!isPaginated) >+ { >+ aRenderingContext.SetColor(nsCSSRendering::TransformColor(aTextStyle.mColor->mColor,canDarkenColor)); >+ aRenderingContext.DrawString(text, PRUint32(textLength), dx, dy + mAscent); >+ } > PaintTextDecorations(aRenderingContext, aStyleContext, aPresContext, > aTextStyle, dx, dy, width, > unicodePaintBuffer.mBuffer, This looks like a merging error and should probably be undone.
Comment on attachment 171310 [details] [diff] [review] more layout changes sr=dbaron given comment 21. And actually ignore comment 18 if you want -- I see that that comment was there before. But please make sure that you check in the whitespace changes that the "diff -w" ignores.
Attachment #171310 - Flags: superreview?(dbaron) → superreview+
updated patch with David Baron's review comments. Moving forward his review.
Attachment #171310 - Attachment is obsolete: true
Attachment #172977 - Flags: superreview+
Attached patch updated editor patch (obsolete) — Splinter Review
Just re-freshing the editor changes against the trunk again. There was a merge conflict in the diff with an IDL IID change. Carrying forward glazman's review and module owner approval. Asking Bienvenu for an sr.
Attachment #171811 - Attachment is obsolete: true
Attachment #172983 - Flags: superreview?(bienvenu)
Attachment #172983 - Flags: review+
Attachment #172983 - Flags: superreview?(bienvenu) → superreview+
The layout changes were actually part of bug 181105. Also, bug 220157 should be fixed by them as well.
I made a minor change to nsTextDocumentServices to use a weak reference to the editor object instead of a strong reference to prevent a circle. Carrying forward the reviews.
Attachment #172983 - Attachment is obsolete: true
Attachment #173104 - Flags: superreview+
Attachment #173104 - Flags: review+
Attached patch updated spell checker patch (obsolete) — Splinter Review
removed more dead code, add more comments about the purpose of the remaining methods, added more argument null pointer checks. This patch is now ready to start the review process. Future issues with the behavior (I'm sure there will be many) can be handled in separate bugs.
Attachment #173113 - Flags: superreview?
Comment on attachment 173113 [details] [diff] [review] updated spell checker patch It helps if I actually specify a reviewer name in the super review box....
Attachment #173113 - Flags: superreview? → superreview?(bienvenu)
Comment on attachment 173113 [details] [diff] [review] updated spell checker patch a bunch of nits which you can ignore if you want... + NS_ENSURE_ARG_POINTER(aEnabled); + *aEnabled = mSpellCheck ? PR_TRUE : PR_FALSE; + return NS_OK; this could be *aEnabled = mSpellCheck != nsnull; + // spell check the text at the previous caret position, but no need to + // check the quoted text itself. why don't we need to spell check the quoted text? Is that what other in line spell checkers do? + case kOpUndo: + case kOpRedo: + // XXX handling these cases is quite hard -- this isn't quite right you might say what's not quite right. I guess we need to spell check in case undoing/redoing (re)introduces misspelling(s). naming convention style nit: + nsAutoString parenttagname; + parentelement->GetTagName(parenttagname); + + if (parenttagname.Equals(NS_LITERAL_STRING("blockquote"), nsCaseInsensitiveStringComparator())) parentTagName similarly for + const PRUnichar *textchrs = textblock.get(); + PRInt32 textlength = textblock.Length(); and there are a few others, like currentrange, checkrange, etc... + nsCOMPtr<nsISelection> spellCheckSelection; + rv = selcon->GetSelection(nsISelectionController::SELECTION_SPELLCHECK, aSpellCheckSelection); + return rv; can just return selcon->GetSelection... + selection->GetFocusOffset(&mCurrentSelectionOffset); + + return NS_OK; maybe can just return selection->GetFocusOffset(), though maybe you just want to always return NS_OK... + PRBool done, isMispelled; should be isMisspelled :-)
Attachment #173113 - Flags: superreview?(bienvenu) → superreview+
Attachment #172703 - Attachment is obsolete: true
Attachment #173113 - Attachment is obsolete: true
Attachment #173197 - Flags: superreview+
all three patches have been checked in. Marking fixed. I'll treat inline spell check behavior issues in individual bugs going forward.
Status: ASSIGNED → RESOLVED
Closed: 20 years ago
Resolution: --- → FIXED
I just tested 1.8b1 and this version doesn't skip quoted text during spellcheck. So I tested the oldest available nightly of 15th February and the newest nightly of 1st March. In both versions the spellchecker doesn't skip the quotes. Then I tested 1.8a6. This version does still skips the quotes. Therefore I think the bug came back between the release of 1.8a6 and 15th February. The patch for this bug was checked in during this period. There were not much other checkins in editor. Is it possible, that the patch effected the skipping of quoted text?
Someone tested some older Seamonkey-Tinderbox-Creature-Zip-Builds for me. Result: 2005020111 -- quoted text is not checked 2005020118 -- quoted text is checked So it appears to prove true, that this patch causes this behavior. Should I open a new bug, or will this patch be fixed here?
Blocks: inlineSpell
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: