aura-x11: Add support for xmodmap'ed pointer buttons.

aura-x11: Add support for xmodmap'ed pointer buttons.

BUG=106305
TEST=manually

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

[Daniel Erat, 2011-12-04 15:51:45]

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc
File ui/base/x/events_x.cc (right):

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcode78
ui/base/x/events_x.cc:78: count = XGetPointerMapping(ui::GetXDisplay(), map,
arraysize(map));
i think that you need to refresh the cached copy of the mapping whenever chrome
receives a MappingNotify event with a request of MappingPointer:
http://tronche.com/gui/x/xlib/events/window-state-change/mapping.html

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcode87
ui/base/x/events_x.cc:87: GetMappedButton(i) : i;
nit: indent one less space?

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcod...
ui/base/x/events_x.cc:304: GetMappedButton(button) : button;
nit: indent one less space?

[Daniel Erat, 2011-12-07 16:32:24]

Ping?  (I can take this change over if you're busy with other stuff.)

[sadrul, 2011-12-07 16:36:12]

Ugh, sorry. I notice that I left a comment and didn't publish it.

I will work on updating the mapping from MappingNotify event.

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc
File ui/base/x/events_x.cc (right):

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcode78
ui/base/x/events_x.cc:78: count = XGetPointerMapping(ui::GetXDisplay(), map,
arraysize(map));
On 2011/12/04 15:51:45, Daniel Erat wrote:
> i think that you need to refresh the cached copy of the mapping whenever
chrome
> receives a MappingNotify event with a request of MappingPointer:
> http://tronche.com/gui/x/xlib/events/window-state-change/mapping.html

Indeed. I thought about it, and assumed perhaps this isn't necessary (in light
of http://codereview.chromium.org/7019007/).

[Daniel Erat, 2011-12-07 16:39:29]

On Wed, Dec 7, 2011 at 08:36,  <sadrul@chromium.org> wrote:
> Ugh, sorry. I notice that I left a comment and didn't publish it.
>
> I will work on updating the mapping from MappingNotify event.
>
>
>
> http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc
> File ui/base/x/events_x.cc (right):
>
>
http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcode78
> ui/base/x/events_x.cc:78: count = XGetPointerMapping(ui::GetXDisplay(),
> map, arraysize(map));
> On 2011/12/04 15:51:45, Daniel Erat wrote:
>>
>> i think that you need to refresh the cached copy of the mapping
>
> whenever chrome
>>
>> receives a MappingNotify event with a request of MappingPointer:
>> http://tronche.com/gui/x/xlib/events/window-state-change/mapping.html
>
>
> Indeed. I thought about it, and assumed perhaps this isn't necessary (in
> light of http://codereview.chromium.org/7019007/).

Wow, that's an old change. :-)  But yeah, it'd be better to have
something around the level of aura handle this instead of having it
live deep in a Chrome-OS-specific class.

> http://codereview.chromium.org/8793002/

[sadrul, 2011-12-07 17:06:47]

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc
File ui/base/x/events_x.cc (right):

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcode87
ui/base/x/events_x.cc:87: GetMappedButton(i) : i;
On 2011/12/04 15:51:45, Daniel Erat wrote:
> nit: indent one less space?

Done.

http://codereview.chromium.org/8793002/diff/3001/ui/base/x/events_x.cc#newcod...
ui/base/x/events_x.cc:304: GetMappedButton(button) : button;
On 2011/12/04 15:51:45, Daniel Erat wrote:
> nit: indent one less space?

Done.

http://codereview.chromium.org/8793002/diff/13002/ui/base/x/x11_util.h
File ui/base/x/x11_util.h (right):

http://codereview.chromium.org/8793002/diff/13002/ui/base/x/x11_util.h#newcod...
ui/base/x/x11_util.h:232: int GetMappedButton(int button);
This is deliberately not exported since it's not used outside of ui at the
moment.

[sadrul, 2011-12-07 17:23:44]

I have tested both with and without XI2, and this works correctly.

[Daniel Erat, 2011-12-07 17:31:42]

LGTM.  Thanks!

[sky, 2011-12-07 17:34:40]

LGTM