774809 - [BrowserAPI] Add methods to send mouse/touch events to the content

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

To make it easy to handle panning from the embedding application it will be helpful to be able to intercept mouse/touch events and redirect only the one we don't want to consume.

Comment 1

[Justin Lebar (not reading bugmail)]

Are you going to take this one, Vivien, or do you want me/dale to look into it?

Comment 2

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Justin I have finally implement separated methods instead of a big unified one.
The main rationale behind that is because it was looking weird to have to initialize an event with a |window| parameter that's not the real window.
And for MouseEvent, relatedTarget does not make sense.

Comment 3

[Justin Lebar (not reading bugmail)]

Comment on attachment 643086 [details] [diff] [review]
Patch

>@@ -431,16 +433,36 @@ BrowserElementChild.prototype = {
>+  _recvSendMouseEvent: function(data) {
>+    let json = data.json;
>+    debug("Received sendMouseEvent message: (" + JSON.stringify(json) + ")");
>+    let utils = content.QueryInterface(Ci.nsIInterfaceRequestor)
>+                       .getInterface(Ci.nsIDOMWindowUtils);
>+    utils.sendMouseEvent(json.type, json.x, json.y, json.button,
>+                         json.clickCount, json.modifiers);
>+  },
>+
>+  _recvSendTouchEvent: function(data) {
>+    let json = data.json;
>+    debug("Received sendTouchEvent message: (" + JSON.stringify(json) + ")");

When the implementation of debug() is empty, we'll still evaluate the args and
do the stringify.  For most of the calls to debug(), this doesn't matter, but
I'm concerned it might here, because we can send a lot of touch events as we
drag (right?).  If so, can you comment out this debug call, to be safe?

>@@ -161,16 +162,20 @@ function BrowserElementParent(frameLoade
>   }
> 
>   function defineDOMRequestMethod(domName, msgName) {
>     XPCNativeWrapper.unwrap(self._frameElement)[domName] = self._sendDOMRequest.bind(self, msgName);
>   }
> 
>   // Define methods on the frame element.
>   defineMethod('setVisible', this._setVisible);
>+  defineMethod('sendMouseEvent', this._sendMouseEvent);
>+  if (Services.prefs.getBoolPref(TOUCH_EVENTS_ENABLED_PREF)) {
>+    defineMethod('sendTouchEvent', this._sendTouchEvent);
>+  }

We don't usually disappear API properties when they're not supported --
desktop machines still have mozBattery, for example.

What happens if you send a touch event at a button on a build that doesn't have
the touch events pref enabled?  Do we just ignore it?  I think we should try to
retain those semantics, whatever they are, if we can.

(I know I said I preferred exceptions rather than silently failing; it's my
mistake for not asking for more context.  Sorry!)

>+  _sendMouseEvent: function(type, x, y, button, clickCount, modifiers) {
>+    this._sendAsyncMsg("send-mouse-event", {
>+      "type": type,
>+      "x": x,
>+      "y": y,
>+      "button": button,
>+      "clickCount": clickCount,
>+      "modifiers": modifiers
>+    });
>+  },
>+
>+  _sendTouchEvent: function(type, identifiers, touchesX, touchesY,
>+                            radiisX, radiisY, rotationAngles, forces,
>+                            count, modifiers) {
>+    this._sendAsyncMsg("send-touch-event", {
>+      "type": type,
>+      "identifiers": identifiers,
>+      "touchesX": touchesX,
>+      "touchesY": touchesY,
>+      "radiisX": radiisX,
>+      "radiisY": radiisY,
>+      "rotationAngles": rotationAngles,
>+      "forces": forces,
>+      "count": count,
>+      "modifiers": modifiers
>+    });
>+  },

The reason I'd suggested one method instead of two is because I expected this
sendEvent method would accept an Event object, instead of the individual
params.

I figure that the vast majority of the time, you'll be sending a mouse/touch
events to content based on events the embedder received, so you'll already have
an event on hand.  And that API would save you from having to pass in a
bajillion parameters.

But this API is good in that it's crystal clear that you can only send mouse
and touch events (whereas something you passed a generic Event to would either
throw when it rejected an event or silently ignore it), so if you like it,
let's keep it for now.

>+++ b/dom/browser-element/mochitest/browserElement_SendEvent.js

I like how you did this test.  :)

r+ as-is, but if we make big changes to the API, I'd like to take another look.

Comment 4

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Justin Lebar [:jlebar] from comment #3)
> Comment on attachment 643086 [details] [diff] [review]
> Patch 
> >@@ -161,16 +162,20 @@ function BrowserElementParent(frameLoade
> >   }
> > 
> >   function defineDOMRequestMethod(domName, msgName) {
> >     XPCNativeWrapper.unwrap(self._frameElement)[domName] = self._sendDOMRequest.bind(self, msgName);
> >   }
> > 
> >   // Define methods on the frame element.
> >   defineMethod('setVisible', this._setVisible);
> >+  defineMethod('sendMouseEvent', this._sendMouseEvent);
> >+  if (Services.prefs.getBoolPref(TOUCH_EVENTS_ENABLED_PREF)) {
> >+    defineMethod('sendTouchEvent', this._sendTouchEvent);
> >+  }
> 
> We don't usually disappear API properties when they're not supported --
> desktop machines still have mozBattery, for example.
> 
> What happens if you send a touch event at a button on a build that doesn't
> have
> the touch events pref enabled?  Do we just ignore it?  I think we should try
> to
> retain those semantics, whatever they are, if we can.
> 

I've tried to respect the semantic of this preference. Afaik Touch interfaces are not exposed to content if this preference is set to false. So you can not create a touch event if this pref is false.

> (I know I said I preferred exceptions rather than silently failing; it's my
> mistake for not asking for more context.  Sorry!)
>

No worries. Ftr, I was actually asking on IRC for a different version of this patch. A version with a single unified method. Errors were to report unsupported event type.
 
> >+  _sendMouseEvent: function(type, x, y, button, clickCount, modifiers) {
> >+    this._sendAsyncMsg("send-mouse-event", {
> >+      "type": type,
> >+      "x": x,
> >+      "y": y,
> >+      "button": button,
> >+      "clickCount": clickCount,
> >+      "modifiers": modifiers
> >+    });
> >+  },
> >+
> >+  _sendTouchEvent: function(type, identifiers, touchesX, touchesY,
> >+                            radiisX, radiisY, rotationAngles, forces,
> >+                            count, modifiers) {
> >+    this._sendAsyncMsg("send-touch-event", {
> >+      "type": type,
> >+      "identifiers": identifiers,
> >+      "touchesX": touchesX,
> >+      "touchesY": touchesY,
> >+      "radiisX": radiisX,
> >+      "radiisY": radiisY,
> >+      "rotationAngles": rotationAngles,
> >+      "forces": forces,
> >+      "count": count,
> >+      "modifiers": modifiers
> >+    });
> >+  },
> 
> The reason I'd suggested one method instead of two is because I expected this
> sendEvent method would accept an Event object, instead of the individual
> params.
> 
> I figure that the vast majority of the time, you'll be sending a mouse/touch
> events to content based on events the embedder received, so you'll already
> have
> an event on hand.  And that API would save you from having to pass in a
> bajillion parameters.

I don't like to pass more than one parameter so I will be more than happy to switch at some point.

> 
> But this API is good in that it's crystal clear that you can only send mouse
> and touch events (whereas something you passed a generic Event to would
> either
> throw when it rejected an event or silently ignore it), so if you like it,
> let's keep it for now.

We can still change later if this is really needed right?


I will comment the debug line  and push that so we can have a try with it on for the browser app.

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Before pushing I will wait to make sure there is an agreement on what should be done to respect the dom.w3c_touch_events.enabled preference.

Comment 6

[Justin Lebar (not reading bugmail)]

(In reply to Vivien Nicolas (:vingtetun) from comment #5)
> Before pushing I will wait to make sure there is an agreement on what should
> be done to respect the dom.w3c_touch_events.enabled preference.

What you have is fine, based on comment 4.  Thanks!

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d3488fdea406

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d3488fdea406