882879 - Spell checking regression in contentEditable elements being focused by script

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[(no longer active)]

Attachment: Testcase

See the test case.  Focusing the element used to trigger spell checking but it doesn't any more.

adw, do you mind taking this please?  Thanks!

Comment 1

[(no longer active)]

I found this out when I was trying to check in the test case patch to bug 674927.  We should probably check that in along the fix to this bug.

Comment 2

[Drew Willcoxon :adw]

Interesting, if you modify the test case to focus the contenteditable after page load, spell check is properly triggered.

Comment 3

[Drew Willcoxon :adw]

06/06 nightly good, 06/07 nightly bad:

http://hg.mozilla.org/mozilla-central/pushloghtml?startdate=2013-06-06&enddate=2013-06-07

Which includes my patches from bug 856270.

Comment 4

[Drew Willcoxon :adw]

Attachment: queue UpdateCurrentDictionary while init is pending

Here's what used to happen before bug 856270:

1. Sometime early before page load (when the contenteditable is created?),
   mozInlineSpellChecker::SetEnableRealTimeSpell(aEnabled=true)
2. nsEditorSpellCheck::InitSpellChecker on mozInlineSpellChecker's mSpellCheck
3. nsEditorSpellCheck::UpdateCurrentDictionary, but this fails because:

> WARNING: NS_ENSURE_TRUE(rootContent) failed: file /Users/adw/mi/editor/composer/src/nsEditorSpellCheck.cpp

   (rootContent is the return value of htmlEditor->GetActiveEditingHost().
   It's null because GetActiveEditingHost looks at the focused node, which
   for some reason here is the body, which is not editable.)

4. At this point, mSpellCheck has no dictionary.  But then:
5. nsEditor::OnFocus when the contenteditable is focused
6. mozInlineSpellChecker::UpdateCurrentDictionary, which finally sets a
   dictionary
7. And then a spell check, which is successful because mSpellCheck has a
   dictionary.

After bug 856270:

1. Sometime early before page load (when the contenteditable is created?),
   mozInlineSpellChecker::SetEnableRealTimeSpell(aEnabled=true)
2. nsEditorSpellCheck::InitSpellChecker on mozInlineSpellChecker's
   mPendingSpellCheck, while mSpellCheck remains null
3. nsEditorSpellCheck::UpdateCurrentDictionary, but this fails for the same
   reason as above
4. At this point, mozInlineSpellChecker's mPendingSpellCheck has no
   dictionary, and mSpellCheck is null
5. nsEditor::OnFocus when the contenteditable is focused
6. mozInlineSpellChecker::UpdateCurrentDictionary, but this bails early
   because mSpellCheck is null because nsEditorSpellCheck::InitSpellChecker's
   callback hasn't been called yet
7. nsEditorSpellCheck::InitSpellChecker's callback is called
8. A spell check, but mSpellCheck has no dictionary, so there's effectively
   no spell check at all.

If you can follow that, the problem is step 6 in the latter series of steps.  There's no reason I can see that UpdateCurrentDictionary couldn't use mPendingSpellCheck if it's nonnull.  The dictionary update will just be queued after the pending initialization.

https://tbpl.mozilla.org/?tree=Try&rev=f4d25f200d23

Comment 5

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/mozilla-inbound/rev/602b497adc01

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/602b497adc01