Should set the modifiers for web mouse event on Mac.

Should set the modifiers for web mouse event on Mac.

We should add the mouse modifiers when creating web mouse event from NSEvent.
The modifiers can be used for MouseEvent.buttons attribute.

BUG=440483
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=187089

[zino, 2014-12-11 16:10:12]

jinho.bang@samsung.com changed reviewers:
+ haraken@chromium.org, rbyers@chromium.org

[zino, 2014-12-11 16:21:34]

PTAL.

I think we need a test to create web mouse event from NSEvent.
However, the method ([NSEvent pressedMouseButtons]) is class(or static?) method
and readonly.
So, I couldn't write a unittest. but I did test manually.
(using http://www.rbyers.net/eventTest.html)

[haraken, 2014-12-11 16:24:14]

https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
File Source/web/WebInputEventFactoryMac.mm (right):

https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
Source/web/WebInputEventFactoryMac.mm:725: if ([NSEvent pressedMouseButtons] & 1
<< 0)

Don't we want to have a constant variable for 1 << 0? At least, let's add '('
and ')' around 1 << 0.

[zino, 2014-12-11 17:01:28]

Please take a look.

Thank you. :)

https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
File Source/web/WebInputEventFactoryMac.mm (right):

https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
Source/web/WebInputEventFactoryMac.mm:725: if ([NSEvent pressedMouseButtons] & 1
<< 0)
On 2014/12/11 16:24:13, haraken wrote:
> 
> Don't we want to have a constant variable for 1 << 0? At least, let's add '('
> and ')' around 1 << 0.

Done.

IMHO, it is better to use constant values here than constant variables.
Because the values will be used only here.
I added some comments instead.

[zino, 2014-12-11 17:04:59]

Patchset #3 (id:40001) has been deleted

[haraken, 2014-12-11 23:28:53]

On 2014/12/11 17:01:28, zino wrote:
> Please take a look.
> 
> Thank you. :)
> 
>
https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
> File Source/web/WebInputEventFactoryMac.mm (right):
> 
>
https://codereview.chromium.org/788173002/diff/20001/Source/web/WebInputEvent...
> Source/web/WebInputEventFactoryMac.mm:725: if ([NSEvent pressedMouseButtons] &
1
> << 0)
> On 2014/12/11 16:24:13, haraken wrote:
> > 
> > Don't we want to have a constant variable for 1 << 0? At least, let's add
'('
> > and ')' around 1 << 0.
> 
> Done.
> 
> IMHO, it is better to use constant values here than constant variables.
> Because the values will be used only here.
> I added some comments instead.

LGTM

[Rick Byers, 2014-12-12 21:16:31]

LGTM

Note that there is Source/web/tests/WebInputEventFactoryTestMac.mm for unit
tests for this file.  But I've previously argued against adding new tests when
making trivial changes to this file (https://codereview.chromium.org/184983003).
 So this is fine with me without a test.

[zino, 2014-12-13 02:44:17]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-12-13 02:45:21]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/788173002/60001

[commit-bot: I haz the power, 2014-12-13 04:48:55]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-13 04:48:56]

Try jobs failed on following builders:
  win_blink_rel on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/win_blink_rel/builds/41458)

[zino, 2014-12-13 05:33:22]

The CQ bit was checked by jinho.bang@samsung.com

[commit-bot: I haz the power, 2014-12-13 05:34:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/788173002/60001

[commit-bot: I haz the power, 2014-12-13 07:49:00]

Committed patchset #3 (id:60001) as
https://src.chromium.org/viewvc/blink?view=rev&revision=187089