809245 - Need a python abstraction layer for touch support

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Malini Das [:mdas] - Away, not checking bugmail]

Until we get gesture support in Marionette itself, we can import a JS library (namely: https://github.com/davidflanagan/Gestures/blob/master/SyntheticGestures.js) and use that. In order to prevent tests from needing to be ported over once we do get the support, it would be best to create an abstraction layer that can be used, and we can select which library it would use. This way you can write your tests with gestures, and once Marionette has support, we can just switch what library the abstraction layer uses.

Comment 1

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: touch "support"

This adds a touch mixin, so it appears as though marionette has touch support. It uses a modified version of SyntheticGestures.js

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 679460 [details] [diff] [review]
touch "support"

Review of attachment 679460 [details] [diff] [review]:
-----------------------------------------------------------------

We should add an MPL header to synthetic_gestures.js.

Comment 3

[David Burns :automatedtester]

Comment on attachment 679460 [details] [diff] [review]
touch "support"


> 
>diff --git a/testing/marionette/client/marionette/marionette_touch.py b/testing/marionette/client/marionette/marionette_touch.py
>new file mode 100644
>--- /dev/null
>+++ b/testing/marionette/client/marionette/marionette_touch.py
>@@ -0,0 +1,35 @@
>+# This Source Code Form is subject to the terms of the Mozilla Public
>+# License, v. 2.0. If a copy of the MPL was not distributed with this
>+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
>+
>+import os
>+
>+"""
>+Adds touch support in Marionette
>+"""
>+class MarionetteTouchMixin(object):
>+
>+    """
>+    Set up the touch layer.
>+    Can specify another library with a path and the name of the library.
>+    """
>+    def setup_touch(self, library=None, library_name=None):
>+        self.library = library or os.path.abspath(os.path.join(__file__, os.pardir, "touch", "synthetic_gestures.js"))
>+        self.library_name = library_name or "SyntheticGestures"
>+        self.import_script(self.library)
>+
>+    def tap(self, element):
>+        self.execute_script("%s.tap(arguments[0]);" % self.library_name, [element])
>+
>+    def double_tap(self, element):
>+        self.execute_script("%s.dbltap(arguments[0]);" % self.library_name, [element])
>+
>+    def swipe(self, element, x1, y1, x2, y2, duration=200):
>+        #jsonwireprotocol has no swipe. we will have to support this after, using up/down/move.
>+        self.execute_script("%s.swipe.apply(this, arguments);" % self.library_name, [element, x1, y1, x2, y2, duration])
>+
>+    def hold(self, element, holdtime, x1, y1, x2, y2, movetime=200):
>+        self.execute_script("%s.hold.apply(this, arguments);" % self.library_name, [element, holdtime, x1, y1, x2, y2, movetime])
>+
>+    def pinch(self, x1, y1, x2, y2, scale, duration=200):
>+        self.execute_script("%s.pinch.apply(this, arguments);" % self.library_name, [x1, y1, x2, y2, scale, duration])


I personally would like this to align with the Selenium Python touch methods so that we don't have to deprecate and rename things at a later stage. They allow use to chain methods together and simplify method signatures.

Comment 4

[David Burns :automatedtester]

URL for touch actions https://code.google.com/p/selenium/source/browse/trunk/py/selenium/webdriver/common/touch_actions.py

Comment 5

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: touch "support" v0.2

(In reply to David Burns :automatedtester from comment #4)
> URL for touch actions
> https://code.google.com/p/selenium/source/browse/trunk/py/selenium/webdriver/
> common/touch_actions.py

So the synthetic gestures library right now does not have support for chaining events. I'm assuming that users will be able to use these actions without chaining them. I've matched tap and double_tap with the same signatures, and added a "holdtime" parameter for long_press, which can be null and it will default to 2 seconds.

There is no flick action based on pixels per second in this library, so I added flick_duration, which will get from point A to point B in the given amount of time. I think this is a pretty useful feature, and I'm willing to keep supporting it in marionette_client (or add it as a chained action once we have support for it)

I removed pinch since it wasn't working, even when I was sending touch (not mouse) events.

Comment 6

[Malini Das [:mdas] - Away, not checking bugmail]

Hmm, I'm at a bit of a crossroads. 

Gaia doesn't actually accept touch events fired from Gecko. It expects mouseevents. These tap(), double_tap() and long_press() methods all get mapped to mouse events. If you send it as a touch event, it will not get translated to a mouse event, and the event will be ignored. Somehow, a physical touch event on the screen gets mapped to a Gecko mouse event and I'm not sure how that's done.

As such, we can keep this abstraction layer and synthetic gestures thing, but it doesn't really help. The tap/double_tap/etc methods all have to be mapped to mouse events, so I don't see why we don't just call click() on a Gaia element, instead of tap(). 

As for the synthetic gestures library, and it doesn't even really work for 'pinch', so we don't really get that much from it other than the fact that it implements long press and double tap for us. I turned on touch event sending and I was hoping that 'pinch' would work, but it just freezes the application.

We're pushing to get this out so the smoketests can be written, but they can be achieved using mouse events (click, etc). Perhaps we should be focusing on building out those so we natively support doubleclick. We'd need to figure out how to support 'pinch'-type actions anyway, since it doesn't work with the synthetic gestures library here.

What do you think, Zac?

Comment 7

[David Burns :automatedtester]

Comment on attachment 679900 [details] [diff] [review]
touch "support" v0.2

I am happy for this to go in like this for now but we will probably deprecate the way things work later.


 
>diff --git a/testing/marionette/client/marionette/marionette_touch.py b/testing/marionette/client/marionette/marionette_touch.py
>new file mode 100644
>--- /dev/null
>+++ b/testing/marionette/client/marionette/marionette_touch.py
>+    def flick_duration(self, element, x1, y1, x2, y2, duration=200):

Can we just call this flick

>diff --git a/testing/marionette/client/marionette/touch/synthetic_gestures.js b/testing/marionette/client/marionette/touch/synthetic_gestures.js

>+  //
>+  function touch(target, duration, xt, yt, then) {

Looking at the gestures library it does appear to do chaining but you will need to track each of the things and pass it in as a callback. I appreciate that its going to probably be error prone. I might be misunderstanding the code in my current flu-induced state though ;)

Comment 8

[Zac C (:zac)]

I can't speak for the technical challenges but at this stage but none of the smoke tests use pinch so we can do without it for now.

Can you just throw an UnsupportedException if it is called?

Overall I would just want the action to be as realistic and representative of touching the screen as possible.

Comment 9

[Malini Das [:mdas] - Away, not checking bugmail]

(In reply to David Burns :automatedtester from comment #7)
> Comment on attachment 679900 [details] [diff] [review]
> touch "support" v0.2
> 
> I am happy for this to go in like this for now but we will probably
> deprecate the way things work later.
> 
> 
>  
> >diff --git a/testing/marionette/client/marionette/marionette_touch.py b/testing/marionette/client/marionette/marionette_touch.py
> >new file mode 100644
> >--- /dev/null
> >+++ b/testing/marionette/client/marionette/marionette_touch.py
> >+    def flick_duration(self, element, x1, y1, x2, y2, duration=200):
> 
> Can we just call this flick
> 
> >diff --git a/testing/marionette/client/marionette/touch/synthetic_gestures.js b/testing/marionette/client/marionette/touch/synthetic_gestures.js
> 
> >+  //
> >+  function touch(target, duration, xt, yt, then) {
> 
> Looking at the gestures library it does appear to do chaining but you will
> need to track each of the things and pass it in as a callback. I appreciate
> that its going to probably be error prone. I might be misunderstanding the
> code in my current flu-induced state though ;)

You're right it is there, but I'd need to export it to use it, and test it more to make sure it works as expected... I'd rather get this in and then start work on getting this working from the marionette side.

Comment 10

[Malini Das [:mdas] - Away, not checking bugmail]

Attachment: touch "support" v0.3

added changes requested by dburns and zac.

Comment 11

[Malini Das [:mdas] - Away, not checking bugmail]

pushed to try:
https://tbpl.mozilla.org/?tree=Try&rev=c28f8f1a548b 

Will push to aurora when I can update it and push it. I'm on my way out

Comment 12

[Malini Das [:mdas] - Away, not checking bugmail]

ash (aurora-try):
https://tbpl.mozilla.org/?tree=Ash&rev=b38eb79c69e1

Comment 13

[Malini Das [:mdas] - Away, not checking bugmail]

landed on mozilla-inbound: 

https://hg.mozilla.org/integration/mozilla-inbound/rev/4be7dbc93bba

Still waiting on ash.

Comment 14

[Malini Das [:mdas] - Away, not checking bugmail]

landed on mozilla-aurora!

https://hg.mozilla.org/releases/mozilla-aurora/rev/276150485251

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/4be7dbc93bba

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Just a note... so once bug 750271 has been fixed we do longer have to make use of this external library.