Closed Bug 1129702 Opened 5 years ago Closed 5 years ago

Add support for doubleclicking to marionette's actions

Categories

(Testing :: Marionette, defect, P1)

defect

Tracking

(firefox38 fixed, firefox39 fixed)

RESOLVED FIXED
mozilla39
Tracking Status
firefox38 --- fixed
firefox39 --- fixed

People

(Reporter: chmanchester, Assigned: chmanchester)

References

(Blocks 1 open bug)

Details

(Keywords: pi-marionette-client, pi-marionette-server, pi-marionette-spec, Whiteboard: [marionette=1.0])

Attachments

(3 files, 1 obsolete file)

This came up talking about converting mozmill tests. The webdriver spec mentions this under the "actions" section with respect to pauses.
This is probably going to be 2 parts.

Adding click to the actions api and then having an alias that does

action(marionette).click().click().perform()

It is not in the spec currently so assuming that is what we meant, will need to go check my notes.
Blocks: m21s
OS: Linux → All
Hardware: x86_64 → All
David, can you confirm comment 2 before I get down to implementing that? Thanks!
Flags: needinfo?(dburns)
I wanted to see how to integrate with the low-level event stuff and ended up here. Morphing ni? into f?
Attachment #8562312 - Flags: feedback?(dburns)
Assignee: nobody → cmanchester
Status: NEW → ASSIGNED
Flags: needinfo?(dburns)
Comment on attachment 8562312 [details] [diff] [review]
Add support for double clicks to marionette's action chains

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

::: testing/marionette/client/marionette/marionette.py
@@ +357,5 @@
> +        :param count: Optional, the count of clicks to synthesize (for double
> +                      click events).
> +        '''
> +        el = element.id
> +        self.action_chain.append(['click', el, None, count])

I'm imagining the button index would go along with this info for bug 1097705.
Comment on attachment 8562312 [details] [diff] [review]
Add support for double clicks to marionette's action chains

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

This is exactly what I was thinking and even follows what I thought my notes said (even though I can't find them)

::: testing/marionette/client/marionette/marionette.py
@@ +357,5 @@
> +        :param count: Optional, the count of clicks to synthesize (for double
> +                      click events).
> +        '''
> +        el = element.id
> +        self.action_chain.append(['click', el, None, count])

Yea, I agree. I think it would be better to have a context_click and middle_click methods that can delegate down to here to simplify discovery of the methods.
Attachment #8562312 - Flags: feedback?(dburns) → feedback+
Priority: -- → P1
See Also: → 1135846
/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.

Pull down this commit:

hg pull review -r cc369a6e776e10dcf698fce0094f573278ecf789
Attachment #8568795 - Flags: review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.

Pull down this commit:

hg pull review -r cc369a6e776e10dcf698fce0094f573278ecf789
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.
/r/4271 - Bug 1097705 - Add ability to right and middle click to marionette's action chains.

Pull down these commits:

hg pull review -r 08641bc7945faba377b316a467ab767f9f6ec540
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.
/r/4271 - Bug 1097705 - Add ability to right and middle click to marionette's action chains.

Pull down these commits:

hg pull review -r ce2f587ff2a1a1ab0987fca3fbeb7babc1bc3dbc
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

https://reviewboard.mozilla.org/r/4267/#review3643

::: testing/marionette/client/marionette/tests/unit/test_click.py
(Diff revision 3)
> +        self.action.click(el)

there is no perform() so this is never going to hit the the server code so we never test if the element is clicked on or not

::: testing/marionette/driver/marionette_driver/marionette.py
(Diff revision 3)
> +        return self.click(element, button=2)

I wonder if we should have form of enum for button= so that the magic numbers are not obvious
Attachment #8568795 - Flags: review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.
/r/4271 - Bug 1097705 - Add ability to right and middle click to marionette's action chains.

Pull down these commits:

hg pull review -r 444438f9bceaa8a6dd6b16003e1752ce1c292862
Attachment #8568795 - Flags: review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

https://reviewboard.mozilla.org/r/4267/#review3647

::: testing/marionette/client/marionette/tests/unit/test_click.py
(Diff revision 4)
> +        rel = self.marionette.find_element("id", "keyReporter")

Looks like halfway through this patch,and sorry I missed it on the initial review, you stopped using `By.*` and started using the strings. Can you update to `By.*`
Attachment #8568795 - Flags: review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

https://reviewboard.mozilla.org/r/4267/#review3649

Ship It!
Attachment #8568795 - Flags: review+
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.
/r/4271 - Bug 1097705 - Add ability to right and middle click to marionette's action chains.

Pull down these commits:

hg pull review -r f76b623aa7bfe729c4c216b27a93ce17db321d13
Attachment #8568795 - Flags: review+ → review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

Fixed nit and carrying r+
Attachment #8568795 - Flags: review?(dburns) → review+
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

/r/4269 - Bug 1129702 - Add support for double clicks to marionette's action chains.
/r/4271 - Bug 1097705 - Add ability to right and middle click to marionette's action chains.

Pull down these commits:

hg pull review -r 3396422860c732f5dd6432f3e56c8e3ce4343833
Attachment #8568795 - Flags: review+ → review?(dburns)
Comment on attachment 8568795 [details]
MozReview Request: bz://1129702/chmanchester

Forgot to qref, this is the version with the nit fixed.
Attachment #8568795 - Flags: review?(dburns) → review+
https://hg.mozilla.org/mozilla-central/rev/d58601e11b7e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
We kinda would like to have this support for Firefox 38 to be able to run our Firefox UI tests. Would it be possible to backport this patch?
Flags: needinfo?(dburns)
Flags: needinfo?(cmanchester)
Flags: needinfo?(cmanchester)
Attachment #8568795 - Attachment is obsolete: true
Attachment #8619318 - Flags: review+
Attachment #8619319 - Flags: review+
You need to log in before you can comment on or make changes to this bug.