[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 #3 from Martin Robinson <mrobinson at webkit.org>  2011-01-28 09:36:20 PST ---
(From update of attachment 80443)
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!

> 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.

> 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

> 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?

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