Bug 15653 - [GTK] Text editor does not handle common keystrokes
Summary: [GTK] Text editor does not handle common keystrokes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 523.x (Safari 3)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
: 15628 (view as bug list)
Depends on:
Blocks:
 
Reported: 2007-10-23 18:01 PDT by Alp Toker
Modified: 2007-11-08 17:16 PST (History)
1 user (show)

See Also:


Attachments
Implementation A (7.57 KB, patch)
2007-10-23 18:07 PDT, Alp Toker
oliver: review-
Details | Formatted Diff | Diff
Implementation B (7.44 KB, patch)
2007-10-23 18:10 PDT, Alp Toker
mrowe: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alp Toker 2007-10-23 18:01:40 PDT
EditorClientGtk is missing several cases and its behaviour does not match that of GTK+.
Comment 1 Alp Toker 2007-10-23 18:07:34 PDT
Created attachment 16823 [details]
Implementation A

This is the first of two possible implementations that should achieve the same thing. Whichever will become part of WebKit/Gtk+ is up to the audience.
Comment 2 Alp Toker 2007-10-23 18:10:17 PDT
Created attachment 16824 [details]
Implementation B

This is the second of two possible implementations that should achieve the same thing. Whichever will become part of WebKit/Gtk+ is up to the audience.
Comment 3 Alp Toker 2007-10-23 18:22:54 PDT
Bug #16104 has yet another approach to this problem that's worth looking at.
Comment 4 Oliver Hunt 2007-10-25 22:26:11 PDT
Hmmm, I think the windows path is clearer, and broadly nicer

See  WebView::interpretKeyEvent in WebKit/win/WebView.cpp


Comment 5 Oliver Hunt 2007-10-25 22:27:21 PDT
IIRC there's a patch somewhere to make gtk, win, and qt share there interpretKeyEvents implementation for handleKeyPress, though i can't rememember the number and i suspect it's been orphaned.
Comment 6 Oliver Hunt 2007-10-25 22:45:39 PDT
Comment on attachment 16823 [details]
Implementation A

As i said in a previous comment i don't think either of these approaches is really ideal
Comment 7 Mike Hommey 2007-10-25 22:52:45 PDT
(In reply to comment #3)
> Bug #16104 has yet another approach to this problem that's worth looking at.

Bug #16104 doesn't exist.

Comment 8 Alp Toker 2007-10-25 23:41:46 PDT
Sorry, I meant #15057

I came up with the quick solution so we could get more testing for clipboard and tab/focus issues, but if an alternative is ready, that's great.

I had a feeling at the start that eventually we're going to use the GDK_KP directly, but maybe it does make sense to share this code instead.
Comment 9 Jan Alonzo 2007-10-26 13:14:40 PDT
*** Bug 15628 has been marked as a duplicate of this bug. ***
Comment 10 Alp Toker 2007-11-06 13:22:45 PST
To be clear, if we were to do a dynamic lookup table, we would just go and use GtkBindingSet. However, there are no resources to implement this right now and the provided patch works, and there is precedent for this hack to get positive review in the Qt port. Please reconsider this review so users can get basic input support as a temporary measure.
Comment 11 Mark Rowe (bdash) 2007-11-08 14:31:59 PST
Comment on attachment 16824 [details]
Implementation B

While this approach is suboptimal in the long run, the Gtk port is in need of some form of sane input handling so that work on related features can progress.  The ideal approach probably involves using GtkBindingSet which, as I understand it, will respect user-customised key bindings and all that fancy stuff.

r=me if a new bug is filed about switching to GtkBindingSet at some point in the future, and the comment in the patch is updated to a FIXME referencing the bug.
Comment 12 Alp Toker 2007-11-08 17:16:42 PST
Landed in r27624.

See also bug #15911 (GtkBindingSet).