Closed Bug 1568327 Opened 4 months ago Closed 4 months ago

WebDriver - PerformActions should return InvalidArgument when passed an action sequence with an invalid id

Categories

(Testing :: geckodriver, defect, P3)

defect

Tracking

(firefox70 fixed)

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: george.roman.99, Assigned: george.roman.99)

References

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

I'm implementing actions support in Servo: https://github.com/servo/servo/pull/23805

Actual results:

No matter whether the id property exists or not, an Optional<String> is always returned:
https://searchfox.org/mozilla-central/source/testing/webdriver/src/command.rs#315
https://searchfox.org/mozilla-central/source/testing/webdriver/src/actions.rs#10

Expected results:

An InvalidArgument error should be returned when processing an action sequence with a missing or invalid id property.

Interesting. Does removing the Option, and the Serde field attribute above fix it for you?

Status: UNCONFIRMED → NEW
Type: enhancement → defect
Ever confirmed: true
Flags: needinfo?(george.roman.99)
Priority: -- → P3

Unfortunately I don't think that removing those fields would fix this as we need to return an InvalidArgument error when the id parameter is missing. Some additional check like in https://searchfox.org/mozilla-central/source/testing/webdriver/src/command.rs#306 would probably be needed.

Flags: needinfo?(george.roman.99)

Change ActionSequence's id field from Option<String> to String

Assignee: nobody → george.roman.99
Blocks: 1520585
Status: NEW → ASSIGNED
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/facd6af4e16c
Make ActionSequence's id field mandatory r=whimboo

I totally missed that the wdspec tests and/or the webdriver client might also need an update. As it looks like the id isn't yet or wrongly specified. The client can be found at: https://searchfox.org/mozilla-central/source/testing/web-platform/tests/tools/webdriver/webdriver/client.py

The wdspec tests can be run like:

$ mach wpt testing/web-platform/tests/webdriver/tests/perform_actions/ --webdriver-binary target/debug/geckodriver --webdriver-arg=-vv

I updated the revision, added a test for this particular case and now the tests pass.

Flags: needinfo?(george.roman.99)
Attachment #9081441 - Attachment description: Bug 1568327 - Make ActionSequence's id field mandatory r=ato,whimboo → Bug 1568327 - [geckodriver] Make ActionSequence's id field mandatory. r=ato,whimboo
Pushed by hskupin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fff7b5299ecf
[geckodriver] Make ActionSequence's id field mandatory. r=whimboo
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/18242 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.