Summary: | [GTK] Simplify the Input Method implementation | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
Component: | WebKitGTK | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | annulen, aperez, berto, bugs-noreply, ews-watchlist, gustavo, gyuyoung.kim, rakuco, ryuan.choi, sergio, zan | ||||||
Priority: | P2 | Keywords: | Gtk | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2019-10-18 03:44:52 PDT
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> |