Bug 203149 – [GTK] Simplify the Input Method implementation

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Carlos Garcia Campos]

There's some dead code to remove and some things that can be simplified.

[Carlos Garcia Campos]

Created attachment 381294 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[Adrian Perez]

Comment on attachment 381294 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381294&action=review

It's definitely pleasant to see this code being simplified, thanks
a lot. The patch looks good overall, with a couple of small comments,
which you may want to address before landing :] 

> Source/WebCore/platform/PlatformKeyboardEvent.h:43
>  #if PLATFORM(GTK)

This whole #if block can be removed, because there are no more uses
of GdkEventKey left.

> Source/WebKit/UIProcess/gtk/InputMethodFilter.h:42
> +    enum class EventFakedForComposition {

You could use

  enum class EventFakedForComposition : bool { No, Yes};

as you have used elsewhere in the patch.

> Source/WebKit/UIProcess/gtk/InputMethodFilter.h:46
> +    enum class EventHandledByInputMethod {

Ditto.

[Carlos Garcia Campos]

(In reply to Adrian Perez from comment #3)
> Comment on attachment 381294 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=381294&action=review
> 
> It's definitely pleasant to see this code being simplified, thanks
> a lot. The patch looks good overall, with a couple of small comments,
> which you may want to address before landing :] 

Sure, thanks!

> > Source/WebCore/platform/PlatformKeyboardEvent.h:43
> >  #if PLATFORM(GTK)
> 
> This whole #if block can be removed, because there are no more uses
> of GdkEventKey left.

Indeed.

> > Source/WebKit/UIProcess/gtk/InputMethodFilter.h:42
> > +    enum class EventFakedForComposition {
> 
> You could use
> 
>   enum class EventFakedForComposition : bool { No, Yes};
> 
> as you have used elsewhere in the patch.
> 
> > Source/WebKit/UIProcess/gtk/InputMethodFilter.h:46
> > +    enum class EventHandledByInputMethod {
> 
> Ditto.

Yes, I didn't do that because I'm going to remove that enum in the next patch.

[Adrian Perez]

(In reply to Carlos Garcia Campos from comment #4)
> (In reply to Adrian Perez from comment #3)
> > Comment on attachment 381294 [details]
> >
> > > [...]
> > >
> > > Source/WebKit/UIProcess/gtk/InputMethodFilter.h:42
> > > +    enum class EventFakedForComposition {
> > 
> > You could use
> > 
> >   enum class EventFakedForComposition : bool { No, Yes};
> > 
> > [...]
> 
> Yes, I didn't do that because I'm going to remove that enum in the next
> patch.

That sounds even better ;-)

[Carlos Garcia Campos]

Created attachment 382051 [details]
Patch for landing

[Carlos Garcia Campos]

Committed r251650: <https://trac.webkit.org/changeset/251650>