Bug 33327 - [GTK] Expose the IM context via the API
Summary: [GTK] Expose the IM context via the API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 33103 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-01-07 10:07 PST by Martin Robinson
Modified: 2010-02-09 07:12 PST (History)
7 users (show)

See Also:


Attachments
Patch for this feature (2.62 KB, patch)
2010-01-07 10:16 PST, Martin Robinson
eric: review-
Details | Formatted Diff | Diff
Updated patch (fixes style issues and passes tests) (4.33 KB, patch)
2010-01-27 00:31 PST, Martin Robinson
gns: review-
mrobinson: commit-queue-
Details | Formatted Diff | Diff
Updated patch (fix documentation mistakes) (4.25 KB, patch)
2010-01-27 07:39 PST, Martin Robinson
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Robinson 2010-01-07 10:07:37 PST
Exposing the IM context of the WebView will allow embedders to recreate the IM context menu items and make possible more extensive unit tests against IM context behavior.
Comment 1 Martin Robinson 2010-01-07 10:16:35 PST
Created attachment 46062 [details]
Patch for this feature
Comment 2 WebKit Review Bot 2010-01-07 10:17:57 PST
Attachment 46062 [details] did not pass style-queue:

Failed to run "WebKitTools/Scripts/check-webkit-style" exit_code: 1
WebKit/gtk/webkit/webkitwebview.cpp:4090:  Use 0 instead of NULL.  [readability/null] [5]
WebKit/gtk/webkit/webkitwebview.h:379:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2
Comment 3 Benjamin Otte 2010-01-07 10:22:57 PST
I'd mirror the behavior of GtkEntry and GtkTextView here.
I was about to say "and they don't expose it" but they grew an "im-module" property (no accessors) in 2.16.

Would that work for your use cases?
Comment 4 Martin Robinson 2010-01-07 12:58:25 PST
From IRC: We determined that this wouldn't work for the use cases that I listed, because im-module only let's you set the type of im-module that the widget uses. We need a handle on the actual IM context instance that the view creates on initialization.
Comment 5 Gustavo Noronha (kov) 2010-01-07 15:37:15 PST
FWIW, I think we should go ahead with the proposed API - perhaps turned into a read-only property that exposes the actual object. What does Xan think, I wonder? =)
Comment 6 Christian Dywan 2010-01-26 10:57:13 PST
Whether a property or function, it should return a GtkIMContext*, which is what gtk_im_multicontext_new also returns.
Comment 7 Eric Seidel (no email) 2010-01-26 14:36:04 PST
Comment on attachment 46062 [details]
Patch for this feature

Comments above seem to suggest this has the wrong return value.  r-  Please re-flag if you disagree.
Comment 8 Martin Robinson 2010-01-27 00:31:39 PST
Created attachment 47504 [details]
Updated patch (fixes style issues and passes tests)

Based on all the comments I received, I'm attaching a new patch for this issue which exposes the context as a property of the WebKitWebView of type GtkIMContext.
Comment 9 Gustavo Noronha (kov) 2010-01-27 04:28:06 PST
Comment on attachment 47504 [details]
Updated patch (fixes style issues and passes tests)


> +        Expose the GtkIMMultiContext as a property of WebKitWebView. This will
> +        allow embedders to generate the input method context menu entires and

s/entires/entries/

> +    * This is the input method context used for all text entry widgets inside
> +    * the #WebKitWebView. It can be used to generate context menu items for
> +    * controlling the active input method. g_object_unref does not need to be
> +    * called after retrieving this property.

I don't think this is true. g_object_get will increase the ref count of the object it is getting, so you need to unref it, right? You could say that the WebView is the owner of the object, though.
Comment 10 Martin Robinson 2010-01-27 07:39:12 PST
Created attachment 47532 [details]
Updated patch (fix documentation mistakes)

Whoops. Working late again. Thanks for the review. I've attached a patch fixing those issues.
Comment 11 Gustavo Noronha (kov) 2010-01-27 19:02:26 PST
Comment on attachment 47532 [details]
Updated patch (fix documentation mistakes)

Looks good, thanks!
Comment 12 WebKit Commit Bot 2010-01-27 19:22:43 PST
Comment on attachment 47532 [details]
Updated patch (fix documentation mistakes)

Clearing flags on attachment: 47532

Committed r53967: <http://trac.webkit.org/changeset/53967>
Comment 13 WebKit Commit Bot 2010-01-27 19:22:49 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Duy Nguyen 2010-02-03 06:45:31 PST
*** Bug 33103 has been marked as a duplicate of this bug. ***
Comment 15 Benjamin Otte 2010-02-09 07:12:32 PST
Some quotes from IRC, where exposing IM contexts came up today:

<Company> webkit-gtk is exposing the im context, too
<Company> because it wants to give developers control over the right-click menu
<Company> and you cannot do that sanely without the im context
<Company> (i think there was another reason, but don't remember it)
<pbor> Company: uh? what does the menu have to do with it?
<Company> pbor: you need to add the im context menu options
<pbor> Company: e.g. textview has a popupmenu signal that you can use to extend the menu
<Company> pbor: yeah, but you cannot use it to create your own menu in a sane way
<mclasen> Company: I can't stop people from doing such things....but in gtk3 the im context is not going to be available...
<mclasen> I don't really see how you need the context at all for that
<mclasen> we do have the id of the current im as a property on the widget now
<Company> mclasen, pbor: https://bugs.webkit.org/show_bug.cgi?id=33327
* mclasen not impressed
<mclasen> with webkit exposing gtk interna
<Company> mclasen: how would you solve the problem (of having an API for right-click menus)?
<mclasen> Company: I would start by describing my needs in a gtk bug...
<mclasen> Company: that bug should also explain how the populate-popup signals are insufficient...