[Webkit-unassigned] [Bug 28733] [GTK] GtkIMContext filtering interferes with DOM key events

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 26 00:16:52 PDT 2009


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





--- Comment #2 from Xan Lopez <xan.lopez at gmail.com>  2009-08-26 00:16:52 PDT ---
(From update of attachment 38597)
So, this looks fine in general to me, but I have some issues:

- How do you know the behavior the IM code should have? By inspection of other
ports? Some spec? Just the webpage you have linked to? (Not that I don't
believe we are broken, especially since the inspector is also failing; just
wondering :))
- I'm kind of weary to touch this stuff a lot without having tests for it, it
would be great to finish eventSender to enable them or add new ones.
- Your patch will break my stuff in
https://bugs.webkit.org/show_bug.cgi?id=25526, which would be also great to
finish :). It's also blocked on doing tests for it though.

A few small details:

> diff --git a/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp b/WebKit/gtk/WebCoreSupport/EditorClientGtk.cpp
>  bool EditorClient::shouldBeginEditing(WebCore::Range*)
>  {
> +    g_free(pendingComposition);
> +    pendingComposition = 0;
> +    g_free(pendingPreedit);
> +    pendingPreedit = 0;
> +

This chunk is repeated three times, probably worth to do a small function for
it.

>      notImplemented();
>      return true;
>  }
>  
>  bool EditorClient::shouldEndEditing(WebCore::Range*)
>  {
> +    g_free(pendingComposition);
> +    pendingComposition = 0;
> +    g_free(pendingPreedit);
> +    pendingPreedit = 0;
> +
>      notImplemented();

Wonder at what point it's silly to keep the notImplemented() in these
functions; what should happen here?

> -
> -    if (keyEvent->type() == PlatformKeyboardEvent::RawKeyDown) {
> -        // WebKit doesn't have enough information about mode to decide how commands that just insert text if executed via Editor should be treated,
> -        // so we leave it upon WebCore to either handle them immediately (e.g. Tab that changes focus) or let a keypress event be generated
> -        // (e.g. Tab that inserts a Tab character, or Enter).
> -        return !command.isTextInsertion() && command.execute(evt);
> +    const gchar* editorCommandString = interpretEditorCommandKeyEvent(event);
> +    if (editorCommandString)
> +    {

Brace in wrong line.

All that being said, I think I would like someone else to look at this before
r+ the patch, I'm not so familiar with all the details in this.

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



More information about the webkit-unassigned mailing list