w3c / webdriver

Remote control interface that enables introspection and control of user agents.
https://w3c.github.io/webdriver/
Other
679 stars 194 forks source link

Fix setting cursor position in send keys #1686

Open jgraham opened 2 years ago

jgraham commented 2 years ago

I believe the expected behaviour is:

Previously the spec was unconditionally focusing the target element (execpting some cases with file uploads), and therefore the letter of the spec was to never move the caret.

This change updates it to only focus the target if it's not already the active element, and then move the caret to the end.


Preview | Diff

jgraham commented 2 years ago

An open question is what to do if a child of the target already has focus e.g. if the <div> is the target and the <span> is the activeElement in

<div contenteditable>
foo
<span>bar</span>
baz
</div>

Currently this will set the focus to the end of the <div>.

jgraham commented 2 years ago

I think I'm convinced that the child element case is reasonable for contenteditable, but what about <select>/<option>. If you sendKeys to an option element should it focus / scrollIntoView the select? I'm not quite sure if an <option> is considered to have a focusable area, but maybe we just want to always use the <select> in that case? There seem to be some existing tests that assume we'll do so.

whimboo commented 2 years ago

Please note that there is an upstream sync PR from @jgraham open which covers that proposed change and might merge soon: https://github.com/web-platform-tests/wpt/pull/36226

whimboo commented 2 years ago

@sadym-chromium maybe you could review as well?

jgraham commented 2 years ago

I ended up moving the difficult <option> cases out of wpt, because they were testing form control rendering in a way that doesn't make sense cross browser/platform.

whimboo commented 1 year ago

@jgraham would you mind checking the feedback as received from @sadym-chromium?

jgraham commented 1 year ago
  1. It looks like the behaviour changed: It used to set a cursor anywhere inside the element, and now it selects the whole content. Was it intended?

Unless I've mae a mistake with the spec (very possible, but you'll need to be more specific if so), it puts the cursor at the end (last editable point) of the element, which I believe was the intended behavior from the start.

  1. (out of scope of the this PR) Probably I'm missing something. How do users suppose to change the value of the "content editable" element?

By inserting text into it? I don't really understand the question, but in general if you want to do arbitary updates to a contenteditable element, then you need to use the actions API.

sadym-chromium commented 1 year ago
  1. It looks like the behaviour changed: It used to set a cursor anywhere inside the element, and now it selects the whole content. Was it intended?

Unless I've mae a mistake with the spec (very possible, but you'll need to be more specific if so), it puts the cursor at the end (last editable point) of the element, which I believe was the intended behavior from the start.

Added the comment in the code.