Add Keys helper to Marionette client

RESOLVED FIXED in mozilla17

Status

Testing
Marionette
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mdas, Assigned: automatedtester)

Tracking

Trunk
mozilla17
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

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.
(Assignee)

Updated

6 years ago
Assignee: nobody → dburns
(Assignee)

Updated

6 years ago
Blocks: 779311
(Assignee)

Comment 1

6 years ago
Created attachment 647756 [details] [diff] [review]
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.
Attachment #647756 - Flags: review?(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.)
Attachment #647756 - Flags: review?(jgriffin) → review-
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Comment 4

6 years ago
Created attachment 648547 [details] [diff] [review]
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.
Attachment #647756 - Attachment is obsolete: true
Attachment #648547 - Flags: review?(jgriffin)
(Assignee)

Comment 5

6 years ago
: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
Hi David: if it's 80% Selenium, it seems to me to make sense to keep the Apache license for that file.

Gerv
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.
Attachment #648547 - Flags: review?(jgriffin) → review-
(Assignee)

Comment 8

6 years ago
Created attachment 649121 [details] [diff] [review]
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.
Attachment #648547 - Attachment is obsolete: true
Attachment #649121 - Flags: review?(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.
Attachment #649121 - Flags: review?(jgriffin) → review+
https://hg.mozilla.org/mozilla-central/rev/ef62a9a2ee42
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.