544504 - Spellcheck shouldn't be setting up whole-document ranges

Status: NEW

(Core :: Spelling checker, defect)

Description

[Boris Zbarsky [:bzbarsky]]

See bug 543494 comment 24.

Comment 1

[Boris Zbarsky [:bzbarsky]]

OK, so what happens is that nsEditorSpellCheck::InitSpellChecker always creates an nsITextServicesDocument, which sets up a document range.  In this case aEnableSelectionChecking is false, so we leave that range in place.  Then we pass the nsITextServicesDocument to mozSpellChecker::SetDocument.

Still trying to figure out whether we ever use it for anything after that.

Comment 2

[Boris Zbarsky [:bzbarsky]]

OK, so in the _inline_ spellcheck case we don't use it.  mozInlineSpellChecker handles its own chunking of the text into words and only uses nsEditorSpellCheck::CheckCurrentWordNoSuggest, which calls mozSpellChecker::CheckWord.  None of that uses the nsITextServicesDocument.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Is there a reason to not pass an extra flag to nsEditorSpellCheck::InitSpellChecker that will tell it to simply not create an nsITextServicesDocument thing at all?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Or would at least collapse its range or something....

Comment 5

[Boris Zbarsky [:bzbarsky]]

The benefit of not creating it at all is of course less code and memory use, but we may get largely there anyway with lazy editor init on textareas.  It looks like nsEditorSpellCheck assumes that it has an nsITextServicesDocument in all the methods that are not called from inline spellcheck; so if we decided to not give it one we'd want to add some null-checks.

Ehsan says he can look at this in a few weeks, hopefully.