Bug 24193 - [GTK] Checkbuttons not activated with space
Summary: [GTK] Checkbuttons not activated with space
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 23948 (view as bug list)
Depends on:
Blocks:
 
Reported: 2009-02-26 01:39 PST by Xan Lopez
Modified: 2009-03-01 13:41 PST (History)
1 user (show)

See Also:


Attachments
imevent.patch (2.27 KB, patch)
2009-02-26 01:45 PST, Xan Lopez
ap: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Xan Lopez 2009-02-26 01:39:25 PST
Checkbuttons are not activated with space. The reason is that the IM code eats the event, then tries to reintroduce it, and fails when it sees that a checkbutton is not editable. I think the proper fix is simply to not have IM eat events when the focus is on non-editable frames (is frame the right terminology here?).

And no, this does not fix https://bugs.webkit.org/show_bug.cgi?id=18363 :)
Comment 1 Xan Lopez 2009-02-26 01:45:43 PST
Created attachment 28010 [details]
imevent.patch

commit 1f23190066d5e8740a0e6ab9b26a944a270d4fc5
Author: Xan Lopez <xan@gnome.org>
Date:   Thu Feb 26 11:44:29 2009 +0200

    2009-02-26  Xan Lopez  <xan@gnome.org>

            Reviewed by NOBODY (OOPS!).

            https://bugs.webkit.org/show_bug.cgi?id=24193
            [GTK] Checkbuttons not activated with space

            Do not swallow key events with GtkIMContext for non-editable
            frames (FIXME: I don't think frame is the right name here).

            * WebCoreSupport/EditorClientGtk.cpp:
            (WebKit::EditorClient::handleInputMethodKeydown):
Comment 2 Alexey Proskuryakov 2009-02-26 05:19:06 PST
Comment on attachment 28010 [details]
imevent.patch

>         Do not swallow key events with GtkIMContext for non-editable
>         frames (FIXME: I don't think frame is the right name here).

That's non-editable content.

> +    if (!targetFrame || !targetFrame->editor()->canEdit())
> +        return;

Can targetFrame really be null here?

It's not obvious to me that no input method has useful functions for non-editable content - some Mac IMs have such. So, it may be not quite correct to do an early return here. But it's clear that this code is in need of large refactoring anyway to support input events, and I think that this patch improves the behavior more than it (potentially) regresses it.

>      // TODO: Dispatch IE-compatible text input events for IM events.

We use FIXME, not TODO. Even thought this is old code, it seems appropriate to correct it now.
Comment 3 Holger Freyther 2009-02-26 08:33:44 PST
Landed in r41249.
Comment 4 Xan Lopez 2009-03-01 13:41:17 PST
*** Bug 23948 has been marked as a duplicate of this bug. ***