1016147 - marionette HTMLElement.send_keys() doesn't insert text after the caret position

Status: RESOLVED FIXED

(Remote Protocol :: Marionette, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

HTMLElement.send_keys() always appends text at the end of the <input> regardless of the caret position.

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Simple test case for send_keys() after tap()

This test case will fail. '56781234' is expected, but the result is '12345678'

The full output is:

$ ./mach marionette-test testing/marionette/client/marionette/tests/unit/test_caret.py
starting httpd
running webserver on http://127.0.0.1:50189/
TEST-START test_caret.py
test_send_keys_to_type_input (test_caret.TestCaret) ... FAIL

======================================================================
FAIL: None
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/tlin/Projects/gecko-dev/testing/marionette/client/marionette/marionette_test.py", line 170, in run
    testMethod()
  File "/Users/tlin/Projects/gecko-dev/testing/marionette/client/marionette/tests/unit/test_caret.py", line 11, in test_send_keys_to_type_input
    self.assertEqual('56781234', self.marionette.execute_script("return arguments[0].value", [num_input]))
TEST-UNEXPECTED-FAIL | test_caret.py test_caret.TestCaret.test_send_keys_to_type_input | AssertionError: '56781234' != u'12345678'
----------------------------------------------------------------------
Ran 1 test in 1.357s

FAILED (failures=1)

SUMMARY
-------
passed: 0
failed: 1
todo: 0

FAILED TESTS
-------
test_caret.py test_caret.TestCaret.test_send_keys_to_type_input

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

I found that sendKeysToElement() moves the caret to the end. Is it OK to remove that?
If it's OK, I could help upload a patch with a test case.
http://dxr.mozilla.org/mozilla-central/source/testing/marionette/marionette-listener.js#1563

BTW, I found tap(0,0) on the <input> HTMLElement might not always move the caret to the front. Is there a better method to do that?

Comment 3

[David Burns :automatedtester]

Making that change should be fine but you will need to do a try run with all Mn, Mnw and Gu tests run.

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Make sendKeysToElement() respect caret position

This allows HTMLElement.send_keys() to insert text after the caret in
<input>.

Also, fix the test cast fail in test_text.py due to caret position was
not being considered.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=a32b25f32b40

Comment 5

[David Burns :automatedtester]

Comment on attachment 8429960 [details] [diff] [review]
Make sendKeysToElement() respect caret position

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

::: testing/marionette/client/marionette/tests/unit/test_text.py
@@ +25,5 @@
>          test_html = self.marionette.absolute_url("test.html")
>          self.marionette.navigate(test_html)
>          l = self.marionette.find_element("name", "myInput")
>          self.assertEqual("asdf", self.marionette.execute_script("return arguments[0].value;", [l]))
> +        # Since the "asdf" is short, tap() will place the caret at the end of the text.

I don't think that this will catch any regressions to what you want.

I suggest setting the caret position with el.selectionStart() and el.selectionEnd() in a execute_script and then do a sendkeys.

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Make sendKeysToElement() respect caret position (v2)

Agree. Setting selectionStart and selectionEnd is better.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=eae6de3878c1

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Make sendKeysToElement() respect caret position (v3)

Change variable name and add semi-colon in execute_script(). The try result should be the same as v2.

Comment 8

[David Burns :automatedtester]

Comment on attachment 8432177 [details] [diff] [review]
Make sendKeysToElement() respect caret position (v3)

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

The test still isnt going to catch any regressions that we might have for this part of the functionality.

::: testing/marionette/client/marionette/tests/unit/test_text.py
@@ +29,5 @@
> +
> +        # Set caret position at the end of the input text.
> +        self.marionette.execute_script(
> +            """var el = arguments[0];
> +            el.selectionStart = el.selectionEnd = el.value.length;""",

This is setting the caret to the end of the string that is in the <input> 

You need to put it to something like el.value.length/2 and then check that it is 

asodf

Comment 9

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Make sendKeysToElement() respect caret position (v4)

I think I understand "failing to catch any regressions" now. I have changed the caret position to el.value.length/2 in the test case.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=46255e9e07df

Comment 10

[David Burns :automatedtester]

Comment on attachment 8432582 [details] [diff] [review]
Make sendKeysToElement() respect caret position (v4)

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

looks good to me!

Comment 11

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

David, thanks for the review!

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dcb77f2cdc4c

Comment 13

[Carsten Book [:Tomcat]]

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