Change MouseEvent.button to be 'short' instead of 'unsigned short'

Source/core/events/MouseEvent.cpp

Index: Source/core/events/MouseEvent.cpp
diff --git a/Source/core/events/MouseEvent.cpp b/Source/core/events/MouseEvent.cpp
index 2ecccb43c3dd31e669da96c199b179628a2bab6b..12b14724518c203c7fd76c2f18415fd464b55183 100644
--- a/Source/core/events/MouseEvent.cpp
+++ b/Source/core/events/MouseEvent.cpp
@@ -60,7 +60,7 @@ PassRefPtrWillBeRawPtr<MouseEvent> MouseEvent::create(const AtomicString& type,
     int detail, int screenX, int screenY, int windowX, int windowY,
     int movementX, int movementY,
     bool ctrlKey, bool altKey, bool shiftKey, bool metaKey,
-    unsigned short button, unsigned short buttons,
+    short button, unsigned short buttons,
     PassRefPtrWillBeRawPtr<EventTarget> relatedTarget, DataTransfer* dataTransfer, bool isSimulated, PlatformMouseEvent::SyntheticEventType syntheticEventType,
     double uiCreateTime)
 {
@@ -84,16 +84,16 @@ MouseEvent::MouseEvent(const AtomicString& eventType, bool canBubble, bool cance
     int detail, int screenX, int screenY, int windowX, int windowY,
     int movementX, int movementY,
     bool ctrlKey, bool altKey, bool shiftKey, bool metaKey,
-    unsigned short button, unsigned short buttons, PassRefPtrWillBeRawPtr<EventTarget> relatedTarget,
+    short button, unsigned short buttons, PassRefPtrWillBeRawPtr<EventTarget> relatedTarget,
     DataTransfer* dataTransfer, bool isSimulated, PlatformMouseEvent::SyntheticEventType syntheticEventType,
     double uiCreateTime)
     : MouseRelatedEvent(eventType, canBubble, cancelable, view, detail, IntPoint(screenX, screenY),
         IntPoint(windowX, windowY),
         IntPoint(movementX, movementY),
         ctrlKey, altKey, shiftKey, metaKey, isSimulated)
-    , m_button(button == (unsigned short)-1 ? 0 : button)
+    , m_button(button == -1 ? 0 : button)

[philipj_slow, 2015/06/09 09:35:38]

Getting rid of these casts will be nice. However, I think that coercing -1 to 0
is wrong for the web-exposed ways of creating a MouseEvent, and Firefox doesn't
do it in this test:
http://jsconsole.com/?new%20MouseEvent%28%27click%27%2C{button%3A-1}%29.button

I tested IE, it doesn't have the MouseEvent contructor, but this also allows -1
to pass through:
var event = document.createEvent("MouseEvent");
event.initMouseEvent('click',false,false,null,0,0,0,0,0,false,false,false,false,-1,null);
alert(event.button);

However, strangely enough, passing -2 to IE results in button 65534, just like
Blink/WebKit.

I think the right thing to do here is to simply pass through the values as they
are, as the spec doesn't require any coercion AFAICT. You will need to check if
there are any internal uses reaching any of these code paths where (unsigned
short)-1 will be passed, by any name.
https://code.google.com/p/chromium/codesearch is a good place to dig into that
kind of question.

[ramya.v, 2015/06/09 11:14:31]

I've checked for any internal paths which could pass value of -1. Apart form
fakeMouseMoves from EventHandler.cpp, PopupMenuImpl::setValueAndClosePopup is
creating default PlatformMouseEvent constructor in which default value of button
is -1.
Apart from these I could not find any specific place where they are sending
explicit value of -1.

The check (unsigned short)-1?0:button might be added because negative values are
not allowed to be assigned to button since it is unsigned short. Now as you
suggested probably we can pass the values as they are similar to firefox
behavior.

[philipj_slow, 2015/06/09 12:04:06]

OK, it looks like the MouseButton enum member NoButton in PlatformMouseEvent.h
is the source of the -1 handling, and MouseEvent.button uses 0 to mean both
"left button" and "no button", which is also why there's an explicit
m_buttonDown member in MouseEvent, and some silly code in MouseEvent::which().

This would probably be less strange if we nuked the MouseButton in
PlatformMouseEvent and the *ButtonDown in PlatformEvent.h's Modifiers, replacing
both with

enum MouseButtons {
    NoButton = 0,
    LeftButton = 1,
    MiddleButton = 2,
    RightButton = 4
}

That matches https://w3c.github.io/uievents/#widl-MouseEvent-buttons

Then PlatformMouseEvent would have both button and buttons members in this form.
It would be the job of MouseEvent to convert this to the slightly wacky format
required to be web-exposed, where both NoButton and LeftButton are represented
by 0 in some contexts.

This is a bit invasive, so let's wait and see what Rick makes of this situation.

     , m_buttons(buttons)
-    , m_buttonDown(button != (unsigned short)-1)
+    , m_buttonDown(button != -1)
     , m_relatedTarget(relatedTarget)
     , m_dataTransfer(dataTransfer)
     , m_syntheticEventType(syntheticEventType)
@@ -106,9 +106,9 @@ MouseEvent::MouseEvent(const AtomicString& eventType, const MouseEventInit& init
         IntPoint(0 /* pageX */, 0 /* pageY */),
         IntPoint(initializer.movementX(), initializer.movementY()),
         initializer.ctrlKey(), initializer.altKey(), initializer.shiftKey(), initializer.metaKey(), false /* isSimulated */)
-    , m_button(initializer.button() == (unsigned short)-1 ? 0 : initializer.button())
+    , m_button(initializer.button() == -1 ? 0 : initializer.button())
     , m_buttons(initializer.buttons())
-    , m_buttonDown(initializer.button() != (unsigned short)-1)
+    , m_buttonDown(initializer.button() != -1)
     , m_relatedTarget(initializer.relatedTarget())
     , m_dataTransfer(nullptr)
     , m_syntheticEventType(PlatformMouseEvent::RealOrIndistinguishable)
@@ -137,7 +137,7 @@ unsigned short MouseEvent::platformModifiersToButtons(unsigned modifiers)
 void MouseEvent::initMouseEvent(ScriptState* scriptState, const AtomicString& type, bool canBubble, bool cancelable, PassRefPtrWillBeRawPtr<AbstractView> view,
                                 int detail, int screenX, int screenY, int clientX, int clientY,
                                 bool ctrlKey, bool altKey, bool shiftKey, bool metaKey,
-                                unsigned short button, PassRefPtrWillBeRawPtr<EventTarget> relatedTarget, unsigned short buttons)
+                                short button, PassRefPtrWillBeRawPtr<EventTarget> relatedTarget, unsigned short buttons)
 {
     if (dispatched())
         return;
@@ -152,9 +152,9 @@ void MouseEvent::initMouseEvent(ScriptState* scriptState, const AtomicString& ty
     m_altKey = altKey;
     m_shiftKey = shiftKey;
     m_metaKey = metaKey;
-    m_button = button == (unsigned short)-1 ? 0 : button;
+    m_button = button == -1 ? 0 : button;
     m_buttons = buttons;
-    m_buttonDown = button != (unsigned short)-1;
+    m_buttonDown = button != -1;
     m_relatedTarget = relatedTarget;
 
     initCoordinates(IntPoint(clientX, clientY));