Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Automated Keyboard Input #10113

Merged
merged 9 commits into from Apr 3, 2018

Conversation

Projects
None yet
6 participants
@kereliuk
Copy link
Contributor

kereliuk commented Mar 20, 2018

This provides the testdriver function for sending keyboard input to an element.


This change is Reviewable

@w3c-bots

This comment has been minimized.

Copy link

w3c-bots commented Mar 20, 2018

Build ERRORED

Started: 2018-04-03 18:02:12
Finished: 2018-04-03 18:55:03

Failing Jobs

  • firefox:nightly
  • chrome:dev

View more information about this build on:

@jgraham

This comment has been minimized.

Copy link
Contributor

jgraham commented Mar 27, 2018

So implementation-wise this looks OK. We need an integration test under infrastructure for the new feature.

In general I'm a little worried about relying on the DWIM methods of WebDriver vs the low level actions.


Reviewed 5 of 9 files at r1, 1 of 1 files at r2.
Review status: 5 of 7 files reviewed at latest revision, all discussions resolved, some commit checks broke.


resources/testdriver.js, line 90 at r2 (raw file):

        send_keys: function(element, keys) {
            if (window.top !== window) {
                return Promise.reject(new Error("can only click in top-level window"));

This isn't a click.


resources/testdriver.js, line 97 at r2 (raw file):

            }

            if (!inView(element)) {

Doesn't webdriver do this for us?


tools/wptrunner/wptrunner/executors/protocol.py, line 270 at r2 (raw file):

    @abstractmethod
    def send_keys(self, element, keys):
        """Perform a trusted click somewhere on a specific element.

Comment is wrong.


Comments from Reviewable

@jgraham

This comment has been minimized.

Copy link
Contributor

jgraham commented Mar 27, 2018

Reviewed 2 of 9 files at r1.
Review status: all files reviewed at latest revision, 3 unresolved discussions, some commit checks broke.


Comments from Reviewable

@gsnedders

This comment has been minimized.

Copy link
Contributor

gsnedders commented Mar 28, 2018

resources/testdriver.js, line 97 at r2 (raw file):

Previously, jgraham wrote…

Doesn't webdriver do this for us?

As with click, we should do this ourselves, because it means a vendor implementations that is just window.internals.send_keys(element, keys) gets the same behaviour as WebDriver


Comments from Reviewable

kereliuk added some commits Apr 3, 2018

let text_box = document.getElementById("text");
test_driver
.send_keys(text_box, "Hello, wpt!")
.then(() => t.done())

This comment has been minimized.

Copy link
@jgraham

jgraham Apr 3, 2018

Contributor

Can we check this actually worked here?

This comment has been minimized.

Copy link
@gsnedders

gsnedders Apr 4, 2018

Contributor

This still seems like a reasonable comment, @kereliuk!

This comment has been minimized.

Copy link
@gsnedders

gsnedders Apr 4, 2018

Contributor

No, GitHub is just showing me random diffs that aren't of the right commit when I first click here. o_O

kereliuk added some commits Apr 3, 2018

@@ -230,7 +231,7 @@ def set(self, name, value):
case prefInterface.PREF_BOOL:
prefInterface.setBoolPref(pref, value);
break;
case prefInterface.PREF_INT:
case prefInterface.PREF_INT:SendKeysProtocolPart

This comment has been minimized.

Copy link
@jgraham

jgraham Apr 3, 2018

Contributor

Looks like some junk here.

@jgraham

jgraham approved these changes Apr 3, 2018

@foolip

This comment has been minimized.

Copy link
Contributor

foolip commented Apr 3, 2018

Travis failures are because of #7660

@foolip foolip merged commit 8f93a78 into web-platform-tests:master Apr 3, 2018

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.