Do not use ui::GetCharacterFromXEvent when Control is pressed.

Do not use ui::GetCharacterFromXEvent when Control is pressed.

The function does not take into account the modifier.

BUG=107837
TEST=see the bug


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=114916

[James Su, 2011-12-16 09:45:08]

On 2011/12/16 07:39:14, Yusuke Sato wrote:

I'm wondering if the original pre-aura views only code is correct or not? If
it's correct, why?

[Yusuke Sato, 2011-12-16 09:48:56]

On 2011/12/16 09:45:08, James Su wrote:
> On 2011/12/16 07:39:14, Yusuke Sato wrote:
> 
> I'm wondering if the original pre-aura views only code is correct or not? If
> it's correct, why?

The original Aura code (i.e. w/o IME support) was correct because it sent a Char
event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
content/browser/renderer_host/web_input_event_aurax11.cc).

See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for details.

[James Su, 2011-12-16 09:53:23]

On 2011/12/16 09:48:56, Yusuke Sato wrote:
> On 2011/12/16 09:45:08, James Su wrote:
> > On 2011/12/16 07:39:14, Yusuke Sato wrote:
> > 
> > I'm wondering if the original pre-aura views only code is correct or not? If
> > it's correct, why?
> 
> The original Aura code (i.e. w/o IME support) was correct because it sent a
Char
> event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
> content/browser/renderer_host/web_input_event_aurax11.cc).
> 
> See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for details.

I'm asking the pre-aura views only code using views::InputMethodIBus and
RenderWidgetHostViewViews.

[Yusuke Sato, 2011-12-16 10:01:44]

I guess that they had the same problem since ui/views/events/event_x.cc used the
same logic, but right now I don't have a way to confirm since RWHVV is already
gone.

Do you remember if there was a trybot which ran interactive_ui_tests using RWHVV
and views::MockInputMethod?

On 2011/12/16 09:53:23, James Su wrote:
> On 2011/12/16 09:48:56, Yusuke Sato wrote:
> > On 2011/12/16 09:45:08, James Su wrote:
> > > On 2011/12/16 07:39:14, Yusuke Sato wrote:
> > > 
> > > I'm wondering if the original pre-aura views only code is correct or not?
If
> > > it's correct, why?
> > 
> > The original Aura code (i.e. w/o IME support) was correct because it sent a
> Char
> > event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
> > content/browser/renderer_host/web_input_event_aurax11.cc).
> > 
> > See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for
details.
> 
> I'm asking the pre-aura views only code using views::InputMethodIBus and
> RenderWidgetHostViewViews.

[James Su, 2011-12-16 10:13:04]

Ok, I remembered that the behavior of X and Windows for handling ctrl- keys is
different on only Enter key :). I didn't know if there was a such trybot for
RWHVV.

Anyway this CL looks good to me. Just please make sure it works for all ctrl-
keys and different keyboard layouts.

On 2011/12/16 10:01:44, Yusuke Sato wrote:
> I guess that they had the same problem since ui/views/events/event_x.cc used
the
> same logic, but right now I don't have a way to confirm since RWHVV is already
> gone.
> 
> Do you remember if there was a trybot which ran interactive_ui_tests using
RWHVV
> and views::MockInputMethod?
> 
> On 2011/12/16 09:53:23, James Su wrote:
> > On 2011/12/16 09:48:56, Yusuke Sato wrote:
> > > On 2011/12/16 09:45:08, James Su wrote:
> > > > On 2011/12/16 07:39:14, Yusuke Sato wrote:
> > > > 
> > > > I'm wondering if the original pre-aura views only code is correct or
not?
> If
> > > > it's correct, why?
> > > 
> > > The original Aura code (i.e. w/o IME support) was correct because it sent
a
> > Char
> > > event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
> > > content/browser/renderer_host/web_input_event_aurax11.cc).
> > > 
> > > See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for
> details.
> > 
> > I'm asking the pre-aura views only code using views::InputMethodIBus and
> > RenderWidgetHostViewViews.

[James Su, 2011-12-16 10:13:54]

LGTM to make Rietveld happy.
On 2011/12/16 10:13:04, James Su wrote:
> Ok, I remembered that the behavior of X and Windows for handling ctrl- keys is
> different on only Enter key :). I didn't know if there was a such trybot for
> RWHVV.
> 
> Anyway this CL looks good to me. Just please make sure it works for all ctrl-
> keys and different keyboard layouts.
> 
> On 2011/12/16 10:01:44, Yusuke Sato wrote:
> > I guess that they had the same problem since ui/views/events/event_x.cc used
> the
> > same logic, but right now I don't have a way to confirm since RWHVV is
already
> > gone.
> > 
> > Do you remember if there was a trybot which ran interactive_ui_tests using
> RWHVV
> > and views::MockInputMethod?
> > 
> > On 2011/12/16 09:53:23, James Su wrote:
> > > On 2011/12/16 09:48:56, Yusuke Sato wrote:
> > > > On 2011/12/16 09:45:08, James Su wrote:
> > > > > On 2011/12/16 07:39:14, Yusuke Sato wrote:
> > > > > 
> > > > > I'm wondering if the original pre-aura views only code is correct or
> not?
> > If
> > > > > it's correct, why?
> > > > 
> > > > The original Aura code (i.e. w/o IME support) was correct because it
sent
> a
> > > Char
> > > > event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
> > > > content/browser/renderer_host/web_input_event_aurax11.cc).
> > > > 
> > > > See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for
> > details.
> > > 
> > > I'm asking the pre-aura views only code using views::InputMethodIBus and
> > > RenderWidgetHostViewViews.

[Peng, 2011-12-16 16:18:47]

LGTM. And I added a comment in bug tracker. It is better to check it later.

On 2011/12/16 10:13:54, James Su wrote:
> LGTM to make Rietveld happy.
> On 2011/12/16 10:13:04, James Su wrote:
> > Ok, I remembered that the behavior of X and Windows for handling ctrl- keys
is
> > different on only Enter key :). I didn't know if there was a such trybot for
> > RWHVV.
> > 
> > Anyway this CL looks good to me. Just please make sure it works for all
ctrl-
> > keys and different keyboard layouts.
> > 
> > On 2011/12/16 10:01:44, Yusuke Sato wrote:
> > > I guess that they had the same problem since ui/views/events/event_x.cc
used
> > the
> > > same logic, but right now I don't have a way to confirm since RWHVV is
> already
> > > gone.
> > > 
> > > Do you remember if there was a trybot which ran interactive_ui_tests using
> > RWHVV
> > > and views::MockInputMethod?
> > > 
> > > On 2011/12/16 09:53:23, James Su wrote:
> > > > On 2011/12/16 09:48:56, Yusuke Sato wrote:
> > > > > On 2011/12/16 09:45:08, James Su wrote:
> > > > > > On 2011/12/16 07:39:14, Yusuke Sato wrote:
> > > > > > 
> > > > > > I'm wondering if the original pre-aura views only code is correct or
> > not?
> > > If
> > > > > > it's correct, why?
> > > > > 
> > > > > The original Aura code (i.e. w/o IME support) was correct because it
> sent
> > a
> > > > Char
> > > > > event in a different way (using MakeWebKeyboardEventFromAuraEvent() in
> > > > > content/browser/renderer_host/web_input_event_aurax11.cc).
> > > > > 
> > > > > See http://code.google.com/p/chromium/issues/detail?id=107837#c1 for
> > > details.
> > > > 
> > > > I'm asking the pre-aura views only code using views::InputMethodIBus and
> > > > RenderWidgetHostViewViews.

[commit-bot: I haz the power, 2011-12-17 07:04:34]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/yusukes@chromium.org/8964011/1

[commit-bot: I haz the power, 2011-12-17 08:47:51]

Change committed as 114916