Closed Bug 1091662 Opened 10 years ago Closed 10 years ago

Ensure shortcuts work in Marionette tests

Categories

(Remote Protocol :: Marionette, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla36

People

(Reporter: whimboo, Assigned: chmanchester)

References

()

Details

Attachments

(1 file, 2 obsolete files)

While working on shortcut examples (see URL field for test file) for Marionette I have seen a couple of issues: * Shortcuts with modifiers are not working when sent to the browser window * There is a missing Accel key, which we were able to use before to circumvent the CONTROL (Linux/Windows) and COMMAND (OS X) modifier. We may want to request this feature in Marionette client * No send_keys support in Actions (see bug 1090925) As talked on IRC, Chris is most likely going to work on this.
There's backdoor to EventUtils.js through the chrome sanbox that exposes a synthesizeKeys function that looks a lot like what mozmill uses. I have a working version of the test at https://github.com/chmanchester/firefox-greenlight-tests/tree/keys that uses this. Using this may end up being more expedient than fixing bug 1090925, but I'll check that out tomorrow.
As David told me send_keys should perfectly work with the modifiers. So this method might not correctly send it.
(In reply to Henrik Skupin (:whimboo) from comment #2) > As David told me send_keys should perfectly work with the modifiers. So this > method might not correctly send it. send_keys is implemented in terms of synthesizeKeys, so I'm not sure about that (although the accel keys was not working when I tried to use it due to navigator not being defined in the execution sandbox). send_keys with modifiers doesn't seem to be working as expected, I'll look into that a further.
In chrome context the marionette server implements send_keys in terms of EventUtils' sendString, which has in a comment (apparently accurate): * For now this method only works for English letters (lower and upper case) * and the digits 0-9. So I don't think this will be sufficient as-is.
I was able to get something like the test linked in the url field of this bug by using the content scope and making a few modifications (that I think are bug fixes; I'll file that separately) to marionette-sendkeys.js. I think our choice now is: Develop a python library based on synthesizeKeys through testUtils mentioned in comment 1 or Get marionette to be where we need it, which would involve fixing bug 1090925 and/or working around comment 4.
We need to fix Marionette here. It looks like we have 2 ways of typing in Marionette and we need to move the Chrome way of testing to be the same as http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-server.js#2051 needs to be more like http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#1620 The Content way uses http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-sendkeys.js which understands(or at least should) modifiers
Assignee: nobody → cmanchester
Attachment #8516437 - Attachment is obsolete: true
Comment on attachment 8516707 [details] [diff] [review] Support for modifier keys in chrome scope in marionette. Review of attachment 8516707 [details] [diff] [review]: ----------------------------------------------------------------- ::: testing/marionette/marionette-sendkeys.js @@ -70,4 @@ > break; > case '\uE03D': > keyCode = "VK_META"; > - metaKey = !metaKey; What about accelKey? This would be a helpful addition in terms of the command key on OS X and ctrl on Linux/Windows. ::: testing/marionette/marionette-server.js @@ +2049,5 @@ > + let el = this.curBrowser.elementManager.getKnownElement( > + aRequest.parameters.id, currentWindow); > + utils.sendKeysToElement(currentWindow, el, aRequest.parameters.value, > + this.sendOk.bind(this), this.sendError.bind(this), > + command_id, "chrome"); Maybe better to pass this.context here.
(In reply to Henrik Skupin (:whimboo) from comment #9) > Comment on attachment 8516707 [details] [diff] [review] > Support for modifier keys in chrome scope in marionette. > > Review of attachment 8516707 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: testing/marionette/marionette-sendkeys.js > @@ -70,4 @@ > > break; > > case '\uE03D': > > keyCode = "VK_META"; > > - metaKey = !metaKey; > > What about accelKey? This would be a helpful addition in terms of the > command key on OS X and ctrl on Linux/Windows. I was thinking we can add this in our own library as a shortcut. I didn't see a reference to it in the character types table in the webdriver spec, but I could have missed it under an alias. > > ::: testing/marionette/marionette-server.js > @@ +2049,5 @@ > > + let el = this.curBrowser.elementManager.getKnownElement( > > + aRequest.parameters.id, currentWindow); > > + utils.sendKeysToElement(currentWindow, el, aRequest.parameters.value, > > + this.sendOk.bind(this), this.sendError.bind(this), > > + command_id, "chrome"); > > Maybe better to pass this.context here.
(In reply to Chris Manchester [:chmanchester] from comment #10) > > What about accelKey? This would be a helpful addition in terms of the > > command key on OS X and ctrl on Linux/Windows. > > I was thinking we can add this in our own library as a shortcut. I didn't > see a reference to it in the character types table in the webdriver spec, > but I could have missed it under an alias. It's more like a meta key set to the appropriate command key. So it's not real. Having that would help a lot in reducing if/else blocks. Having that in general would be a great enhancement for desktop applications, but yeah if it is not in the spec.... David, what do you think?
(In reply to Henrik Skupin (:whimboo) from comment #11) > (In reply to Chris Manchester [:chmanchester] from comment #10) > > > What about accelKey? This would be a helpful addition in terms of the > > > command key on OS X and ctrl on Linux/Windows. > > > > I was thinking we can add this in our own library as a shortcut. I didn't > > see a reference to it in the character types table in the webdriver spec, > > but I could have missed it under an alias. > > It's more like a meta key set to the appropriate command key. So it's not > real. Having that would help a lot in reducing if/else blocks. Having that > in general would be a great enhancement for desktop applications, but yeah > if it is not in the spec.... David, what do you think? Sounds like it would be useful. You can handle this on the local by checking the platform that is returned when you start a new session. I suggest raising a bug against https://www.w3.org/Bugs/Public/enter_bug.cgi?comment=&blocked=20860&short_desc=%5BWebDriver%20Spec%5D%3A%20&product=Browser%20Test%2FTools%20WG&component=WebDriver for AccelKeys to get added and I will put it forward for discussion within the group
Comment on attachment 8516707 [details] [diff] [review] Support for modifier keys in chrome scope in marionette. Review of attachment 8516707 [details] [diff] [review]: ----------------------------------------------------------------- As discussed on IRC, it would be great to have a test for this ::: testing/marionette/marionette-server.js @@ +2049,5 @@ > + let el = this.curBrowser.elementManager.getKnownElement( > + aRequest.parameters.id, currentWindow); > + utils.sendKeysToElement(currentWindow, el, aRequest.parameters.value, > + this.sendOk.bind(this), this.sendError.bind(this), > + command_id, "chrome"); yea, please change this to this.context
Attachment #8516707 - Attachment is obsolete: true
Attachment #8516707 - Flags: review?(dburns)
(In reply to David Burns :automatedtester from comment #12) > I suggest raising a bug against [..] > I will put it forward for discussion within the group Thank's David! I filed it as https://www.w3.org/Bugs/Public/show_bug.cgi?id=27244.
Comment on attachment 8517087 [details] [diff] [review] Support for modifier keys in chrome scope in marionette. Review of attachment 8517087 [details] [diff] [review]: ----------------------------------------------------------------- Try looks good with the copy/paste test. This is also passing the greenlight test linked in the url field of the bug.
Attachment #8517087 - Flags: review?(dburns)
Attachment #8517087 - Flags: review?(dburns) → review+
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
Product: Testing → Remote Protocol
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: