Skip to content

Deprecate driver methods not possible with W3C WebDriver.#828

Closed
mparker17 wants to merge 1 commit intominkphp:masterfrom
mparker17:deprecate-non-w3c-driver-methods
Closed

Deprecate driver methods not possible with W3C WebDriver.#828
mparker17 wants to merge 1 commit intominkphp:masterfrom
mparker17:deprecate-non-w3c-driver-methods

Conversation

@mparker17
Copy link

Problem / motivation

This project (minkphp/Mink) defines a common interface (\Behat\Mink\Driver\DriverInterface) and base implementation (\Behat\Mink\Driver\CoreDriver) for Drivers: code which remotely/headlessly controls a browser.

At time-of-writing (2022-04-19), packagist reports that the most-used Mink driver was behat/mink-selenium2-driver, which controls a browser using the Selenium WebDriver 2 protocol. Happily, it appears that a Selenium WebDriver 3 endpoint can understand behat/mink-selenium2-driver without any changes.

The most-recent Selenium WebDriver 2 release, 2.53.1, was on 2016-06-30 (about 6 years ago). The most-recent Selenium WebDriver 3 release, 3.150.0, was on 2019-08-22 (about 2.5 years ago).

Since then, Selenium WebDriver 4.0.0 and 4.1.0 were released. Also, the Selenium Webdriver 4 standard was adopted by the W3C as WebDriver1 and has become a Recommendation; and a W3C WebDriver2 standard has reached Working Draft status.

Selenium 4 / W3C WebDriver has had some notable changes, including a new wire protocol; but also some notable deprecations / removals, such as removing support for HTTP response parsing and getting the HTTP response status code (e.g.: 200, 301, 404, etc.).

I think that it is reasonable to anticipate that, at some point in the future, some of the projects which this project (minkphp/Mink) abstracts through its drivers (e.g.: ChromeDriver, BrowserStack Automate, etc.) will eventually drop support for Selenium 2 and 3 in favor of the W3C's WebDriver protocol.

I propose that the Mink community begin to consider small changes to \Behat\Mink\Driver\DriverInterface and \Behat\Mink\Driver\CoreDriver, in order to reflect the direction that the W3C is taking the WebDriver protocol, so that when support for Selenium 2 and 3 is dropped by "downstream" projects, it will not be as painful for Mink users, i.e.: so they will not be caught off-guard by driver methods that can no longer do what they describe.

(note: this issue isn't intended to address all of the Driver-related changes between Selenium 3 and 4 - much of this can and should be left to downstream drivers. To some extent, even if we don't end up changing Mink at all, I at least hope this issue will start a productive Selenium4/W3C WebDriver support discussion 😄 )

Proposed resolution

In particular, there are three methods defined in \Behat\Mink\Driver\DriverInterface and \Behat\Mink\Driver\CoreDriver which can no longer be completed with W3C WebDriver:

  1. \Behat\Mink\Driver\DriverInterface::setRequestHeader()
    • This method was actually not supported in Selenium 2 or 3 either, and an implementation is not present in behat/mink-selenium2-driver
  2. \Behat\Mink\Driver\DriverInterface::getResponseHeaders()
  3. \Behat\Mink\Driver\DriverInterface::getStatusCode()
    • getStatusCode() is, in some ways, a special case of getResponseHeaders()
    • Work-arounds for Selenium 4 do exist, but they involve using Selenium to open and use the browser's DevTools to find out the response status code and headers - and this is made uglier because the DevTools in Chromium-based browsers are different from the DevTools in Firefox-based browsers.
    • For more information on why these were removed, see SeleniumHQ/selenium-google-code-issue-archive#141

I propose that we mark DriverInterface::setRequestHeader(), DriverInterface::getResponseHeaders(), and DriverInterface::getStatusCode() as deprecated by:

  1. Adding @deprecated to the DriverInterface docblocks (with an explanation); and;
  2. Adding calls to @trigger_error(__METHOD__ . ' is deprecated...', E_USER_DEPRECATED); to the corresponding functions in CoreDriver

... I've made these changes in this merge request.

Out-of-scope

Note that \Behat\Mink\Driver\DriverInterface::setBasicAuth() no longer directly corresponds with a W3C WebDriver method; but it can be worked-around relatively easily by passing the HTTP Basic Auth credentials as part of the URL in \Behat\Mink\Driver\DriverInterface::visit()

Notes and links

  • php-webdriver/webdriver provides Selenium 4 / W3C WebDriver1 bindings for PHP.
  • There is not yet a mink-selenium4-driver, but I may publish one soon (it would depend on php-webdriver/webdriver).
    • Part of the reason why I'm filing this issue is to get a sense of the direction that minkphp/Mink is going to take in the future, which will help clarify what goes into / belongs in a mink-selenium4-driver package

Thanks in advance for your understanding and patience.

@codecov
Copy link

codecov bot commented Apr 19, 2022

Codecov Report

Merging #828 (85a03e8) into master (19e5890) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master     #828   +/-   ##
=========================================
  Coverage     98.47%   98.47%           
  Complexity      345      345           
=========================================
  Files            23       23           
  Lines           983      986    +3     
=========================================
+ Hits            968      971    +3     
  Misses           15       15           
Impacted Files Coverage Δ
src/Driver/CoreDriver.php 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 19e5890...85a03e8. Read the comment docs.

@mvorisek
Copy link
Contributor

what about window size? see instaclick/php-webdriver@dc18e7f

@stof
Copy link
Member

stof commented Apr 20, 2022

status code and request header are support by some other drivers though. I would not deprecate those as it would break valid use cases. It is totally valid for some drivers to mark some functions as unsupported (Selenium 2 does not support those methods either).

the ones that should actually be deprecated are the separate keydown, keypress and keyup methods in favor of a single pressKey method to match what is actually possible in browsers.

There is not yet a mink-selenium4-driver, but I may publish one soon (it would depend on php-webdriver/webdriver).

* Part of the reason why I'm filing this issue is to get a sense of the direction that minkphp/Mink is going to take in the future, which will help clarify what goes into / belongs in a `mink-selenium4-driver` package

see minkphp/MinkSelenium2Driver#298 and minkphp/MinkSelenium2Driver#293 for existing discussions about that. But I never started the work.

@mparker17
Copy link
Author

Sorry for the long delay in answering! It's been a busy couple of weeks for me!


status code and request header are support by some other drivers though. I would not deprecate those as it would break valid use cases. It is totally valid for some drivers to mark some functions as unsupported (Selenium 2 does not support those methods either).

Fair enough; makes sense to me!

the ones that should actually be deprecated are the separate keydown, keypress and keyup methods in favor of a single pressKey method to match what is actually possible in browsers.

I'll look into doing this in a separate merge request. Thanks!

see minkphp/MinkSelenium2Driver#298 and minkphp/MinkSelenium2Driver#293 for existing discussions about that. But I never started the work.

I will check out those issues - I think I have some code that I can contribute! 😁


Thanks again for your understanding and patience with me!

@mparker17 mparker17 closed this May 4, 2022
@mparker17 mparker17 deleted the deprecate-non-w3c-driver-methods branch May 4, 2022 14:30
@mparker17
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants