Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
5254011:
Forward unhandled KeyEvents to WidgetGtk's HandleKeyboardEvent() in native textfields. (Closed)
|
Created:
10 years ago by Zachary Kuznia Modified:
9 years, 7 months ago CC:
chromium-reviews, ben+cc_chromium.org, Daniel Erat, Nikita (slow) Visibility:
Public. |
DescriptionForward unhandled KeyEvents to WidgetGtk's HandleKeyboardEvent() in native textfields.
BUG=chromium-os:6990, chromium-os:9738
TEST=Click on the omnibox's star. Press enter. Check that the bookmark bubble closes.
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=69241
Patch Set 1 #Patch Set 2 : Code Review #
Total comments: 2
Patch Set 3 : Invoke HandleKeyboardEvent from OnKeyEvent() #
Total comments: 2
Patch Set 4 : Re-add activate, add fix for 9738 #
Total comments: 8
Patch Set 5 : Fix nits #
Total comments: 4
Messages
Total messages: 22 (0 generated)
Do we know why hitting return key does not activate accelerator? ctrl-return seems to be working for findbar, so I wonder why return does not work for bookmark bubble. I have a concern about adding this behavior to all textfield because win's textfield doesn't issue the accelerator event and this may have side effect in other places.
On 2010/11/30 18:44:05, oshima wrote: > Do we know why hitting return key does not activate accelerator? ctrl-return > seems to be working for findbar, so I wonder why return does not work for > bookmark bubble. > > I have a concern about adding this behavior to all textfield > because win's textfield doesn't issue the accelerator event and this may have > side effect in other places. On 2010/11/30 18:44:05, oshima wrote: > Do we know why hitting return key does not activate accelerator? ctrl-return > seems to be working for findbar, so I wonder why return does not work for > bookmark bubble. > > I have a concern about adding this behavior to all textfield > because win's textfield doesn't issue the accelerator event and this may have > side effect in other places. I followed the keyboard handling logic a bit more, and it seems that the problem is that the text box is consuming the key event, so it stops getting processed at "handled = gtk_window_propagate_key_event(GTK_WINDOW(widget), event);" in views/widget/widget_gtk.cc:1186. This causes HandleKeyboardEvent() to be skipped, which would pass the key event to the focus manager, which processes accelerators. James, you moved this check here to fix crbug.com/40966, would you happen to know the proper way to keep the textbox from consuming the keystrokes?
For this case, handling the "activate" signal should be the most proper way. http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... views/controls/textfield/native_textfield_gtk.cc:372: Accelerator(app::VKEY_RETURN, false, false, false)); It's probably better to call the widget's HandleKeyboardEvent() method with the current gdk event (if it's a keyboard event). In case the accelerator is handled elsewhere. http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... views/controls/textfield/native_textfield_gtk.cc:426: g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), this); Only GtkEntry has "activate" signal.
My question was that the accelerator is supposed to capture keyevents before being sent to GTK widgets, and views code expects this behavior. I wonder why the textfield has to explicitly sends accelerator event in this case. - oshima On Wed, Dec 1, 2010 at 10:35 AM, <suzhe@chromium.org> wrote: > For this case, handling the "activate" signal should be the most proper > way. > > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > File views/controls/textfield/native_textfield_gtk.cc (right): > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > views/controls/textfield/native_textfield_gtk.cc:372: > > Accelerator(app::VKEY_RETURN, false, false, false)); > It's probably better to call the widget's HandleKeyboardEvent() method > with the current gdk event (if it's a keyboard event). > In case the accelerator is handled elsewhere. > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > views/controls/textfield/native_textfield_gtk.cc:426: > > g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), > this); > Only GtkEntry has "activate" signal. > > > http://codereview.chromium.org/5254011/ >
Please refer to: http://codereview.chromium.org/3046041 In short, handling key events as accelerators before sending them to widgets may break widgets' behavior badly. So we should always send key events to widgets first, it matches the behavior of linux and mac ports. Window ports behaves differently because DefWindowProc() will be called if a native widget doesn't handle a key event, thus we have no chance to handle it anymore. On 2010/12/01 20:00:29, oshima wrote: > My question was that the accelerator is supposed to capture keyevents before > being sent to GTK widgets, > and views code expects this behavior. I wonder why the textfield has to > explicitly sends accelerator event > in this case. > > - oshima > > On Wed, Dec 1, 2010 at 10:35 AM, <mailto:suzhe@chromium.org> wrote: > > > For this case, handling the "activate" signal should be the most proper > > way. > > > > > > > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > > File views/controls/textfield/native_textfield_gtk.cc (right): > > > > > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > > views/controls/textfield/native_textfield_gtk.cc:372: > > > > Accelerator(app::VKEY_RETURN, false, false, false)); > > It's probably better to call the widget's HandleKeyboardEvent() method > > with the current gdk event (if it's a keyboard event). > > In case the accelerator is handled elsewhere. > > > > > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > > views/controls/textfield/native_textfield_gtk.cc:426: > > > > g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), > > this); > > Only GtkEntry has "activate" signal. > > > > > > http://codereview.chromium.org/5254011/ > >
On Wed, Dec 1, 2010 at 12:25 PM, <suzhe@chromium.org> wrote: > Please refer to: http://codereview.chromium.org/3046041 > > In short, handling key events as accelerators before sending them to > widgets may > break widgets' behavior badly. So we should always send key events to > widgets > first, it matches the behavior of linux and mac ports. Window ports behaves > differently because DefWindowProc() will be called if a native widget > doesn't > handle a key event, thus we have no chance to handle it anymore. > > I see. I still worrying about adding accelerato rto textfield itself, so james' suggestion (calling HandleKeyboardEvent) sounds the right solution to me. - oshima > On 2010/12/01 20:00:29, oshima wrote: > >> My question was that the accelerator is supposed to capture keyevents >> before >> being sent to GTK widgets, >> and views code expects this behavior. I wonder why the textfield has to >> explicitly sends accelerator event >> in this case. >> > > - oshima >> > > On Wed, Dec 1, 2010 at 10:35 AM, <mailto:suzhe@chromium.org> wrote: >> > > > For this case, handling the "activate" signal should be the most proper >> > way. >> > >> > >> > >> > >> > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > >> > File views/controls/textfield/native_textfield_gtk.cc (right): >> > >> > >> > >> > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > >> > views/controls/textfield/native_textfield_gtk.cc:372: >> > >> > Accelerator(app::VKEY_RETURN, false, false, false)); >> > It's probably better to call the widget's HandleKeyboardEvent() method >> > with the current gdk event (if it's a keyboard event). >> > In case the accelerator is handled elsewhere. >> > >> > >> > >> > > > http://codereview.chromium.org/5254011/diff/2001/views/controls/textfield/nat... > >> > views/controls/textfield/native_textfield_gtk.cc:426: >> > >> > g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), >> > this); >> > Only GtkEntry has "activate" signal. >> > >> > >> > http://codereview.chromium.org/5254011/ >> > >> > > > > http://codereview.chromium.org/5254011/ >
Updated this change to instead forward key events to WidgetGtk's HandleKeyboardEvent(). Please have another look.
Please make sure that a key event will always be handled by the native widget. Otherwise it may affect the behavior of native widget or input methods. You may try this use case: open find in page box, enable an input method and input something to trigger candidate window, then press Up and Down to select candidate. You may find that the keys scroll the web page rather than selecting the candidate. http://codereview.chromium.org/5254011/diff/13001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/13001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:372: return handled; This approach can cause problem as it handles a key event before sending it to the native widget. As I said, all keyboard events should be sent to the native widget before being handled in views. Actually the original OnKeyPressEvent() method already has this issue. Hooking "activate" signal should be the best approach. Just call widget->HandleKeyboardEvent(gtk_get_current_event()) in the signal handler. Of course, it's better to check if the current event is a key press event. http://codereview.chromium.org/5254011/diff/13001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:423: G_CALLBACK(OnKeyPressEventHandler), this); We need to use g_signal_connect_after() here to make sure all key events will be handled by the native widget first.
Updated the change to fix 9738 as well. Please have another look.
LGTM, except some nits. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:376: } No braces. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:390: } No braces. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:442: G_CALLBACK(OnKeyPressEventHandler), this); Alignment. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:445: g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), this); & is not necessary.
http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:376: } On 2010/12/15 06:06:43, James Su wrote: > No braces. Done. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:390: } On 2010/12/15 06:06:43, James Su wrote: > No braces. Done. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:442: G_CALLBACK(OnKeyPressEventHandler), this); On 2010/12/15 06:06:43, James Su wrote: > Alignment. Done. http://codereview.chromium.org/5254011/diff/18001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:445: g_signal_connect(widget, "activate", G_CALLBACK(&OnActivateHandler), this); On 2010/12/15 06:06:43, James Su wrote: > & is not necessary. Done.
http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:440: g_signal_connect_after(widget, "key-press-event", Changing g_signal_connect_after to g_signal_connect basically broke OnKeyPressEvent functionality for edit boxes. Almost no events now delivered to OnKeyPressEvent because default processing handles most of them. In particular I'm fixing http://code.google.com/p/chromium-os/issues/detail?id=10958 that happened due to this change. There is another issue http://code.google.com/p/chromium-os/issues/detail?id=10362. I think this change should be reverted to repair OnKeyPressEvent functionality.
In the future, please CC any CL related to keyboard event handling to me. http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:440: g_signal_connect_after(widget, "key-press-event", On 2011/01/17 10:50:19, Dmitry Polukhin wrote: > Changing g_signal_connect_after to g_signal_connect basically broke > OnKeyPressEvent functionality for edit boxes. Almost no events now delivered to > OnKeyPressEvent because default processing handles most of them. In particular > I'm fixing http://code.google.com/p/chromium-os/issues/detail?id=10958 that > happened due to this change. There is another issue > http://code.google.com/p/chromium-os/issues/detail?id=10362. > > I think this change should be reverted to repair OnKeyPressEvent functionality. We should never intercept key events before being handled by the native control. Otherwise the functionalities of the native control and the input method may be broken. For each key event needs intercepting, we need to handle it like what we have done for Enter key in OnAcivateHandler(). For example, we need to handle arrow keys in "move-cursor" signal handler and Tab key in "move-focus" signal handler, etc.
http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:440: g_signal_connect_after(widget, "key-press-event", On 2011/01/17 18:35:16, James Su wrote: > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: > > Changing g_signal_connect_after to g_signal_connect basically broke > > OnKeyPressEvent functionality for edit boxes. Almost no events now delivered > to > > OnKeyPressEvent because default processing handles most of them. In particular > > I'm fixing http://code.google.com/p/chromium-os/issues/detail?id=10958 that > > happened due to this change. There is another issue > > http://code.google.com/p/chromium-os/issues/detail?id=10362. > > > > I think this change should be reverted to repair OnKeyPressEvent > functionality. > We should never intercept key events before being handled by the native control. > Otherwise the functionalities of the native control and the input method may be > broken. For each key event needs intercepting, we need to handle it like what we > have done for Enter key in OnAcivateHandler(). For example, we need to handle > arrow keys in "move-cursor" signal handler and Tab key in "move-focus" signal > handler, etc. What will happen on Windows? Is it possible to write platform independent code if different events are delivered to handler? Should we remove OnKeyPressEvent because it doesn't work anymore (no events are delivered)?
http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... File views/controls/textfield/native_textfield_gtk.cc (right): http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... views/controls/textfield/native_textfield_gtk.cc:440: g_signal_connect_after(widget, "key-press-event", On 2011/01/17 20:10:21, Dmitry Polukhin wrote: > On 2011/01/17 18:35:16, James Su wrote: > > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: > > > Changing g_signal_connect_after to g_signal_connect basically broke > > > OnKeyPressEvent functionality for edit boxes. Almost no events now delivered > > to > > > OnKeyPressEvent because default processing handles most of them. In > particular > > > I'm fixing http://code.google.com/p/chromium-os/issues/detail?id=10958 that > > > happened due to this change. There is another issue > > > http://code.google.com/p/chromium-os/issues/detail?id=10362. > > > > > > I think this change should be reverted to repair OnKeyPressEvent > > functionality. > > We should never intercept key events before being handled by the native > control. > > Otherwise the functionalities of the native control and the input method may > be > > broken. For each key event needs intercepting, we need to handle it like what > we > > have done for Enter key in OnAcivateHandler(). For example, we need to handle > > arrow keys in "move-cursor" signal handler and Tab key in "move-focus" signal > > handler, etc. > > What will happen on Windows? Is it possible to write platform independent code > if different events are delivered to handler? Should we remove OnKeyPressEvent > because it doesn't work anymore (no events are delivered)? Windows has different event flow, which will always dispatch an key event to the input method, so things like OnKeyPressEvent and SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key events are dispatched to the input method by the widget itself, so we may break the input method badly if we intercept key events before the widget. So it's impossible for us to the exact same code for these two platforms. But we can abstract and share at least part of code between them. Like what we have done in above OnActivate() method, we can use such kind of trick to deal with the platform difference and put shared key event handling logic in controller's HandleKeystroke() method.
On Mon, Jan 17, 2011 at 1:25 PM, <suzhe@chromium.org> wrote: > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > File views/controls/textfield/native_textfield_gtk.cc (right): > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > views/controls/textfield/native_textfield_gtk.cc:440: > g_signal_connect_after(widget, "key-press-event", > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: > >> On 2011/01/17 18:35:16, James Su wrote: >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: >> > > Changing g_signal_connect_after to g_signal_connect basically >> > broke > >> > > OnKeyPressEvent functionality for edit boxes. Almost no events now >> > delivered > >> > to >> > > OnKeyPressEvent because default processing handles most of them. >> > In > >> particular >> > > I'm fixing >> > http://code.google.com/p/chromium-os/issues/detail?id=10958 that > >> > > happened due to this change. There is another issue >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. >> > > >> > > I think this change should be reverted to repair OnKeyPressEvent >> > functionality. >> > We should never intercept key events before being handled by the >> > native > >> control. >> > Otherwise the functionalities of the native control and the input >> > method may > >> be >> > broken. For each key event needs intercepting, we need to handle it >> > like what > >> we >> > have done for Enter key in OnAcivateHandler(). For example, we need >> > to handle > >> > arrow keys in "move-cursor" signal handler and Tab key in >> > "move-focus" signal > >> > handler, etc. >> > > What will happen on Windows? Is it possible to write platform >> > independent code > >> if different events are delivered to handler? Should we remove >> > OnKeyPressEvent > >> because it doesn't work anymore (no events are delivered)? >> > Windows has different event flow, which will always dispatch an key > event to the input method, so things like OnKeyPressEvent and > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key > events are dispatched to the input method by the widget itself, so we > may break the input method badly if we intercept key events before the > widget. > So it's impossible for us to the exact same code for these two > platforms. But we can abstract and share at least part of code between > them. Like what we have done in above OnActivate() method, we can use > such kind of trick to deal with the platform difference and put shared > key event handling logic in controller's HandleKeystroke() method. > > > http://codereview.chromium.org/5254011/ > This is FYI. We're working on gtk less chrome, primarily for touch, but also for future version of chromeos (schedule is TBD). Replacement for chromeos won't happen anytime soon, but hopefully we can address this issue sometime in the future. - oshima
On 2011/01/18 20:19:07, oshima wrote: > On Mon, Jan 17, 2011 at 1:25 PM, <mailto:suzhe@chromium.org> wrote: > > > > > > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > > File views/controls/textfield/native_textfield_gtk.cc (right): > > > > > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > > views/controls/textfield/native_textfield_gtk.cc:440: > > g_signal_connect_after(widget, "key-press-event", > > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: > > > >> On 2011/01/17 18:35:16, James Su wrote: > >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: > >> > > Changing g_signal_connect_after to g_signal_connect basically > >> > > broke > > > >> > > OnKeyPressEvent functionality for edit boxes. Almost no events now > >> > > delivered > > > >> > to > >> > > OnKeyPressEvent because default processing handles most of them. > >> > > In > > > >> particular > >> > > I'm fixing > >> > > http://code.google.com/p/chromium-os/issues/detail?id=10958 that > > > >> > > happened due to this change. There is another issue > >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. > >> > > > >> > > I think this change should be reverted to repair OnKeyPressEvent > >> > functionality. > >> > We should never intercept key events before being handled by the > >> > > native > > > >> control. > >> > Otherwise the functionalities of the native control and the input > >> > > method may > > > >> be > >> > broken. For each key event needs intercepting, we need to handle it > >> > > like what > > > >> we > >> > have done for Enter key in OnAcivateHandler(). For example, we need > >> > > to handle > > > >> > arrow keys in "move-cursor" signal handler and Tab key in > >> > > "move-focus" signal > > > >> > handler, etc. > >> > > > > What will happen on Windows? Is it possible to write platform > >> > > independent code > > > >> if different events are delivered to handler? Should we remove > >> > > OnKeyPressEvent > > > >> because it doesn't work anymore (no events are delivered)? > >> > > Windows has different event flow, which will always dispatch an key > > event to the input method, so things like OnKeyPressEvent and > > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key > > events are dispatched to the input method by the widget itself, so we > > may break the input method badly if we intercept key events before the > > widget. > > So it's impossible for us to the exact same code for these two > > platforms. But we can abstract and share at least part of code between > > them. Like what we have done in above OnActivate() method, we can use > > such kind of trick to deal with the platform difference and put shared > > key event handling logic in controller's HandleKeystroke() method. > > > > > > http://codereview.chromium.org/5254011/ > > > > This is FYI. We're working on gtk less chrome, primarily for touch, but also > for future version of chromeos (schedule is TBD). > Replacement for chromeos won't happen anytime soon, but hopefully we can > address this issue sometime in the future. > > - oshima Thanks for your information. I noticed that you are working on a native textfield implementation based on Views. I'm wondering is there any documentation/web site/issue to trace the progress of this effort?
On Tue, Jan 18, 2011 at 12:31 PM, <suzhe@chromium.org> wrote: > On 2011/01/18 20:19:07, oshima wrote: > > On Mon, Jan 17, 2011 at 1:25 PM, <mailto:suzhe@chromium.org> wrote: >> > > > >> > >> > >> > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > >> > File views/controls/textfield/native_textfield_gtk.cc (right): >> > >> > >> > >> > > > http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... > >> > views/controls/textfield/native_textfield_gtk.cc:440: >> > g_signal_connect_after(widget, "key-press-event", >> > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: >> > >> >> On 2011/01/17 18:35:16, James Su wrote: >> >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: >> >> > > Changing g_signal_connect_after to g_signal_connect basically >> >> >> > broke >> > >> >> > > OnKeyPressEvent functionality for edit boxes. Almost no events now >> >> >> > delivered >> > >> >> > to >> >> > > OnKeyPressEvent because default processing handles most of them. >> >> >> > In >> > >> >> particular >> >> > > I'm fixing >> >> >> > http://code.google.com/p/chromium-os/issues/detail?id=10958 that >> > >> >> > > happened due to this change. There is another issue >> >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. >> >> > > >> >> > > I think this change should be reverted to repair OnKeyPressEvent >> >> > functionality. >> >> > We should never intercept key events before being handled by the >> >> >> > native >> > >> >> control. >> >> > Otherwise the functionalities of the native control and the input >> >> >> > method may >> > >> >> be >> >> > broken. For each key event needs intercepting, we need to handle it >> >> >> > like what >> > >> >> we >> >> > have done for Enter key in OnAcivateHandler(). For example, we need >> >> >> > to handle >> > >> >> > arrow keys in "move-cursor" signal handler and Tab key in >> >> >> > "move-focus" signal >> > >> >> > handler, etc. >> >> >> > >> > What will happen on Windows? Is it possible to write platform >> >> >> > independent code >> > >> >> if different events are delivered to handler? Should we remove >> >> >> > OnKeyPressEvent >> > >> >> because it doesn't work anymore (no events are delivered)? >> >> >> > Windows has different event flow, which will always dispatch an key >> > event to the input method, so things like OnKeyPressEvent and >> > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key >> > events are dispatched to the input method by the widget itself, so we >> > may break the input method badly if we intercept key events before the >> > widget. >> > So it's impossible for us to the exact same code for these two >> > platforms. But we can abstract and share at least part of code between >> > them. Like what we have done in above OnActivate() method, we can use >> > such kind of trick to deal with the platform difference and put shared >> > key event handling logic in controller's HandleKeystroke() method. >> > >> > >> > http://codereview.chromium.org/5254011/ >> > >> > > This is FYI. We're working on gtk less chrome, primarily for touch, but >> also >> for future version of chromeos (schedule is TBD). >> Replacement for chromeos won't happen anytime soon, but hopefully we can >> address this issue sometime in the future. >> > > - oshima >> > > Thanks for your information. I noticed that you are working on a native > textfield implementation based on Views. I'm wondering is there any > documentation/web site/issue to trace the progress of this effort? No, not really. Is there any specific thing you want to know about? - oshima > > > > http://codereview.chromium.org/5254011/ >
在 2011年1月18日 下午9:19，oshima <oshima@chromium.org>写道： > > > On Tue, Jan 18, 2011 at 12:31 PM, <suzhe@chromium.org> wrote: > >> On 2011/01/18 20:19:07, oshima wrote: >> >> On Mon, Jan 17, 2011 at 1:25 PM, <mailto:suzhe@chromium.org> wrote: >>> >> >> > >>> > >>> > >>> >> >> >> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >> >>> > File views/controls/textfield/native_textfield_gtk.cc (right): >>> > >>> > >>> > >>> >> >> >> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >> >>> > views/controls/textfield/native_textfield_gtk.cc:440: >>> > g_signal_connect_after(widget, "key-press-event", >>> > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: >>> > >>> >> On 2011/01/17 18:35:16, James Su wrote: >>> >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: >>> >> > > Changing g_signal_connect_after to g_signal_connect basically >>> >> >>> > broke >>> > >>> >> > > OnKeyPressEvent functionality for edit boxes. Almost no events now >>> >> >>> > delivered >>> > >>> >> > to >>> >> > > OnKeyPressEvent because default processing handles most of them. >>> >> >>> > In >>> > >>> >> particular >>> >> > > I'm fixing >>> >> >>> > http://code.google.com/p/chromium-os/issues/detail?id=10958 that >>> > >>> >> > > happened due to this change. There is another issue >>> >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. >>> >> > > >>> >> > > I think this change should be reverted to repair OnKeyPressEvent >>> >> > functionality. >>> >> > We should never intercept key events before being handled by the >>> >> >>> > native >>> > >>> >> control. >>> >> > Otherwise the functionalities of the native control and the input >>> >> >>> > method may >>> > >>> >> be >>> >> > broken. For each key event needs intercepting, we need to handle it >>> >> >>> > like what >>> > >>> >> we >>> >> > have done for Enter key in OnAcivateHandler(). For example, we need >>> >> >>> > to handle >>> > >>> >> > arrow keys in "move-cursor" signal handler and Tab key in >>> >> >>> > "move-focus" signal >>> > >>> >> > handler, etc. >>> >> >>> > >>> > What will happen on Windows? Is it possible to write platform >>> >> >>> > independent code >>> > >>> >> if different events are delivered to handler? Should we remove >>> >> >>> > OnKeyPressEvent >>> > >>> >> because it doesn't work anymore (no events are delivered)? >>> >> >>> > Windows has different event flow, which will always dispatch an key >>> > event to the input method, so things like OnKeyPressEvent and >>> > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key >>> > events are dispatched to the input method by the widget itself, so we >>> > may break the input method badly if we intercept key events before the >>> > widget. >>> > So it's impossible for us to the exact same code for these two >>> > platforms. But we can abstract and share at least part of code between >>> > them. Like what we have done in above OnActivate() method, we can use >>> > such kind of trick to deal with the platform difference and put shared >>> > key event handling logic in controller's HandleKeystroke() method. >>> > >>> > >>> > http://codereview.chromium.org/5254011/ >>> > >>> >> >> This is FYI. We're working on gtk less chrome, primarily for touch, but >>> also >>> for future version of chromeos (schedule is TBD). >>> Replacement for chromeos won't happen anytime soon, but hopefully we can >>> address this issue sometime in the future. >>> >> >> - oshima >>> >> >> Thanks for your information. I noticed that you are working on a native >> textfield implementation based on Views. I'm wondering is there any >> documentation/web site/issue to trace the progress of this effort? > > > No, not really. Is there any specific thing you want to know about? > I'm just wondering how to integrate with input method framework. I'm currently designing and implementing a new input method framework and hope it can replace ibus in the future. > > - oshima > >> >> >> >> http://codereview.chromium.org/5254011/ >> > >
On Tue, Jan 18, 2011 at 9:23 PM, James Su <suzhe@chromium.org> wrote: > > > 在 2011年1月18日 下午9:19，oshima <oshima@chromium.org>写道： > > >> >> On Tue, Jan 18, 2011 at 12:31 PM, <suzhe@chromium.org> wrote: >> >>> On 2011/01/18 20:19:07, oshima wrote: >>> >>> On Mon, Jan 17, 2011 at 1:25 PM, <mailto:suzhe@chromium.org> wrote: >>>> >>> >>> > >>>> > >>>> > >>>> >>> >>> >>> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >>> >>>> > File views/controls/textfield/native_textfield_gtk.cc (right): >>>> > >>>> > >>>> > >>>> >>> >>> >>> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >>> >>>> > views/controls/textfield/native_textfield_gtk.cc:440: >>>> > g_signal_connect_after(widget, "key-press-event", >>>> > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: >>>> > >>>> >> On 2011/01/17 18:35:16, James Su wrote: >>>> >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: >>>> >> > > Changing g_signal_connect_after to g_signal_connect basically >>>> >> >>>> > broke >>>> > >>>> >> > > OnKeyPressEvent functionality for edit boxes. Almost no events >>>> now >>>> >> >>>> > delivered >>>> > >>>> >> > to >>>> >> > > OnKeyPressEvent because default processing handles most of them. >>>> >> >>>> > In >>>> > >>>> >> particular >>>> >> > > I'm fixing >>>> >> >>>> > http://code.google.com/p/chromium-os/issues/detail?id=10958 that >>>> > >>>> >> > > happened due to this change. There is another issue >>>> >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. >>>> >> > > >>>> >> > > I think this change should be reverted to repair OnKeyPressEvent >>>> >> > functionality. >>>> >> > We should never intercept key events before being handled by the >>>> >> >>>> > native >>>> > >>>> >> control. >>>> >> > Otherwise the functionalities of the native control and the input >>>> >> >>>> > method may >>>> > >>>> >> be >>>> >> > broken. For each key event needs intercepting, we need to handle it >>>> >> >>>> > like what >>>> > >>>> >> we >>>> >> > have done for Enter key in OnAcivateHandler(). For example, we need >>>> >> >>>> > to handle >>>> > >>>> >> > arrow keys in "move-cursor" signal handler and Tab key in >>>> >> >>>> > "move-focus" signal >>>> > >>>> >> > handler, etc. >>>> >> >>>> > >>>> > What will happen on Windows? Is it possible to write platform >>>> >> >>>> > independent code >>>> > >>>> >> if different events are delivered to handler? Should we remove >>>> >> >>>> > OnKeyPressEvent >>>> > >>>> >> because it doesn't work anymore (no events are delivered)? >>>> >> >>>> > Windows has different event flow, which will always dispatch an key >>>> > event to the input method, so things like OnKeyPressEvent and >>>> > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key >>>> > events are dispatched to the input method by the widget itself, so we >>>> > may break the input method badly if we intercept key events before the >>>> > widget. >>>> > So it's impossible for us to the exact same code for these two >>>> > platforms. But we can abstract and share at least part of code between >>>> > them. Like what we have done in above OnActivate() method, we can use >>>> > such kind of trick to deal with the platform difference and put shared >>>> > key event handling logic in controller's HandleKeystroke() method. >>>> > >>>> > >>>> > http://codereview.chromium.org/5254011/ >>>> > >>>> >>> >>> This is FYI. We're working on gtk less chrome, primarily for touch, but >>>> also >>>> for future version of chromeos (schedule is TBD). >>>> Replacement for chromeos won't happen anytime soon, but hopefully we can >>>> address this issue sometime in the future. >>>> >>> >>> - oshima >>>> >>> >>> Thanks for your information. I noticed that you are working on a native >>> textfield implementation based on Views. I'm wondering is there any >>> documentation/web site/issue to trace the progress of this effort? >> >> >> No, not really. Is there any specific thing you want to know about? >> > I'm just wondering how to integrate with input method framework. I'm > currently designing and implementing a new input method framework and hope > it can replace ibus in the future. > IME support is currently low priority. Since we control most of input path, we should have more flexibility to design it the way we want though (possibly handle IME event before sending to textfield, like win?) - oshima > > >> >> - oshima >> >>> >>> >>> >>> http://codereview.chromium.org/5254011/ >>> >> >> >
在 2011年1月19日 下午12:02，oshima <oshima@chromium.org>写道： > > > On Tue, Jan 18, 2011 at 9:23 PM, James Su <suzhe@chromium.org> wrote: > >> >> >> 在 2011年1月18日 下午9:19，oshima <oshima@chromium.org>写道： >> >> >>> >>> On Tue, Jan 18, 2011 at 12:31 PM, <suzhe@chromium.org> wrote: >>> >>>> On 2011/01/18 20:19:07, oshima wrote: >>>> >>>> On Mon, Jan 17, 2011 at 1:25 PM, <mailto:suzhe@chromium.org> wrote: >>>>> >>>> >>>> > >>>>> > >>>>> > >>>>> >>>> >>>> >>>> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >>>> >>>>> > File views/controls/textfield/native_textfield_gtk.cc (right): >>>>> > >>>>> > >>>>> > >>>>> >>>> >>>> >>>> http://codereview.chromium.org/5254011/diff/24001/views/controls/textfield/na... >>>> >>>>> > views/controls/textfield/native_textfield_gtk.cc:440: >>>>> > g_signal_connect_after(widget, "key-press-event", >>>>> > On 2011/01/17 20:10:21, Dmitry Polukhin wrote: >>>>> > >>>>> >> On 2011/01/17 18:35:16, James Su wrote: >>>>> >> > On 2011/01/17 10:50:19, Dmitry Polukhin wrote: >>>>> >> > > Changing g_signal_connect_after to g_signal_connect basically >>>>> >> >>>>> > broke >>>>> > >>>>> >> > > OnKeyPressEvent functionality for edit boxes. Almost no events >>>>> now >>>>> >> >>>>> > delivered >>>>> > >>>>> >> > to >>>>> >> > > OnKeyPressEvent because default processing handles most of them. >>>>> >> >>>>> > In >>>>> > >>>>> >> particular >>>>> >> > > I'm fixing >>>>> >> >>>>> > http://code.google.com/p/chromium-os/issues/detail?id=10958 that >>>>> > >>>>> >> > > happened due to this change. There is another issue >>>>> >> > > http://code.google.com/p/chromium-os/issues/detail?id=10362. >>>>> >> > > >>>>> >> > > I think this change should be reverted to repair OnKeyPressEvent >>>>> >> > functionality. >>>>> >> > We should never intercept key events before being handled by the >>>>> >> >>>>> > native >>>>> > >>>>> >> control. >>>>> >> > Otherwise the functionalities of the native control and the input >>>>> >> >>>>> > method may >>>>> > >>>>> >> be >>>>> >> > broken. For each key event needs intercepting, we need to handle >>>>> it >>>>> >> >>>>> > like what >>>>> > >>>>> >> we >>>>> >> > have done for Enter key in OnAcivateHandler(). For example, we >>>>> need >>>>> >> >>>>> > to handle >>>>> > >>>>> >> > arrow keys in "move-cursor" signal handler and Tab key in >>>>> >> >>>>> > "move-focus" signal >>>>> > >>>>> >> > handler, etc. >>>>> >> >>>>> > >>>>> > What will happen on Windows? Is it possible to write platform >>>>> >> >>>>> > independent code >>>>> > >>>>> >> if different events are delivered to handler? Should we remove >>>>> >> >>>>> > OnKeyPressEvent >>>>> > >>>>> >> because it doesn't work anymore (no events are delivered)? >>>>> >> >>>>> > Windows has different event flow, which will always dispatch an key >>>>> > event to the input method, so things like OnKeyPressEvent and >>>>> > SkipDefaultKeyEventProcessing work on Windows. But on Linux/Gtk, key >>>>> > events are dispatched to the input method by the widget itself, so we >>>>> > may break the input method badly if we intercept key events before >>>>> the >>>>> > widget. >>>>> > So it's impossible for us to the exact same code for these two >>>>> > platforms. But we can abstract and share at least part of code >>>>> between >>>>> > them. Like what we have done in above OnActivate() method, we can use >>>>> > such kind of trick to deal with the platform difference and put >>>>> shared >>>>> > key event handling logic in controller's HandleKeystroke() method. >>>>> > >>>>> > >>>>> > http://codereview.chromium.org/5254011/ >>>>> > >>>>> >>>> >>>> This is FYI. We're working on gtk less chrome, primarily for touch, but >>>>> also >>>>> for future version of chromeos (schedule is TBD). >>>>> Replacement for chromeos won't happen anytime soon, but hopefully we >>>>> can >>>>> address this issue sometime in the future. >>>>> >>>> >>>> - oshima >>>>> >>>> >>>> Thanks for your information. I noticed that you are working on a native >>>> textfield implementation based on Views. I'm wondering is there any >>>> documentation/web site/issue to trace the progress of this effort? >>> >>> >>> No, not really. Is there any specific thing you want to know about? >>> >> I'm just wondering how to integrate with input method framework. I'm >> currently designing and implementing a new input method framework and hope >> it can replace ibus in the future. >> > > IME support is currently low priority. Since we control most of input > path, we should have more flexibility > to design it the way we want though (possibly handle IME event before > sending to textfield, like win?) > The only thing I'm concerning is: as IME becomes more and more complicated, and as input method engines run in separated processes, we probably need to support calling IME in async mode to avoid blocking the UI process. In other words, we may need to send a key event to the IME asynchronously and continue to process the key event until we receive result from the IME. This change may affect our keyboard event processing flow dramatically. Btw, Android system is a good example for this kind of async keyboard event model. > - oshima > > > >> >> >>> >>> - oshima >>> >>>> >>>> >>>> >>>> http://codereview.chromium.org/5254011/ >>>> >>> >>> >> > |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
