Closed Bug 845849 Opened 8 years ago Closed 8 years ago

Remove MarionetteTouchMixin

Categories

(Testing :: Marionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla24

People

(Reporter: mdas, Assigned: mdas)

References

Details

Attachments

(1 file)

We're currently using synthetic_gestures.js to drive actions in the python abstraction layer, but we should swap it out for marionette methods.

We currently have single_tap support, but we'll also need mousedown/mouseup/mouseclick support in marionette so that self.marionette.tap() will do what we expect.

self.marionette.tap() will fire the chain of events expected by gaia, that is either 1) touchstart/touchend/mousedown/mouseup/mouseclick if there is no MouseShim or 2)just the touch events if the MouseShim is defined on the test page.
Once Bug 850819 lands, we can use the send_mouse_event flag to control if we're sending the full chain of events or not.

Bug 845925 is for dealing with scroll/pinch, adding as a dependency.
Assignee: nobody → yiyang
Depends on: 850819, 845925
No longer depends on: 845848
Since we added touch events to marionette, we're now using this python abstraction layer purely for dealing with the mouse event shim. The way it currently works is that we add support for any touch method you want, like tap or flick, by checking if the shim is there and firing some respective events. Considering how the shim is app-specific and that we now handle complex actions like action chains, this really isn't good design. I was thinking that we can remove this layer completely from the marionette client, since it's not really marionette specific. It would make more sense to be in gaia-ui-tests.

I was thinking that in gaia_test.py, I suppose in GaiaApps, whenever we launch an app/switch to a frame we should check for the presence of the shim, and set send_mouse_events_only (from Bug 857582) to True if it is there, False otherwise. That way, it will enable all touch related marionette commands in the test to do the right thing (send mouse events) when the shim is present. If you switch out of that frame and into one that doesn't contain the shim, send_mouse_events_only will be switched to False, and we'll send the touch events we expect.

This way, tests can be written normally, without the MarionetteTouchMixin we're using, and we won't need to rely on marionette.tap(element) or other python abstraction layer specific commands. Thoughts?
Assignee: annyang121 → mdas
Flags: needinfo?(dave.hunt)
I wouldn't really agree that it makes more sense. gaia-ui-tests just treats Marionette like an API. When you do el.tap() it only wants to do a tap, it doesn't know about mousedowns or shims. It just wants a tap to occur.

However I do understand the dilemma of every app being different and if we had to wrap switch_to_frame locally we could handle it.

Another issue is that other suites will use Marionette for functional tests, both inside and outside of Mozilla. The other suites would need this logic instead of just using marionette as is. It would need to be well documented or intuitive for someone else setting up their own test repository that they need to look out for this shim or some equivalent of it that's in their own app.
So, it actually turns out that after Bug 850819 landed, we no longer need to detect the shim, since we're sending the trusted events that the shim expects now. 

This means we can now deprecate marionette.tap() in the MarionetteTouchMixin.

Now, I'd like to bring up removing MarionetteTouchMixin altogether. It's no longer needed, and if you want to have additional helpers like it currently has for flick(), we can add them to http://mxr.mozilla.org/mozilla-central/source/testing/marionette/client/marionette/gestures.py, which is a collection of gesture related helper methods included in marionette-client.
Reading comment 4, do you still needinfo from me? I'm happy with dropping MarionetteTouchMixin if it's no longer needed.
Flags: needinfo?(dave.hunt)
(In reply to Dave Hunt (:davehunt) from comment #5)
> Reading comment 4, do you still needinfo from me? I'm happy with dropping
> MarionetteTouchMixin if it's no longer needed.

That's all I wanted to hear! :)

I'll morph this bug into a 'remove MarionetteTouchMixin' bug then.
We no longer need this mixin, as we provide the touch tools we need in marionette directly, and don't need to detect if the shim is there anymore since we're now sending trusted mouse events.
Summary: Update python touch abstraction layer to use marionette's methods instead of synthetic_gestures.js → Remove MarionetteTouchMixin
The removal of this module is dependent on the conversion of all the gaia-ui-tests away from using the touch mixin. When its use has stopped, I'll remove it.
webQA is done with the mixin, and roydude from Telefonica no longer depends on it either. We should be clear to remove this for the next client release.
Attachment #757501 - Flags: review?(jgriffin)
Comment on attachment 757501 [details] [diff] [review]
remove MarionetteTouchMixin and related files

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

I like!
Attachment #757501 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/mozilla-central/rev/bd6a9518088c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.