751637 - Add Keys helper to Marionette client

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Malini Das [:mdas] - Away, not checking bugmail]

http://code.google.com/p/selenium/source/browse/trunk/py/selenium/webdriver/common/keys.py#26

All non-alphabet keys have to be sent over the wire as the above strings. We should add a module that provides this mapping to users so they can just do something like "keys.ENTER" instead of using the actual string.

Comment 1

[David Burns :automatedtester]

Attachment: added PUA codes helper code plus tests

I have removed the sendKeys atom and replaced it with more "Firefox" approach and added tests to check it fires events correctly.

Comment 2

[Jonathan Griffin (:jgriffin)]

Comment on attachment 647756 [details] [diff] [review]
added PUA codes helper code plus tests

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

Nice work!  r- only because this patch breaks test_text.py:test_sendKeys:

TEST-START test_text.py
test_clearText (test_text.TestText) ... ok
test_getText (test_text.TestText) ... ok
test_sendKeys (test_text.TestText) ... FAIL
test_clearText (test_text.TestTextChrome) ... ok
test_getText (test_text.TestTextChrome) ... ok
test_sendKeys (test_text.TestTextChrome) ... ERROR

======================================================================
ERROR: test_sendKeys (test_text.TestTextChrome)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/tests/unit/test_text.py", line 68, in test_sendKeys
    box.send_keys("at")
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/marionette.py", line 63, in send_keys
    return self.marionette._send_message('sendKeysToElement', 'ok', element=self.id, value=typing)
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/marionette.py", line 166, in _send_message
    self._handle_error(response)
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/marionette.py", line 223, in _handle_error
    raise MarionetteException(message=message, status=status, stacktrace=stacktrace)
MarionetteException: aStr.charAt is not a function
stacktrace:
	sendString@chrome://marionette/content/EventUtils.js:103
	MDA_sendKeysToElement@chrome://marionette/content/marionette-actors.js:1154
	DSC_onPacket@chrome://global/content/devtools/dbg-server.js:439
	@chrome://global/content/devtools/dbg-transport.js:170
	


======================================================================
FAIL: test_sendKeys (test_text.TestText)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/tests/unit/test_text.py", line 29, in test_sendKeys
    self.assertEqual("asdfo", self.marionette.execute_script("return arguments[0].value;", [l]))
AssertionError: 'asdfo' != u'oasdf'

This is possibly because both EventUtils.js and marionette-sendkeys.js define KeyEvent, and they're loaded in the same scope, but there could be something else going on as well.

* marionette-sendkeys.js:  should have a licensing header of some type (I'm not sure if this is homegrown or comes from Selenium).

* javascriptPage.html:  can we give this file a more descriptive name, like test_typing.html?

* Should we update sendKeysToElement in marionette-actors.js as well? (I don't know.)

Comment 3

[David Burns :automatedtester]

Going to use the comments as a todo so I dont lose my thoughts 



> This is possibly because both EventUtils.js and marionette-sendkeys.js
> define KeyEvent, and they're loaded in the same scope, but there could be
> something else going on as well.

The issue is because when Marionette now types it starts from the beginning and not the end. I will need to push that to the end because that is the correct behavior
 
> * marionette-sendkeys.js:  should have a licensing header of some type (I'm
> not sure if this is homegrown or comes from Selenium).
> 

I need to speak to Gerv. This is a mishmash of Selenium and my own code. Perhaps a dual licence since Apache and MPL 2 play well?


> * javascriptPage.html:  can we give this file a more descriptive name, like
> test_typing.html?
> 

I spoke to :JGriffin, since this is pulled from Selenium we will keep it as it will help porting tests easier.

> * Should we update sendKeysToElement in marionette-actors.js as well? (I
> don't know.)

I don't think so since the code I wrote is currently only going to be used in the content context. I need to double check that.

Comment 4

[David Burns :automatedtester]

Attachment: added PUA codes helper code plus tests. Fixes comments from previous patch

I have done all comments expect the licence, will do a follow up comment with gerv cc'ed to get his thoughts.

Comment 5

[David Burns :automatedtester]

:gerv

I have used a code from the Selenium project in my patch, which creates marionette-sendkeys.js and have mixed it with my own to make this work. This is mainly Selenium project code (80%) so should marionette-sendkeys.js have an Apache licence (which Selenium use) or MPL2.

Your advice is greatly appreciated

Comment 6

[Gervase Markham [:gerv]]

Hi David: if it's 80% Selenium, it seems to me to make sense to keep the Apache license for that file.

Gerv

Comment 7

[Jonathan Griffin (:jgriffin)]

Comment on attachment 648547 [details] [diff] [review]
added PUA codes helper code plus tests. Fixes comments from previous patch

This fixes the chrome test, but the content test still fails for me with:

======================================================================
FAIL: test_sendKeys (test_text.TestText)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jgriffin/mozilla-central/src/testing/marionette/client/marionette/tests/unit/test_text.py", line 29, in test_sendKeys
    self.assertEqual("asdfo", self.marionette.execute_script("return arguments[0].value;", [l]))
AssertionError: 'asdfo' != u'oasdf'

----------------------------------------------------------------------

Looks like we're still typing at the beginning of the text field, instead of the end.

Comment 8

[David Burns :automatedtester]

Attachment: added PUA codes helper code plus tests. Fixes comments from previous patch

I have no idea how I lost my cursor positioning part of my patch :(

All tests definitely pass now, double and triple checked.

Comment 9

[Jonathan Griffin (:jgriffin)]

Comment on attachment 649121 [details] [diff] [review]
added PUA codes helper code plus tests. Fixes comments from previous patch

Looks good, thanks!  Note marionette-sendkeys.js needs a license header before check-in.

Comment 10

[David Burns :automatedtester]

landed in https://hg.mozilla.org/integration/mozilla-inbound/rev/ef62a9a2ee42

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/ef62a9a2ee42