There's some dead code to remove and some things that can be simplified.
Created attachment 381294 [details] Patch
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
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.
(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.
(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 ;-)
Created attachment 382051 [details] Patch for landing
Committed r251650: <https://trac.webkit.org/changeset/251650>