Implement MouseEvent buttons attribute.

Source/web/WebInputEventConversion.cpp

 /*
  * Copyright (C) 2009 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 77 matching lines @@
 
 PlatformMouseEventBuilder::PlatformMouseEventBuilder(Widget* widget, const WebMouseEvent& e)
 {
     // FIXME: Widget is always toplevel, unless it's a popup. We may be able
     // to get rid of this once we abstract popups into a WebKit API.
     m_position = widget->convertFromContainingWindow(flooredIntPoint(convertHitPointToWindow(widget, IntPoint(e.x, e.y))));
     m_globalPosition = IntPoint(e.globalX, e.globalY);
     m_movementDelta = IntPoint(scaleDeltaToWindow(widget, e.movementX), scaleDeltaToWindow(widget, e.movementY));
     m_button = static_cast<MouseButton>(e.button);
 
+    m_buttons = 0;
+    if (e.modifiers & WebInputEvent::LeftButtonDown)

[Rick Byers, 2014/11/14 19:48:09]

Please extend the unit tests in WebInputEventConversionTest.cpp to cover a
couple buttons cases also.

[Rick Byers, 2014/11/14 19:48:09]

Oh cool, I didn't realize this data was already present in the WebInputEvent
modifiers.  Great!

What platforms have (or will) you be able to validate that this is already set
properly on?  It looks like there's not much relying on these modifiers today,
right?  I'm just wondering if it might not be universally supported across
platforms.  That would be a chromium bug, and I'm OK landing the blink side
before worrying too much about this but we shouldn't ship the API to beta
without having validated all the platforms set the modifiers properly.

[zino, 2014/11/18 14:19:35]

Windows / Linux / Mac works well via layout tests on build bot.
Android works well also. I've confirmed it manually.
I couldn't check it in IOS.

[zino, 2014/11/18 14:19:35]

I don't think we may need it in new implementation.

[Rick Byers, 2014/11/18 20:29:18]

I guess your changes in WebInputConversion.cpp are covered by your layout test,
so perhaps that's enough.  Is that what you mean?  WebInputConversionTest.cpp
does add unit-test coverage for much of the logic in WebInputConversion.cpp.

[Rick Byers, 2014/11/18 20:29:18]

The behavior I'm worried about here isn't covered by the LayoutTest - it's
whether chromium is setting the modifiers correctly.  Arguably we should look
for or add some test coverage in chromium for this.

Can you please check manually on Linux that your behavior matches Firefox?
Great, thanks.  Verifying Android and Linux is good enough for me, especially
since you have the RuntimeEnabledFeature.  Once it lands, I'll manually verify
on Windows, Mac and ChromeOS canary builds before we flip the
RuntimeEnabledFeature to stable.
iOS doesn't use blink so not relevant here.

[zino, 2014/11/24 13:07:07]

The modifiers value is passed from ui::EventFlagsFromNative() which is
implemented on each platform.
X(Chrome OS and Linux) and Cocoa(Mac) have already unit tests for it.
 - X : src/ui/events/x/events_x_unittest.cc
 - Cocoa : src/ui/events/cocoa/events_mac_unittest.mm

Windows doesn't have a unit test but I'm not sure whether it is really
necessary. (for |buttons| attribute)
IMHO, we will have to do test manually to check whether each platform sets the
modifiers correctly.
If not, we will use simulated value instead of real value from system call and
it will not be meaningful.
(I mean eventSender is enough.)

For example, please see EventFlagsFromNative test in events_mac_unittest.mm
https://code.google.com/p/chromium/codesearch#chromium/src/ui/events/cocoa/ev...

According to the codes, using cocoa_test_event_utils::MouseEventWithType() to
make a native mouse event and then checking its value via
EventFlagsFromNative().
However, we know that EventFlagsFromNative() returns value which we passed to
MouseEventWithType(). In order words, the value is simulated value. (not real
value)
Therefore I think we are enough as eventSender and layout test.
Yep, I had already checked.
But there is a bit of difference between new patch set for chromium and firefox.
Because I've addressed mouse-up case per your comment. Other cases are the same.

[Rick Byers, 2014/11/25 17:44:27]

Sounds good, thanks for digging into this!
Perfect, thanks!

+        m_buttons |= 1;
+    if (e.modifiers & WebInputEvent::RightButtonDown)
+        m_buttons |= 2;
+    if (e.modifiers & WebInputEvent::MiddleButtonDown)
+        m_buttons |= 4;
+
     m_modifiers = 0;
     if (e.modifiers & WebInputEvent::ShiftKey)
         m_modifiers |= PlatformEvent::ShiftKey;
     if (e.modifiers & WebInputEvent::ControlKey)
         m_modifiers |= PlatformEvent::CtrlKey;
     if (e.modifiers & WebInputEvent::AltKey)
         m_modifiers |= PlatformEvent::AltKey;
     if (e.modifiers & WebInputEvent::MetaKey)
         m_modifiers |= PlatformEvent::MetaKey;
 
@@ skipping 706 matching lines @@
     modifiers = getWebInputModifiers(event);
 
     globalX = event.screenX();
     globalY = event.screenY();
     IntPoint localPoint = convertAbsoluteLocationForRenderObject(event.absoluteLocation(), *renderObject);
     x = localPoint.x();
     y = localPoint.y();
 }
 
 } // namespace blink