Expose controller.MouseEvent

RESOLVED FIXED

Status

Mozilla QA
Mozmill Tests
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: geo, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [module-refactor])

Attachments

(1 attachment)

Tests need it for when the standard click/etc functions aren't rich enough. Expose it as listed here:

https://github.com/mozautomation/mozmill/blob/hotfix-1.5.2/mozmill/mozmill/extension/resource/modules/controller.js#L409
IMO it should be bound to the element as all the other mouse and keyboard events. We should probably rename it so the method starts with an action.
Created attachment 519308 [details] [diff] [review]
Adds Widget.mouseEvent()

Henrik, couple of review points:

1) I intentionally kept the name the same. Changing it would increase confusion, I think. Also there's precedent with "keyPress" for the names on these being the name of the action being injected (vs. "pressKey" which would be a verb phrase).

2) The docs are old-style, because that's what matches master at the time this was branched. I'll change all of them at once when I give you the patch for that bug.

3) I didn't expose expectedEvent. It's not exposed on the other event methods yet either, and even if that changes I suspect we'll want our methods to supply it automatically.

http://people.mozilla.com/~gmealer/docs/ms1-mouseevent/
https://github.com/geoelectric/mozmill-api-refactor/tree/ms1-mouseevent
Attachment #519308 - Flags: review?(hskupin)
Comment on attachment 519308 [details] [diff] [review]
Adds Widget.mouseEvent()

>+   * @param {String} [aEvent.type="mousedown" + "mouseup"] Type of the mouse event

Not sure what's the best way to correctly outline the values will be. But what do you think about "mousedown" & "mouseup"? The '+' operator implicitly tells me the two strings are concatenated.

Each of the event methods on the controller have the expectedEvent as parameter. Just file a follow-up so it can be added to all of our methods. It can become fairly important for us.
Attachment #519308 - Flags: review?(hskupin) → review+
Yeah, I totally punted on that default. Means concatenation to me, too, but &/and didn't seem to be right either. Truth is, it's kinda sloppy to have a function that has a default you can't explicitly supply, so it's a weirdo.

Re: expectedEvent, I don't think that should be exposed to the tests, as I stated above. Assuming I don't grossly misunderstand how that mechanism works, the mouseDown() handler should supply an expectedEvent that says type: mousedown, target: me; and similar for the rest of the handlers. 

Honestly, it's a little weird that the functions are built to take the expected info explicitly in the first place. You're going to click one control but expect the event to land on another? You're always going to expect the thing you just told it to do, right?
Merged into master, https://github.com/geoelectric/mozmill-api-refactor/commit/42905de3ae092f1fde6cb3221f362848b03a7c91
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
(In reply to comment #4)
> Re: expectedEvent, I don't think that should be exposed to the tests, as I
> stated above. Assuming I don't grossly misunderstand how that mechanism works,
> the mouseDown() handler should supply an expectedEvent that says type:
> mousedown, target: me; and similar for the rest of the handlers. 

Well, it's not always the same element which sends the event. For XBL bindings it will probably be not the anonymous child but the binding itself. So you have to have the option to specify another target element. But in Mozmill we default to the element has been used to fire the event. That's how I have it implemented. For more details of expectedEvent see:

http://mxr.mozilla.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/EventUtils.js
Hm. OK. We'll have to figure out the right way to do this, then. All I know at this point is if it's this complicated to explain, it's also too complicated to expose to the tests if we can possibly help it.
Sure. I have noted that item in our brainstorming etherpad.
You need to log in before you can comment on or make changes to this bug.