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

RESOLVED FIXED in mozilla32

Status

defect
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: TYLin, Assigned: TYLin)

Tracking

({pi-marionette-server, pi-marionette-userinput})

Trunk
mozilla32
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

Assignee

Description

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

Comment 1

5 years ago
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
Assignee

Updated

5 years ago
Blocks: 960897
Assignee

Updated

5 years ago
Assignee: nobody → tlin
Assignee

Comment 2

5 years ago
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?
Flags: needinfo?(dburns)
Making that change should be fine but you will need to do a try run with all Mn, Mnw and Gu tests run.
Flags: needinfo?(dburns)
Assignee

Comment 4

5 years ago
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
Attachment #8428960 - Attachment is obsolete: true
Assignee

Updated

5 years ago
Attachment #8429960 - Flags: review?(dburns)
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.
Attachment #8429960 - Flags: review?(dburns) → review-
Assignee

Comment 6

5 years ago
Agree. Setting selectionStart and selectionEnd is better.

Try result:
https://tbpl.mozilla.org/?tree=Try&rev=eae6de3878c1
Attachment #8432097 - Flags: review?(dburns)
Assignee

Comment 7

5 years ago
Change variable name and add semi-colon in execute_script(). The try result should be the same as v2.
Attachment #8429960 - Attachment is obsolete: true
Attachment #8432097 - Attachment is obsolete: true
Attachment #8432097 - Flags: review?(dburns)
Attachment #8432177 - Flags: review?(dburns)
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
Attachment #8432177 - Flags: review?(dburns) → review-
Assignee

Comment 9

5 years ago
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
Attachment #8432582 - Flags: review?(dburns)
Comment on attachment 8432582 [details] [diff] [review]
Make sendKeysToElement() respect caret position (v4)

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

looks good to me!
Attachment #8432582 - Flags: review?(dburns) → review+
Assignee

Updated

5 years ago
Attachment #8432177 - Attachment is obsolete: true
Assignee

Comment 11

5 years ago
David, thanks for the review!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/dcb77f2cdc4c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.