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

Source/core/events/MouseEvent.cpp

 /*
  * Copyright (C) 2001 Peter Kelly (pmk@post.com)
  * Copyright (C) 2001 Tobias Anton (anton@stud.fbi.fh-darmstadt.de)
  * Copyright (C) 2006 Samuel Weinig (sam.weinig@gmail.com)
  * Copyright (C) 2003, 2005, 2006, 2008 Apple Inc. All rights reserved.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Library General Public
  * License as published by the Free Software Foundation; either
  * version 2 of the License, or (at your option) any later version.
@@ skipping 42 matching lines @@
         event.movementDelta().x(), event.movementDelta().y(),
         event.ctrlKey(), event.altKey(), event.shiftKey(), event.metaKey(), event.button(),
         platformModifiersToButtons(event.modifiers()),
         relatedTarget, nullptr, false, event.syntheticEventType(), event.timestamp());
 }
 
 PassRefPtrWillBeRawPtr<MouseEvent> MouseEvent::create(const AtomicString& type, bool canBubble, bool cancelable, PassRefPtrWillBeRawPtr<AbstractView> view,
     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)
 {
     return adoptRefWillBeNoop(new MouseEvent(type, canBubble, cancelable, view,
         detail, screenX, screenY, windowX, windowY,
         movementX, movementY,
         ctrlKey, altKey, shiftKey, metaKey, button, buttons, relatedTarget, dataTransfer, isSimulated, syntheticEventType, uiCreateTime));
 }
 
 MouseEvent::MouseEvent()
     : m_button(0)
     , m_buttons(0)
     , m_buttonDown(false)
     , m_relatedTarget(nullptr)
     , m_dataTransfer(nullptr)
     , m_syntheticEventType(PlatformMouseEvent::RealOrIndistinguishable)
 {
 }
 
 MouseEvent::MouseEvent(const AtomicString& eventType, bool canBubble, bool cancelable, PassRefPtrWillBeRawPtr<AbstractView> view,
     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)
 {
     setUICreateTime(uiCreateTime);
 }
 
 MouseEvent::MouseEvent(const AtomicString& eventType, const MouseEventInit& initializer)
     : MouseRelatedEvent(eventType, initializer.bubbles(), initializer.cancelable(), initializer.view(), initializer.detail(), IntPoint(initializer.screenX(), initializer.screenY()),
         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)
 {
     initCoordinates(IntPoint(initializer.clientX(), initializer.clientY()));
 }
 
 MouseEvent::~MouseEvent()
 {
 }
 
 unsigned short MouseEvent::platformModifiersToButtons(unsigned modifiers)
 {
     unsigned short buttons = 0;
 
     if (modifiers & PlatformEvent::LeftButtonDown)
         buttons |= 1;
     if (modifiers & PlatformEvent::RightButtonDown)
         buttons |= 2;
     if (modifiers & PlatformEvent::MiddleButtonDown)
         buttons |= 4;
 
     return buttons;
 }
 
 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;
 
     if (scriptState && scriptState->world().isIsolatedWorld())
         UIEventWithKeyState::didCreateEventInIsolatedWorld(ctrlKey, altKey, shiftKey, metaKey);
 
     initUIEvent(type, canBubble, cancelable, view, detail);
 
     m_screenLocation = IntPoint(screenX, screenY);
     m_ctrlKey = ctrlKey;
     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));
 
     // FIXME: m_isSimulated is not set to false here.
     // FIXME: m_dataTransfer is not set to nullptr here.
 }
 
 const AtomicString& MouseEvent::interfaceName() const
 {
@@ skipping 129 matching lines @@
         event().button(), relatedTarget, event().buttons());
     if (event().defaultHandled())
         doubleClickEvent->setDefaultHandled();
     EventDispatcher::dispatchEvent(dispatcher.node(), MouseEventDispatchMediator::create(doubleClickEvent));
     if (doubleClickEvent->defaultHandled() || doubleClickEvent->defaultPrevented())
         return false;
     return !swallowEvent;
 }
 
 } // namespace blink