[webkit-reviews] review granted: [Bug 32290] [GTK] can't input korean into lower all input box except adress input box in webkit gtk launcher : [Attachment 50775] Patch for this issue (rev9)

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 32290: [GTK] can't input korean into lower all input box except adress
input box in webkit gtk launcher
https://bugs.webkit.org/show_bug.cgi?id=32290

Attachment 50775: Patch for this issue (rev9)
https://bugs.webkit.org/attachment.cgi?id=50775&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
>-static void setPendingComposition(gchar* newComposition)
>+static void imContextCommitted(GtkIMContext* context, const gchar*
compositionString, EditorClient* client)
> {
>-    g_free(pendingComposition);
>-    pendingComposition = newComposition;
>-}
>+    // If this signal fires during a keydown event when we are not in the
middle
>+    // of a composition, then treat this 'commit' as a normal key event and
just
>+    // change the editable area right before the keypress event.
>+    if (client->treatContextCommitAsKeyEvent()) {
>+	  client->updatePendingComposition(compositionString);
>+	  return;
>+    }
> 
>-static void setPendingPreedit(gchar* newPreedit)
>-{
>-    g_free(pendingPreedit);
>-    pendingPreedit = newPreedit;
>-}
>+    Frame* frame =
core(client->webView())->focusController()->focusedOrMainFrame();
>+    if (!frame || !frame->editor()->canEdit())
>+	  return;

I thought we had decided to move this check to the beginning of the function?
Did you forget or have you found any problems with that?

It looks fine to me other than that, so r=me with that change unless you have a
good reason not to do it.