[Webkit-unassigned] [Bug 53213] Refactoring: Extract TextCheckerClient from EditorClient

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=53213





--- Comment #27 from MORITA Hajime <morrita at google.com>  2011-02-14 19:57:16 PST ---
Hi Ryosuke, thank you for your detailed review!

> > Source/WebCore/ChangeLog:9
> > +        spellcheck related API which is splitted 
> 
> Typo: splitted should be split. (paste tense and missing period)
Fixed. Good catch...

> 
> > Source/WebCore/accessibility/mac/AccessibilityObjectWrapper.mm:491
> > -        client->checkSpellingOfString(charData, charLength, &misspellingLocation, &misspellingLength);
> > +        client->textChecker()->checkSpellingOfString(charData, charLength, &misspellingLocation, &misspellingLength);
> 
> Given that "client" isn't used anywhere else, we should be calling Editor::textChecker here.
Fixed.

> 
> > Source/WebCore/editing/SpellChecker.cpp:109
> > -    m_client->requestCheckingOfString(this, m_requestSequence, m_requestText);
> > +    m_client->textChecker()->requestCheckingOfString(this, m_requestSequence, m_requestText);
> 
> Should SpellChecker take TextCheckerClient instead of EditorClient (or maybe both)?  It seems odd that this is the only place we access m_client but only thing we do is to obtain its TextCheckerClient.
> 
Good point. Fixed.

> > Source/WebCore/editing/TextCheckingHelper.cpp:183
> > -            m_client->checkSpellingOfString(chars, len, &misspellingLocation, &misspellingLength);
> > +            m_client->textChecker()->checkSpellingOfString(chars, len, &misspellingLocation, &misspellingLength);
> 
> Same comment about m_client used only to obtain TextCheckerClient.  But I guess calls updateSpellingUIWithGrammarString and updateSpellingUIWithMisspelledWord prevent moving forward her.
Yes. actually we/I need to consider the design around TextCheckingHelper and SpellChecker.
I want spell-checking concept out of Editor into its own class, but don't have clear idea yet.

> 
> > Source/WebCore/platform/text/TextCheckerClient.h:15
> > + * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
> > + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> 
> We should probably use other version that doesn't explicitly say Apple since the code in this file has been contributed by Apple, Nokia, & Google.
Replaced with "contributors" version.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.