W3C-compliant implementation, native geckodriver support #560

dunglas · 2018-04-09T14:32:09Z

This PR provides an implementation of the W3C flavor of the WebDriver protocol.This implem is good enough to make the Symfony Panther's test suite green when using the latest version of geckodriver.
It is 100% backward compatible with the current (JsonWire) implementation.

The (only?) missing part is the capabilities support. It's a step to close #469.

TODO:

Add geckodriver in Travis to run the project's test suite against a valid implementation of the W3C protocol.

Subsequent work (in other PRs?):

Add new classes for exceptions that wasn't part of JsonWire
Add support for capabilities and other missing parts

lib/Remote/HttpCommandExecutor.php

lib/Remote/JsonWireCompat.php

OndraM

Hi @dunglas , thanks for the great PR! And sorry for delay... I have few minor notes in the PR, and besides I' have a question - I see you plan to hardcode in Panther the w3c_compliant parameter when using Firefox, but I see this as temporary workaround - in future (and perhaps in different PR) there should be implemented some kind of handshake with the remote end, based on which the protocol can be autodetected. Do you plan implementing this or you don't need it right now? Because until this is implemented, people using php-webdriver directly won't benefit from the W3C protocol support, as they would not have an easy way how to trigger the W3C-compliant mode.

OndraM · 2018-04-13T14:16:50Z

lib/Cookie.php

-        return $this->cookie;
+        $cookie = $this->cookie;
+        if (null === $cookie['secure']) {
+            // Passing a boolean value for the "secure" flag is mandatory when using geckodriver


Why is this? Is it bug or feature? Is there an issue in geckodriver for this?

that's indeed a bug. The W3C spec marks it as optional: https://www.w3.org/TR/webdriver1/#dfn-table-for-cookie-conversion

Yes it's a bug... But we have to workaround it for Geckodriver support.

have you reported it to the geckodriver team ?

Chrome now has the same behavior: Codeception/Codeception@ec2fc73#diff-b69c529efcc6368b5c564f8b36665a22R827

We should leave this as is.

lib/Exception/WebDriverException.php

OndraM · 2018-09-12T10:12:56Z

lib/Remote/HttpCommandExecutor.php

@@ -242,7 +262,7 @@ public function execute(WebDriverCommand $command)
            }
        }

-        if ($params && is_array($params) && $http_method !== 'POST') {
+        if ($params && \is_array($params) && $http_method !== 'POST') {


I wouldn't change this as part of this PR (also on other lines).

The codestyle may be changed, but then we can do this at once in whole codebase, and do not change it randomly to keep this PR self-contained.

I'll revert these changes (can't remember why I've done that)

probably phpstorm suggestion :)

lib/Remote/RemoteWebElement.php

OndraM · 2018-09-12T10:20:44Z

lib/Remote/RemoteWebElement.php

@@ -371,6 +388,18 @@ public function setFileDetector(FileDetector $detector)
     */
    public function submit()
    {
+        if ($this->w3cCompliant) {
+            $this->executor->execute(DriverCommand::EXECUTE_SCRIPT, [


I guess this will face this issue SeleniumHQ/selenium#3398 in some forms:

If a form control (such as a submit button) has a name or id of submit it will mask the form's submit method.

See referenced issue for more universal way (using prototype method).

I already use the prototype method. Am I missing something?

lib/Remote/RemoteWebElement.php

lib/Remote/JsonWireCompat.php

OndraM · 2018-09-12T10:39:01Z

@dunglas BTW have you seen https://github.com/facebook/php-webdriver/wiki/JsonWireProtocol-vs.-W3C-WebDriver ?

stof · 2018-09-12T13:41:26Z

@OndraM IIRC, the body of the New session response allows to identify whether it is a W3C or JsonWire response (and the request for the new session works the same in both protocols IIRC). so this would allow detecting automatically whether the session runs in W3C mode or JsonWire mode.

OndraM · 2018-09-12T13:43:53Z

@stof Right, for example how its done in:

ProtocolHandshake in java bindings
Handshake and capabilities resolving in python bindings

dunglas · 2018-09-25T10:22:31Z

Not yet.

.travis.yml

dunglas · 2018-09-28T06:52:05Z

Protocol handshake added. And tests are all green locally with Geckodriver.

.travis.yml

lib/Exception/WebDriverException.php

lib/Remote/HttpCommandExecutor.php

lib/Remote/JsonWireCompat.php

lib/Remote/RemoteMouse.php

lib/Remote/RemoteWebDriver.php

stof · 2018-09-28T08:18:50Z

lib/Remote/RemoteWebDriver.php

+        ];
+
+        // Legacy protocol
+        if (null !== $required_capabilities && $required_capabilities_array = $required_capabilities->toArray()) {


is this case really part of the legacy protocol, or should the comment placed above the next if instead ?

I'm not sure... ping @OndraM

stof · 2018-09-28T08:20:20Z

lib/Remote/RemoteWebDriver.php

@@ -149,15 +182,17 @@ public static function createBySessionID(
        $connection_timeout_in_ms = null,
        $request_timeout_in_ms = null
    ) {
-        $executor = new HttpCommandExecutor($selenium_server_url);
+        // BC layer to not break the method signature


should you trigger a deprecation warning if a child class extends this method without adding the argument ?

I've not done added it because the library currently doesn't trigger any deprecation. But why not. WDYT @OndraM?

lib/Remote/RemoteWebDriver.php

lib/Remote/RemoteWebElement.php

dunglas · 2018-10-01T09:57:45Z

Tests are green (including when using Geckodriver directly), and all issues are resolved.
As Geckodriver is working, it's already a large step forward to full W3C compatibility.

Do you think we can merge it as an "experimental" feature @OndraM? We'll still be able to improve things if needed after.

tests/functional/RemoteWebDriverFindElementTest.php

dunglas · 2019-10-28T17:37:59Z

@OndraM while I'm here... :) It would be awesome if this patch could be included in the release adding support for Symfony 5 (#670)!

OndraM · 2019-11-05T15:39:36Z

It seems Geckodriver meanwhile started enforcing more parts of W3C protocol - in this case it requires POST body to be always valid JSON (even if empty), as per Processing model (5.1).

So I updated HttpCommandExecutor accordingly - lets see if it fixes the builds 🍀 .

dunglas · 2019-11-05T17:33:36Z

And it's now green! Thank you for rebasing and fixing the remaining issues!

OndraM · 2019-11-09T23:53:32Z

Well, finally merged to community branch! 🎉 I will try to merge few other active PRs now, then tag a new version and get things rolling.

Once again thanks @dunglas for your work!

OndraM · 2019-11-10T01:21:27Z

Now I'm working on issue with ChromeOptions capability which emerged after this was merged.

The handshake now breaks newer selenium servers - which will not allow chromeOptions in firstMatch W3C capabilities (because it is against the spec).

Facebook\WebDriver\Exception\UnknownServerException: Illegal key values seen in w3c capabilities: [chromeOptions]

However if we just rename chromeOptions to goog:chromeOptions, we would break it for Chromedriver <2.31 which added support for both of them...

OndraM · 2019-11-11T14:40:07Z

Ok, having some good news.

Issue with chromeOptions was resolved and merged in #673.

In #674 I also removed workaround which forced Chrome to always use JsonWire OSS protocol. Now we always let the handshake determine the proper protocol. I also added Chromedriver builds to make sure it works with both OSS and W3C protocols - and the tests are passing 🎉 . (The coverage we have is far from being perfect, so there may be some unresolved issues with W3C protocol - in this case one can explicitly disable W3C protocol on Chromedriver, see issue description in #674.)

So the community branch could now officialy undergo some testing of W3C protocol.

facebook-github-bot added the CLA Signed label Apr 9, 2018

dunglas force-pushed the geckodriver branch from 5f30a57 to 4dc8416 Compare April 9, 2018 14:41

mickaelandrieu reviewed Apr 9, 2018

View reviewed changes

lib/Remote/HttpCommandExecutor.php Outdated Show resolved Hide resolved

dunglas force-pushed the geckodriver branch 3 times, most recently from f66a990 to 30862ff Compare April 9, 2018 14:47

dunglas mentioned this pull request Apr 9, 2018

Native Firefox support (through geckodriver) symfony/panther#14

Merged

dunglas force-pushed the geckodriver branch 3 times, most recently from 161bcca to be980ed Compare June 11, 2018 14:40

stof reviewed Jul 5, 2018

View reviewed changes

lib/Remote/JsonWireCompat.php Outdated Show resolved Hide resolved

williamdes mentioned this pull request Sep 7, 2018

Session [(null externalkey)] not available and is not among the last 1000 terminated sessions. Active sessions are[ext. key xa154aa5-4bef-41c1-a0b8-610c462493a6] #578

Closed

OndraM requested changes Sep 12, 2018

View reviewed changes

This was referenced Sep 20, 2018

W3c #599

Closed

Mozilla firefox geckodriver fix. #575

Closed

OndraM changed the title ~~native geckodriver support, partial W3C implementation~~ Partial W3C implementation, native geckodriver support Sep 20, 2018

OndraM mentioned this pull request Sep 20, 2018

Update platform capability to match the w3c spec #461

Closed

This was referenced Sep 24, 2018

Expose whether a session is using W3C or JsonWire instaclick/php-webdriver#86

Closed

Support the W3C webdriver spec minkphp/MinkSelenium2Driver#293

Closed

dunglas force-pushed the geckodriver branch from 127f5c9 to 0630dba Compare September 26, 2018 11:55

stof reviewed Sep 26, 2018

View reviewed changes

.travis.yml Outdated Show resolved Hide resolved

stof reviewed Sep 28, 2018

View reviewed changes

stof reviewed Oct 1, 2018

View reviewed changes

tests/functional/RemoteWebDriverFindElementTest.php Outdated Show resolved Hide resolved

dunglas force-pushed the geckodriver branch from d6a67ef to e474771 Compare October 1, 2018 13:14

tienvx mentioned this pull request Oct 15, 2019

Operation timed out after 30001 milliseconds with 0 bytes received symfony/panther#235

Closed

OndraM force-pushed the geckodriver branch from 61a5aad to 66a6593 Compare November 3, 2019 00:33

dunglas added 3 commits November 9, 2019 23:06

W3C-compatible implementation

87be633

Add back Geckodriver to the Travis matrix

4a65bed

Improve tests stability

5729355

OndraM force-pushed the geckodriver branch from 306c213 to 0801483 Compare November 9, 2019 22:07

OndraM added 2 commits November 9, 2019 23:07

Use latest geckodriver

721a7c2

Make POST body always valid JSON in W3C mode

71a6698

OndraM approved these changes Nov 9, 2019

View reviewed changes

OndraM force-pushed the geckodriver branch from 0801483 to 7119d7f Compare November 9, 2019 22:21

Codestyle: unify casing in w3c methods

7119d7f

This was referenced Nov 9, 2019

Compatibility with Symfony 5 #670

Merged

Bugfix. Undefined index: ELEMENT, from Codeception #669

Closed

OndraM merged commit 6dd8b58 into php-webdriver:community Nov 9, 2019

This was referenced Nov 10, 2019

Makes it work with Chrome 77 #663

Closed

Fixing Chrome 77 #664

Closed

Problems working with chrome driver 78 #672

Closed

Make chromeOptions compatible with both JsonWire and W3C protocols #673

Merged

Enable W3C protocol for Chrome 🎉 #674

Merged

OndraM mentioned this pull request Nov 11, 2019

Geckodriver 0.24.0 #654

Closed

dmitryuk mentioned this pull request Dec 12, 2019

New release? #710

Closed

OndraM mentioned this pull request Dec 24, 2019

UnknownCommandException mouseMoveTo when calling moveToElement() #462

Closed

driesvints mentioned this pull request Feb 11, 2020

sendKeys on RemoteWebElement no longer accepts an array #758

Closed

tomaszszopinski mentioned this pull request May 4, 2020

[BEHAT] Updated selenium container to 20200326 version ezsystems/ezplatform#538

Merged

W3C-compliant implementation, native geckodriver support #560

W3C-compliant implementation, native geckodriver support #560

Uh oh!

Conversation

dunglas commented Apr 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OndraM left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

OndraM commented Sep 12, 2018

Uh oh!

stof commented Sep 12, 2018

Uh oh!

OndraM commented Sep 12, 2018

Uh oh!

dunglas commented Sep 25, 2018 via email

Uh oh!

Uh oh!

dunglas commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dunglas commented Oct 1, 2018

Uh oh!

Uh oh!

dunglas commented Oct 28, 2019

Uh oh!

OndraM commented Nov 5, 2019

Uh oh!

dunglas commented Nov 5, 2019

Uh oh!

OndraM commented Nov 9, 2019

Uh oh!

OndraM commented Nov 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

dunglas commented Apr 9, 2018 •

edited

Loading

dunglas commented Sep 28, 2018 •

edited

Loading

OndraM commented Nov 10, 2019 •

edited

Loading