[Webkit-unassigned] [Bug 52997] [GTK] DRT's TextInputController is unimplemented on GTK

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52997





--- Comment #4 from Carlos Garcia Campos <cgarcia at igalia.com>  2011-01-28 10:00:23 PST ---
(In reply to comment #3)
> (From update of attachment 80443 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=80443&action=review
> 
> This is awesome! I'd prefer you to try to go through the im-context where possible so that we have adequate testing on the WebKit layer. Does this also need "doCommand"? Other than that, it looks great!

Yes, well, that's why the changelog says "initial" implementation, I added the minimum stuff to fix the tests mentioned in this bug report, but there are several TextInputController methods not implemented, like doCommand that we can add in future patches. 

> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:471
> > +void DumpRenderTreeSupportGtk::setComposition(WebKitWebView* webView, const gchar* text, gint start, gint end)
> 
> Please use char and int here, since this isn't public API.

Ok

> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:489
> > +    String compositionString = String::fromUTF8(text);
> > +    Vector<CompositionUnderline> underlines;
> > +    underlines.append(CompositionUnderline(0, compositionString.length(), Color(0, 0, 0), false));
> > +    editor->setComposition(compositionString, underlines, start, end);
> 
> If possible I think I'd either like to see this touch the GtkIMContext of the WebView either here or below. That way this will also test the WebKit layer.
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:492
> > +void DumpRenderTreeSupportGtk::confirmComposition(WebKitWebView* webView, const gchar* text)
> 
> ghcar -> char
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:510
> > +    if (editor->hasComposition()) {
> > +        if (text)
> > +            editor->confirmComposition(String::fromUTF8(text));
> > +        else
> > +            editor->confirmComposition();
> > +    } else
> > +        editor->insertText(String::fromUTF8(text), 0);
> 
> Same comment as above.
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:513
> > +bool DumpRenderTreeSupportGtk::firstRectForCharacterRange(WebKitWebView* webView, gint location, gint length, GdkRectangle* rect)
> 
> gint -> int. It's fine to keep GdkRectangle since it's the only rect type that DumpRenderTree and WebKit share.
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:540
> > +bool DumpRenderTreeSupportGtk::selectedRange(WebKitWebView* webView, gint* start, gint* end)
> 
> Ditto.
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.cpp:562
> > +    RefPtr<Range> testRange = Range::create(scope->document(), scope, 0, range->startContainer(), range->startOffset());
> > +    ASSERT(testRange->startContainer() == scope);
> > +    *start = TextIterator::rangeLength(testRange.get());
> > +
> > +    ExceptionCode ec;
> > +    testRange->setEnd(range->endContainer(), range->endOffset(), ec);
> > +    ASSERT(testRange->startContainer() == scope);
> > +    *end = TextIterator::rangeLength(testRange.get());
> 
> GTK+ doesn't seem to have the equivalent of seletion events the same way Qt and Mac do, so we must do this here. :)
> 
> > Source/WebKit/gtk/WebCoreSupport/DumpRenderTreeSupportGtk.h:76
> > +    static void setComposition(WebKitWebView*, const gchar* text, gint start, gint end);
> > +    static void confirmComposition(WebKitWebView*, const gchar* text);
> > +    static bool firstRectForCharacterRange(WebKitWebView*, gint location, gint length, GdkRectangle*);
> > +    static bool selectedRange(WebKitWebView*, gint* start, gint* end);
> 
> These should probably just be static functions in TextInputController.h

They are here because they use WebCore API.

> > Tools/DumpRenderTree/gtk/TextInputController.cpp:65
> > +    g_return_val_if_fail((!exception || !*exception), JSValueMakeUndefined(context));
> > +
> > +    DumpRenderTreeSupportGtk::setComposition(view, stringBuffer.get(), start, end);
> > +
> > +    return JSValueMakeUndefined(context);
> 
> Can you just use the im-context property from the WebView and emit the preedit-changed signal?
> 
> > Tools/DumpRenderTree/gtk/TextInputController.cpp:87
> > +static JSValueRef insertTextCallback(JSContextRef context, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* exception)
> > +{
> > +    WebKitWebView* view = webkit_web_frame_get_web_view(mainFrame);
> > +    if (!view)
> > +        return JSValueMakeUndefined(context);
> > +
> > +    if (argumentCount < 1)
> > +        return JSValueMakeUndefined(context);
> > +
> > +    JSStringRef string = JSValueToStringCopy(context, arguments[0], exception);
> > +    g_return_val_if_fail((!exception || !*exception), JSValueMakeUndefined(context));
> > +
> > +    size_t bufferSize = JSStringGetMaximumUTF8CStringSize(string);
> > +    GOwnPtr<gchar> stringBuffer(static_cast<gchar*>(g_malloc(bufferSize)));
> > +    JSStringGetUTF8CString(string, stringBuffer.get(), bufferSize);
> > +    JSStringRelease(string);
> > +
> > +    DumpRenderTreeSupportGtk::confirmComposition(view, stringBuffer.get());
> > +
> > +    return JSValueMakeUndefined(context);
> 
> Is it possible just to get the im-context via the im-context property on the WebView and then emit the "commit" signal?

I don't know how im-context works, I'll try to use it instead. Thanks for the comments!

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.