Need a python abstraction layer for touch support

RESOLVED FIXED in Firefox 18

Status

RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mdas, Assigned: mdas)

Tracking

Trunk
B2G C1 (to 19nov)
x86
All
Points:
---

Firefox Tracking Flags

(firefox18 fixed)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
Created attachment 679460 [details] [diff] [review]
touch "support"

This adds a touch mixin, so it appears as though marionette has touch support. It uses a modified version of SyntheticGestures.js
Attachment #679460 - Flags: review?(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 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.
Attachment #679460 - Flags: feedback-
Target Milestone: --- → B2G C1 (to 19nov)
Created attachment 679900 [details] [diff] [review]
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.
Attachment #679460 - Attachment is obsolete: true
Attachment #679460 - Flags: review?(jgriffin)
Attachment #679900 - Flags: review?(jgriffin)
Attachment #679900 - Flags: feedback?(dburns)
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?
Flags: needinfo?(zcampbell)
See Also: → bug 750271
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 ;)
Attachment #679900 - Flags: feedback?(dburns) → feedback+

Comment 8

6 years ago
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.
Flags: needinfo?(zcampbell)
(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.
Created attachment 680099 [details] [diff] [review]
touch "support" v0.3

added changes requested by dburns and zac.
Attachment #679900 - Attachment is obsolete: true
Attachment #679900 - Flags: review?(jgriffin)
Attachment #680099 - Flags: feedback+
Attachment #680099 - Flags: review?(jgriffin)
Attachment #680099 - Flags: review?(jgriffin) → review+
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
https://hg.mozilla.org/mozilla-central/rev/4be7dbc93bba
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED

Updated

6 years ago
status-firefox18: --- → fixed
Duplicate of this bug: 829614
Just a note... so once bug 750271 has been fixed we do longer have to make use of this external library.
You need to log in before you can comment on or make changes to this bug.