[chromium-reviews] Fix for spell check underlines disappearing when user clicks out of the Text ...

Fix for spell check underlines disappearing when user clicks out of the Text Box. WebKit removes all spell check markers in Frame::respondToChangedSelection() if, for a text box, the ShouldSpellcheckByDefault() method we have defined in the glue returns false. The problem is, ShouldSpellcheckByDefault() is also called just after the user clicks outside the textbox. As a result, it either detects that the text box is not a focussed node, or determines that the editor is not editable, and returns false, which makes the Frame remove all the markers from the corresponding document object.

BUG=www.crbug.com/6058
TEST=Write random stuff in a text box - after the underlines appear, click outside the text box - the underlines should not disappear. Other functionalities of spellcheck, such as changing spell check language, toggling spell check language on/off, should not be affected at all.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=21892

[brettw, 2009-07-28 20:38:47]

http://codereview.chromium.org/159516/diff/1/2
File webkit/glue/editor_client_impl.cc (right):

http://codereview.chromium.org/159516/diff/1/2#newcode134
Line 134: return true;  // Not an issue since the Frame will take care of edits.
This comment doesn't make much sense to me. How about "Defaulting to allowing
spellchecking since the frame will decide not to do spellchecking if the user
can't edit."

...or something. Does that even make sense?

http://codereview.chromium.org/159516/diff/1/2#newcode139
Line 139: return (!renderer->isTextField());
Remove the extra parens around here.

[sidchat (Google), 2009-07-28 21:08:05]

Made changes as per your suggestions - please have a look again.

-Sid

http://codereview.chromium.org/159516/diff/1/2
File webkit/glue/editor_client_impl.cc (right):

http://codereview.chromium.org/159516/diff/1/2#newcode134
Line 134: return true;  // Not an issue since the Frame will take care of edits.
I updated the comment on these lines.

On 2009/07/28 20:38:47, brettw wrote:
> This comment doesn't make much sense to me. How about "Defaulting to allowing
> spellchecking since the frame will decide not to do spellchecking if the user
> can't edit."
> 
> ...or something. Does that even make sense?

http://codereview.chromium.org/159516/diff/1/2#newcode139
Line 139: return (!renderer->isTextField());
On 2009/07/28 20:38:47, brettw wrote:
> Remove the extra parens around here.

Done.

[brettw, 2009-07-28 21:13:57]

Great, LGTM