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

[bugzilla-daemon at webkit.org]

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





--- Comment #24 from MORITA Hajime <morrita at google.com>  2011-02-13 21:51:53 PST ---
Ryosuke, thank you for reviewing again and I'm sorry for missing your last feedback.

(In reply to comment #21)
> (From update of attachment 80314 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80314&action=review
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:22447
> > +				A7151BD812F1558F005A0F64 /* TextCheckerClient.h in Headers */,
> 
> You should put this in the right lexicological order.
Fixed.

> 
> > Source/WebCore/loader/EmptyClients.h:396
> > -class EmptyEditorClient : public EditorClient {
> > +class EmptyEditorClient : public EditorClient, public TextCheckerClient {
> 
> I'm not sure if EmptyEditorClient should be both EditorClient and TextCheckerClient.  I think we should have a separate EmptyTextCheckerClient.
Defined EmptyTextCheckerClient and return it.

> 
> > Source/WebCore/page/EditorClient.h:73
> > +class TextCheckerClient;
> 
> Huh, shouldn't T come before V?
Oops. Fixed.

> 
> > Source/WebCore/page/EditorClient.h:163
> > +    virtual TextCheckerClient* textChecker() = 0;
> 
> Is it common for a client to return another client?  If not, we should probably add textCheckingClient() to Editor, and let Page return TextCheckerClient.
There isn't a common pattern. But here is a reason:
In contrast of other client objects which are owned by Page (PagecClient),
TextCheckerClient is owned by EditorClien   because of compatibility reason. 
(At this time EditorClient and TextChekerClient is a same object.)
If TextCheckerClient implementation is a standalone object, holding it on Page is reasonable.
But splitting not only interface abut also its implementation is too big to change at once.
I'd like to do it after all migration is done if it's good to do.

> we should probably add textCheckingClient() to Editor
Done this part.

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