Editor class has two different path to do spellchecking. One is using both TextCheckerClient::checkSpellingOfString() and TextCheckingClient::checkGrammarOfString() separately, Another is using the unified text checker via TextCheckerClient::checkGrammarOfString(). These two passes should be embraced by clearly-named symbols, like SUPPORT_UNIFIED_TEXT_CHECKING.
Created attachment 86454 [details] Patch
Comment on attachment 86454 [details] Patch I think it's better to adopt USE(UNIFIED_TEXT_CHECKING) and USE(GRAMMER_CHECKING) macros. See wtf/Platform.h
Created attachment 86568 [details] Patch
Hi Kent-san, thanks for your suggestion! I replaced them with using USE() macro.
Comment on attachment 86568 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86568&action=review > Source/JavaScriptCore/wtf/Platform.h:1148 > +/* Text checking and editing */ > +#if !defined(BUILDING_ON_TIGER) > +#define WTF_USE_GRAMMAR_CHECKING 1 > +#endif > + > +#if PLATFORM(MAC) && !defined(BUILDING_ON_TIGER) && !defined(BUILDING_ON_LEOPARD) > +#define WTF_USE_UNIFIED_TEXT_CHECKING 1 > +#define WTF_USE_AUTOMATIC_TEXT_REPLACEMENT 1 > +#endif I think it's ok to define WTF_USE_* symbols in TextCheckerClient.h like the previous patch. We have some instances defining WTF_USE_* not in Platform.h, like WebCore/platform/graphics/ImageBuffer.h.
Hi Kent-san, thank you for the review again! > > I think it's ok to define WTF_USE_* symbols in TextCheckerClient.h like the previous patch. > We have some instances defining WTF_USE_* not in Platform.h, like WebCore/platform/graphics/ImageBuffer.h. I added WTF_USE_AUTOMATIC_TEXT_REPLACEMENT at the last update and it is used in Editor.h, which doesn't include TextCheckingClient.h. So I'd like to put them inside Platform.h. I think I can move these definitions somewhere on Bug 56085, in which I'm going to introduce a small header file which is shared between Editror.h and TextCheckingClient.h.
(In reply to comment #6) > I added WTF_USE_AUTOMATIC_TEXT_REPLACEMENT at the last update > and it is used in Editor.h, which doesn't include TextCheckingClient.h. > So I'd like to put them inside Platform.h. > > I think I can move these definitions somewhere > on Bug 56085, in which I'm going to introduce a small header file > which is shared between Editror.h and TextCheckingClient.h. Can you introduce the small header file in this change? A Platform.h change causes rebuilding almost all of source files in all of buildbots. We'd like to avoid it if possible.
Created attachment 86744 [details] Patch
> Can you introduce the small header file in this change? A Platform.h change causes rebuilding almost all of source files in all of buildbots. We'd like to avoid it if possible. Sure. Updated the patch to add new TextChecking.h.
Comment on attachment 86744 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=86744&action=review > Source/JavaScriptCore/ChangeLog:1 > +2011-03-23 MORITA Hajime <morrita@google.com> Please revert the change for JavaScriptCore/ChangeLog > Source/WebCore/WebCore.xcodeproj/project.pbxproj:22700 > + A77D0012133B0AEB00D6658C /* TextChecking.h in Headers */, nit: should be inserted to the appropriate position.
Created attachment 86749 [details] Patch
Hi Kent-san, thanks again! The update patch fixed both. (In reply to comment #10) > (From update of attachment 86744 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=86744&action=review > > > Source/JavaScriptCore/ChangeLog:1 > > +2011-03-23 MORITA Hajime <morrita@google.com> > > Please revert the change for JavaScriptCore/ChangeLog > > > Source/WebCore/WebCore.xcodeproj/project.pbxproj:22700 > > + A77D0012133B0AEB00D6658C /* TextChecking.h in Headers */, > > nit: should be inserted to the appropriate position.
Committed r81855: <http://trac.webkit.org/changeset/81855>