Closed Bug 547996 Opened 11 years ago Closed 11 years ago

Be able to tell when a click was generated by a tap on the screen

Categories

(Core :: DOM: Events, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: Felipe, Assigned: Felipe)

References

Details

(Keywords: dev-doc-complete)

Attachments

(2 files, 5 obsolete files)

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.
Blocks: 547997
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.
Blocks: 548100
Assignee: nobody → felipc
Summary: Be able to tell when a click was generate by a tap on the screen → Be able to tell when a click was generated by a tap on the screen
Attached patch V1 - mozInputSource (obsolete) — Splinter Review
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.
Attached patch v1.1 - mozInputSource + test (obsolete) — Splinter Review
Now with tests
Attachment #430369 - Attachment is obsolete: true
Attached patch v2 - mozInputSource (obsolete) — Splinter Review
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.
Attachment #430636 - Attachment is obsolete: true
Attachment #431289 - Flags: review?
Attachment #431289 - Flags: review? → review?(Olli.Pettay)
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.
Attachment #431289 - Flags: review?(Olli.Pettay) → review-
>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.
Attachment #431289 - Attachment is obsolete: true
Attachment #431792 - Flags: review?(Olli.Pettay)
Comment on attachment 431792 [details] [diff] [review]
v3 - mozInputSource

Someone needs to look at the windows specific bits.
Attachment #431792 - Flags: review?(Olli.Pettay) → review+
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.
Attachment #431792 - Flags: review?(jmathies)
+#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.
Attachment #431792 - Flags: review?(jmathies) → review+
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.
Attachment #431792 - Flags: superreview?(jst)
Comment on attachment 431792 [details] [diff] [review]
v3 - mozInputSource

Looks good.
Attachment #431792 - Flags: superreview?(jst) → superreview+
This patch doesn't apply cleanly.
Keywords: checkin-needed
Attached patch v3 - mozInputSource - rebased (obsolete) — Splinter Review
>This patch doesn't apply cleanly.

Ah ok, rebased today
Attached patch v3 - mozInputSource rebased (obsolete) — Splinter Review
just rebased again
Attachment #433929 - Attachment is obsolete: true
rebased on today's tip
Attachment #435954 - Attachment is obsolete: true
http://hg.mozilla.org/mozilla-central/rev/13cfdaebbd40
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Depends on: 586961
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...
You need to log in before you can comment on or make changes to this bug.