We should have a flag on the click event, or an extra event, (or some other alternative?) so that we (and webapps) have the possibility to tell if a click event was generated by a regular mouse click or a tap on the screen (using touch or a pen). This is useful for apps that want to handle different behaviors for clicks and taps, for example to take into account the less precise nature of a tap.

nsIDOMNSMouseEvent could have some property and consts for "input method". Possible values (nor sure about the naming) could be at least mouse, touchscreen, keyboard, pen.

This patch adds attribute mozInputSource to DOMNSMouseEvent, with the possible values of: eSourceMouse, eSourcePen, eSourceEraser, eSourceCursor, eSourceTouch, eSourceKeyboard The first 4 values of this list consist from the values of GDK (http://library.gnome.org/devel/gdk/unstable/gdk-Input-Devices.html#GdkInputSource), and I added Touch and Keyboard (for accesskeys, right?) per your suggestion. I'm just not sure on what to do about nsXULElement and some nsHTML* elements that have a Click method which is called from PerformAccesskey. Should I add a parameter to the function call to differentiate a click from the mouse or generated by an access key, or this leave them as is (ignoring this attribute for them?). nsGenericElement and nsHTMLGenericElement are fine. This patch is basically complete; after hearing your thoughts on the above question I can resubmit a patch with tests and ask for review, and find someone to review the Windows' bits too.

Now with tests

Patch completed. This adds the attribute mozInputSource to nsIDOMNSMouseEvent. It's able to set the value for 'Touch' or 'Pen' in win7 and vista, and the value for 'Keyboard' in all platforms. Values for all mouse events are set whenever possible, not only click. 'Keyboard' is set whenever a click is generated through an accesskey or by pressing the space bar or Enter in a button or a link, for example. WRT the patch, I'm just not sure if the changes in nsXULElement.cpp/.h are worth it. They are needed only to support accesskey in XUL elements (the spacebar/enter key works independently of it). After I get the content/events parts reviewed I'll request review for the widget part. WRT the windows part, I don't know if the call to GetMessageExtraInfo is expensive. If it is we can remove it for NS_MOUSE_MOVE, and keep it only for mouseup/mousedown which is necessary to set the value for NS_MOUSE_CLICK in the ESM.

Comment on attachment 431289 [details] [diff] [review] v2 - mozInputSource >diff --git a/content/events/src/nsDOMEvent.cpp b/content/events/src/nsDOMEvent.cpp >--- a/content/events/src/nsDOMEvent.cpp >+++ b/content/events/src/nsDOMEvent.cpp >@@ -846,16 +846,17 @@ NS_METHOD nsDOMEvent::DuplicatePrivateDa > NS_ENSURE_TRUE(mouseEvent, NS_ERROR_OUT_OF_MEMORY); > isInputEvent = PR_TRUE; > mouseEvent->clickCount = oldMouseEvent->clickCount; > mouseEvent->acceptActivation = oldMouseEvent->acceptActivation; > mouseEvent->context = oldMouseEvent->context; > mouseEvent->relatedTarget = oldMouseEvent->relatedTarget; > mouseEvent->button = oldMouseEvent->button; > mouseEvent->pressure = oldMouseEvent->pressure; >+ mouseEvent->inputSource = oldMouseEvent->inputSource; > newEvent = mouseEvent; > break; > } What about drag and mousescroll events? >+<script type="application/javascript"> >+ >+/** Test for Bug 547996 **/ >+/* mouseEvent.mozInputSource attribute */ >+ >+const INPUT_SOURCE_MOUSE = 0; >+const INPUT_SOURCE_KEYBOARD = 5; Couldn't you get these values from the interface? Components.interfaces.nsIDOMNSMouseEvent.SOURCE_MOUSE > NS_IMETHODIMP > nsXULElement::Click() > { >+ return ClickWithInputSource(nsMouseEvent::eSourceMouse); >+} I think for click() there should be SOURCE_UNKNOWN. >+++ b/content/xul/content/src/nsXULElement.h >@@ -518,16 +518,17 @@ public: > virtual void List(FILE* out, PRInt32 aIndent) const; > virtual void DumpContent(FILE* out, PRInt32 aIndent,PRBool aDumpAll) const > { > } > #endif > > virtual void PerformAccesskey(PRBool aKeyCausesActivation, > PRBool aIsTrustedEvent); >+ NS_IMETHOD ClickWithInputSource(PRUint16 aInputSource); This should be just nsresult ClickWithInputSource(PRUint16 aInputSource); >-[scriptable, uuid(1b8e528d-7dca-44ee-8ee6-c44594ebcef1)] >+[scriptable, uuid(f5dd5fbb-f4ff-4277-819c-f31aa1dafc32)] > interface nsIDOMNSMouseEvent : nsIDOMMouseEvent > { > // Finger or touch pressure event value > // ranges between 0.0 and 1.0 > readonly attribute float mozPressure; > >+ const unsigned short SOURCE_MOUSE = 0; >+ const unsigned short SOURCE_PEN = 1; >+ const unsigned short SOURCE_ERASER = 2; >+ const unsigned short SOURCE_CURSOR = 3; >+ const unsigned short SOURCE_TOUCH = 4; >+ const unsigned short SOURCE_KEYBOARD = 5; I'd add SOURCE_UNKNOWN, and perhaps that could be the value 0. Are the const values defined here visible when enumerating mouse event properties? If yes, then perhaps they should have MOZ_SOURCE_ prefix. > class nsMouseEvent_base : public nsInputEvent > { > public: > nsMouseEvent_base(PRBool isTrusted, PRUint32 msg, nsIWidget *w, PRUint8 type) >- : nsInputEvent(isTrusted, msg, w, type), button(0), pressure(0) {} >+ : nsInputEvent(isTrusted, msg, w, type), button(0), pressure(0), inputSource(0) {} If SOURCE_UNKNOWN is added, perhaps inputSource could get the value SOURCE_MOUSE here. Then you wouldn't need to change nsMouseEvent usage in non-windows widget code. Though, I'm not sure what the value should be for events which are created by a script. Perhaps it should be UNKNOWN. That could be set in nsDOMMouseEvent/nsDOMMouseScrollEvent/nsDOMDragEvent if aEvent to the ctor is null. > > /// The possible related target > nsCOMPtr<nsISupports> relatedTarget; > > PRInt16 button; > > // Finger or touch pressure of event > // ranges between 0.0 and 1.0 > float pressure; >+ >+ // Mouse, keyboard, pen or touch I'd rather point to nsIDOMNSMouseEvent here. >+ PRUint16 inputSource; > }; > > class nsMouseEvent : public nsMouseEvent_base > { > public: > enum buttonType { eLeftButton = 0, eMiddleButton = 1, eRightButton = 2 }; > enum reasonType { eReal, eSynthesized }; > enum contextType { eNormal, eContextMenuKey }; > enum exitType { eChild, eTopLevel }; >+ enum inputSourceType { eSourceMouse = 0, eSourcePen, eSourceEraser, >+ eSourceCursor, eSourceTouch, eSourceKeyboard }; Would it be possible to use the consts from nsIDOMNSMouseEvent everywhere, so that there wasn't need for this enum? Jimm needs to review the windows/ part.

>What about drag and mousescroll events? Added now all the handling to DOMMouseScrollEvent, DOMSimpleGestureEvent and DOMDragEvent, which are the events that inherit from MouseEvent >Couldn't you get these values from the interface? >Components.interfaces.nsIDOMNSMouseEvent.SOURCE_MOUSE Yes >I think for click() there should be SOURCE_UNKNOWN. >This should be just >nsresult ClickWithInputSource(PRUint16 aInputSource); done and done >I'd add SOURCE_UNKNOWN, and perhaps that could be the value 0. So I added MOZ_SOURCE_UNKNOWN with value 0. The base constructor for MouseEvent defaults it to MOZ_SOURCE_MOUSE (1), and from scripted Clicks from XULElement, HTMLButtonElement and HTMLInputElement we set it to UNKNOWN. >Though, I'm not sure what the value should be for events which are created by >a script. Perhaps it should be UNKNOWN. That could be set in >nsDOMMouseEvent/nsDOMMouseScrollEvent/nsDOMDragEvent if aEvent to the ctor >is null. Also setting to UNKOWN in this case >Are the const values defined here visible when enumerating mouse event >properties? >If yes, then perhaps they should have MOZ_SOURCE_ prefix. Yes they are, added the MOZ_ prefix >>+ // Mouse, keyboard, pen or touch >I'd rather point to nsIDOMNSMouseEvent here. Ok >>+ enum inputSourceType { eSourceMouse = 0, eSourcePen, eSourceEraser, >>+ eSourceCursor, eSourceTouch, eSourceKeyboard }; >Would it be possible to use the consts from nsIDOMNSMouseEvent everywhere, >so that there wasn't need for this enum? Yes, got rid of the enum, and used the consts directly from nsIDOMNSMouseEvent The patch is bigger because there was a lot of handling to add for the drag events. Also updated the tests to reflect these new changes.

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource Someone needs to look at the windows specific bits.

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource Jimm, can you take a look at the Windows changes here? It adds a call to GetMessageExtraInfo on mouse messages to get info about the input source and set the proper value for the mouse events.

+#define TABLET_INK_SIGNATURE 0xFFFFFF00 +#define TABLET_INK_CHECK 0xFF515700 +#define TABLET_INK_TOUCH 0x00000080 Do we have these if the win7 sdk is used? If so we should use the sdk values rather than our own.

It doesn't look like so. They are only described here: http://msdn.microsoft.com/en-us/library/ms703320%28VS.85%29.aspx and I didn't find other place on the docs saying that they're part of any sdk. I will investigate further and be back in a few

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource if you can find those and use them, that would be good.

Yeah, I grepped through the SDK headers and the values aren't there. Does this need a super-review?

Perhaps jst could sreview, since this includes API change.

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource jst, can you take a look at this patch? It adds attribute mozInputSource to nsIDOMNSMouseElement and make the related changes elsewhere on the code where we need to set this value.

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource Looks good.

This patch doesn't apply cleanly.

>This patch doesn't apply cleanly. Ah ok, rebased today

just rebased again

rebased on today's tip

Documented: https://developer.mozilla.org/en/DOM/event.mozInputSource And listed on both of these: https://developer.mozilla.org/en/DOM/event#Properties https://developer.mozilla.org/en/Firefox_4_for_developers#Miscellaneous_DOM_changes

Comment on attachment 431792 [details] [diff] [review] v3 - mozInputSource >- if (aKeyCausesActivation && tag != nsGkAtoms::textbox && tag != nsGkAtoms::menulist) >- elm->Click(); >+ if (aKeyCausesActivation && tag != nsGkAtoms::textbox && tag != nsGkAtoms::menulist) { >+ ClickWithInputSource(nsIDOMNSMouseEvent::MOZ_SOURCE_KEYBOARD); Oops. This calls ClickWithInputSource on the wrong element...