Implement ElementClick wd command #24090

georgeroman · 2019-08-28T20:43:46Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors

There are tests for these changes

This change is

highfive · 2019-08-28T20:43:51Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/webdriver_handlers.rs, components/webdriver_server/lib.rs, components/webdriver_server/actions.rs, components/script/script_thread.rs
@jgraham: components/webdriver_server/lib.rs, components/webdriver_server/actions.rs
@KiChjang: components/script/webdriver_handlers.rs, components/script/script_thread.rs, components/script_traits/webdriver_msg.rs

jdm · 2019-08-29T05:24:56Z

components/script/webdriver_handlers.rs

+                        let event_target = option_element.upcast::<EventTarget>();
+                        event_target.fire_event(atom!("mouseover"));
+                        event_target.fire_event(atom!("mousemove"));
+                        event_target.fire_event(atom!("mousedown"));


The spec says to fire events at the parent node throughout this method.

jdm · 2019-08-29T05:24:56Z

components/script/webdriver_handlers.rs

+                            let previous_selectedness = option_element.Selected();
+
+                            // Step 8.6.3
+                            option_element.SetSelected(true);


This seems to be ignoring the bit in the spec about the multiple attribute in the element's container.

jdm · 2019-08-29T18:44:19Z

components/script/webdriver_handlers.rs

+                // Step 8
+                match node.downcast::<HTMLOptionElement>() {
+                    Some(option_element) => {
+                        // https://w3c.github.io/webdriver/#dfn-container


We should file an issue about extract this into a function that supports optgroup as well (per the spec).

georgeroman · 2019-08-30T07:25:09Z

@jdm Is this ready for merging?

jdm · 2019-08-30T07:44:20Z

@bors-servo r+

bors-servo · 2019-08-30T07:44:21Z

📌 Commit a0ad0fd has been approved by jdm

bors-servo · 2019-08-30T07:44:26Z

⌛ Testing commit a0ad0fd with merge af8fc44...


        Auto merge of #24090 - georgeroman:implement_element_click_wd_command…

…, r=jdm Implement ElementClick wd command  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors  - [X] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24090)

bors-servo · 2019-08-30T08:34:15Z

💔 Test failed - linux-rel-wpt

CYBAI · 2019-08-30T08:39:10Z

{
    "status": "FAIL", 
    "group": "default", 
    "message": "AssertionError: javascript error (500): JS evaluation raised an exception", 
    "stack": "session = <Session 1f42a8a2-3e79-4849-bcc9-b7e17b9c0dee>\n\n    def test_element_in_collection(session):\n        session.url = inline(\"<div></div>\")\n        divs = session.find.css(\"div\")\n    \n        response = execute_script(session, \"\"\"\n            let div = document.querySelector(\"div\");\n            div.reference = div;\n            return [div];\n            \"\"\")\n>       value = assert_success(response)\n\ndivs       = [<Element 7733c369f4894450a776e665557100d7>]\nresponse   = <Response status=500 error=<JavascriptErrorException http_status=500>>\nsession    = <Session 1f42a8a2-3e79-4849-bcc9-b7e17b9c0dee>\n\ntests/wpt/web-platform-tests/webdriver/tests/execute_script/cyclic.py:61: \n_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ \n\nresponse = <Response status=500 error=<JavascriptErrorException http_status=500>>\nvalue = None\n\n    def assert_success(response, value=None):\n        \"\"\"\n        Verify that the provided webdriver.Response instance described\n        a valid success response as defined by `dfn-send-a-response` and\n        the provided response value.\n    \n        :param response: ``webdriver.Response`` instance.\n        :param value: Expected value of the response body, if any.\n        \"\"\"\n>       assert response.status == 200, str(response.error)\nE       AssertionError: javascript error (500): JS evaluation raised an exception\nE         \nE       assert 500 == 200\nE        +  where 500 = <Response status=500 error=<JavascriptErrorException http_status=500>>.status\n\nresponse   = <Response status=500 error=<JavascriptErrorException http_status=500>>\nvalue      = None\n\ntests/wpt/web-platform-tests/webdriver/tests/support/asserts.py:67: AssertionError", 
    "subtest": "test_element_in_collection", 
    "test": "/webdriver/tests/execute_script/cyclic.py", 
    "line": 1338, 
    "action": "test_result", 
    "expected": "PASS"
}

georgeroman · 2019-08-31T07:14:20Z

Hmm, this is the same test that strangely failed on Mac in #23725. It now also fails on Linux, that's weird, especially since this PR should have no influence on it.


        Implement ElementClick wd command

georgeroman · 2019-09-01T21:43:18Z

@bors-servo try=wpt

bors-servo · 2019-09-01T21:43:23Z

⌛ Trying commit 32f7254 with merge 1c13c9a...


        Auto merge of #24090 - georgeroman:implement_element_click_wd_command…

…, r=<try> Implement ElementClick wd command  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors  - [X] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24090)

bors-servo · 2019-09-01T22:33:15Z

💔 Test failed - linux-rel-wpt

jdm · 2019-09-04T15:02:41Z

@bors-servo r+

bors-servo · 2019-09-04T15:02:42Z

📌 Commit 32f7254 has been approved by jdm

bors-servo · 2019-09-04T16:25:52Z

⌛ Testing commit 32f7254 with merge 068997c...


        Auto merge of #24090 - georgeroman:implement_element_click_wd_command…

…, r=jdm Implement ElementClick wd command  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors  - [X] There are tests for these changes    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/24090)

bors-servo · 2019-09-04T18:20:25Z

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: jdm
Pushing 068997c to master...

highfive assigned paulrouget Aug 28, 2019

highfive added the S-awaiting-review label Aug 28, 2019

jdm assigned jdm and unassigned paulrouget Aug 28, 2019

jdm requested changes Aug 29, 2019

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Aug 29, 2019

highfive added S-awaiting-review and removed S-needs-code-changes labels Aug 29, 2019

jdm approved these changes Aug 29, 2019

View changes

highfive added S-awaiting-merge and removed S-awaiting-review labels Aug 30, 2019

highfive added S-tests-failed and removed S-awaiting-merge labels Aug 30, 2019

Implement ElementClick wd command

Loading status checks…

32f7254

highfive added S-awaiting-review and removed S-tests-failed labels Aug 31, 2019

highfive added the S-tests-failed label Sep 1, 2019

highfive added S-awaiting-merge and removed S-awaiting-review S-tests-failed labels Sep 4, 2019

bors-servo merged commit 32f7254 into servo:master Sep 4, 2019
3 checks passed

3 checks passed

Taskcluster (pull_request) TaskGroup: success
Details

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Sep 4, 2019

servo / servo

Implement ElementClick wd command #24090

Implement ElementClick wd command #24090

georgeroman commented Aug 28, 2019 •

edited by SimonSapin

highfive commented Aug 28, 2019

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

georgeroman commented Aug 30, 2019

jdm commented Aug 30, 2019

bors-servo commented Aug 30, 2019

bors-servo commented Aug 30, 2019

bors-servo commented Aug 30, 2019

CYBAI commented Aug 30, 2019

georgeroman commented Aug 31, 2019 •

edited

georgeroman commented Sep 1, 2019

bors-servo commented Sep 1, 2019

bors-servo commented Sep 1, 2019

jdm commented Sep 4, 2019

bors-servo commented Sep 4, 2019

bors-servo commented Sep 4, 2019

bors-servo commented Sep 4, 2019

servo / servo

Join GitHub today

Implement ElementClick wd command #24090

Implement ElementClick wd command #24090

Conversation

georgeroman commented Aug 28, 2019 • edited by SimonSapin

highfive commented Aug 28, 2019

This comment has been minimized.

jdm Aug 29, 2019

This comment has been minimized.

jdm Aug 29, 2019

This comment has been minimized.

jdm Aug 29, 2019

georgeroman commented Aug 30, 2019

jdm commented Aug 30, 2019

bors-servo commented Aug 30, 2019

bors-servo commented Aug 30, 2019

bors-servo commented Aug 30, 2019

CYBAI commented Aug 30, 2019

georgeroman commented Aug 31, 2019 • edited

georgeroman commented Sep 1, 2019

bors-servo commented Sep 1, 2019

bors-servo commented Sep 1, 2019

jdm commented Sep 4, 2019

bors-servo commented Sep 4, 2019

bors-servo commented Sep 4, 2019

bors-servo commented Sep 4, 2019

georgeroman commented Aug 28, 2019 •

edited by SimonSapin

georgeroman commented Aug 31, 2019 •

edited