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

The "EventsTest::testRightClick" test fails on Selenium 3.x #381

Closed
aik099 opened this issue Feb 18, 2024 · 9 comments · Fixed by #407
Closed

The "EventsTest::testRightClick" test fails on Selenium 3.x #381

aik099 opened this issue Feb 18, 2024 · 9 comments · Fixed by #407
Labels

Comments

@aik099
Copy link
Member

aik099 commented Feb 18, 2024

When the \Behat\Mink\Tests\Driver\Js\EventsTest::testRightClick test is executed on Selenium 3.x it fails with this message:

Failed asserting that two strings are equal.
Expected :'right clicked'
Actual   :'single clicked'

The driver code seems to be written correctly (I've double-checked with the Facebook's driver version: https://github.com/php-webdriver/php-webdriver/blob/5d8e66ff849b5614015d35e99d759252e371ce26/lib/Remote/RemoteMouse.php#L100 ).

Not sure why the test fails.

More research is done in #354 (comment) .

The fix was done in #372 , but it's using Actions , which isn't implemented in the WebDriver.

@aik099
Copy link
Member Author

aik099 commented Oct 31, 2024

The https://github.com/instaclick/php-webdriver doesn't seem to be maintained anymore. Without fixing it there is no way to make right-clicking work.

I'm proposing to release this driver as-is so that developers can have almost full Selenium 3 support (without right-clicking) until the https://github.com/minkphp/webdriver-classic-driver is released.

@uuf6429
Copy link
Member

uuf6429 commented Jan 9, 2025

If it can't be fixed, why not skip the test (with a reasonable skip message)?

@aik099
Copy link
Member Author

aik099 commented Jan 9, 2025

If you can manage to invoke the right-click action using instaclick/web-driver on Selenium 3, then it can be fixed.

I wasn't able to understand how to make it happen.

I don't use right-clicking in my tests, but maybe somebody does.

@uuf6429
Copy link
Member

uuf6429 commented Jan 10, 2025

@aik099 while running the test on chrome, the selenium docker service has the following output:

selenium+chrome docker service output
2025-01-10T21:36:34.461004430Z 21:36:34.460 INFO [ActiveSessionFactory.apply] - Capabilities are: {
2025-01-10T21:36:34.461989114Z   "browserName": "chrome",
2025-01-10T21:36:34.461998895Z   "name": "Behat Test"
2025-01-10T21:36:34.462001371Z }
2025-01-10T21:36:34.462003556Z 21:36:34.460 INFO [ActiveSessionFactory.lambda$apply$11] - Matched factory org.openqa.selenium.grid.session.remote.ServicedSession$Factory (provider: org.openqa.selenium.chrome.ChromeDriverService)
2025-01-10T21:36:34.491335455Z Starting ChromeDriver 94.0.4606.61 (418b78f5838ed0b1c69bb4e51ea0252171854915-refs/branch-heads/4606@{#1204}) on port 32163
2025-01-10T21:36:34.491384414Z Only local connections are allowed.
2025-01-10T21:36:34.491388199Z Please see https://chromedriver.chromium.org/security-considerations for suggestions on keeping ChromeDriver safe.
2025-01-10T21:36:34.504852290Z ChromeDriver was started successfully.
2025-01-10T21:36:35.079217644Z 21:36:35.078 INFO [ProtocolHandshake.createSession] - Detected dialect: W3C
2025-01-10T21:36:35.081419939Z 21:36:35.081 INFO [RemoteSession$Factory.lambda$performHandshake$0] - Started new session 0e99e5c1a03674a41ab2ff2ecf09efb4 (org.openqa.selenium.chrome.ChromeDriverService)
2025-01-10T21:36:35.740005486Z 21:36:35.739 INFO [ActiveSessions$1.onStop] - Removing session 0e99e5c1a03674a41ab2ff2ecf09efb4 (org.openqa.selenium.chrome.ChromeDriverService)

This part is particularly suspicious: Detected dialect: W3C

From what I know, Selenium in this version can switch between W3C and JWP (JsonWireProtocol AKA OSS).

I've found out that I can force chrome to disable W3C, but it's not very well documented. When I apply the following patch to the MinkSelenium2Driver, the test passes (for chrome):

diff --git a/src/Selenium2Driver.php b/src/Selenium2Driver.php
index fe8b165..83b4050 100755
--- a/src/Selenium2Driver.php
+++ b/src/Selenium2Driver.php
@@ -93,6 +93,7 @@ class Selenium2Driver extends CoreDriver
      */
     public function __construct(string $browserName = 'firefox', ?array $desiredCapabilities = null, string $wdHost = 'http://localhost:4444/wd/hub')
     {
+        $desiredCapabilities['goog:chromeOptions'] = array_merge($desiredCapabilities['goog:chromeOptions'] ?? [], ['w3c' => false]);
         $this->setBrowserName($browserName);
         $this->setDesiredCapabilities($desiredCapabilities);
         $this->setWebDriver(new WebDriver($wdHost));

Similarly, this is the selenium docker service output for firefox:

selenium+firefox docker service output
2025-01-10T22:06:48.959611026Z 22:06:48.959 INFO [ActiveSessionFactory.apply] - Capabilities are: {
2025-01-10T22:06:48.959652308Z   "browserName": "firefox",
2025-01-10T22:06:48.959656000Z   "name": "Behat Test"
2025-01-10T22:06:48.959657897Z }
2025-01-10T22:06:48.959659526Z 22:06:48.959 INFO [ActiveSessionFactory.lambda$apply$11] - Matched factory org.openqa.selenium.grid.session.remote.ServicedSession$Factory (provider: org.openqa.selenium.firefox.GeckoDriverService)
2025-01-10T22:06:48.961584562Z 1736546808961	geckodriver	INFO	Listening on 127.0.0.1:6153
2025-01-10T22:06:48.968428331Z 1736546808967	mozrunner::runner	INFO	Running command: "/usr/bin/firefox" "--marionette" "-foreground" "-no-remote" "-profile" "/tmp/rust_mozprofile1lvfTm"
2025-01-10T22:06:49.302135727Z [GFX1-]: glxtest: libpci missing
2025-01-10T22:06:49.302177941Z [GFX1-]: glxtest: libEGL missing
2025-01-10T22:06:49.321587639Z 1736546809321	Marionette	INFO	Marionette enabled
2025-01-10T22:06:49.803655945Z console.warn: SearchSettings: "get: No settings file exists, new profile?" (new NotFoundError("Could not open the file at /tmp/rust_mozprofile1lvfTm/search.json.mozlz4", (void 0)))
2025-01-10T22:06:50.210024822Z 1736546810209	Marionette	INFO	Listening on port 33701
2025-01-10T22:06:50.288245090Z 22:06:50.287 INFO [ProtocolHandshake.createSession] - Detected dialect: W3C
2025-01-10T22:06:50.290899076Z 22:06:50.290 INFO [RemoteSession$Factory.lambda$performHandshake$0] - Started new session 64a857e1-ceb5-4be8-8d46-35f5d456e7a4 (org.openqa.selenium.firefox.GeckoDriverService)
2025-01-10T22:06:50.890965094Z 1736546810890	Marionette	INFO	Stopped listening on port 33701
2025-01-10T22:06:50.936366953Z JavaScript error: resource:///modules/Interactions.jsm, line 230: NS_ERROR_FAILURE: Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIUserIdleService.removeIdleObserver]
2025-01-10T22:06:50.943973789Z console.error: Region.jsm: "Error fetching region" (new TypeError("NetworkError when attempting to fetch resource.", ""))
2025-01-10T22:06:50.944100953Z console.error: Region.jsm: "Failed to fetch region" (new Error("NO_RESULT", "resource://gre/modules/Region.jsm", 419))
2025-01-10T22:06:52.366091354Z 
2025-01-10T22:06:52.366133868Z ###!!! [Parent][RunMessage] Error: Channel closing: too late to send/recv, messages will be lost
2025-01-10T22:06:52.366138368Z 
2025-01-10T22:06:52.447681366Z 22:06:52.447 INFO [ActiveSessions$1.onStop] - Removing session 64a857e1-ceb5-4be8-8d46-35f5d456e7a4 (org.openqa.selenium.firefox.GeckoDriverService)

There are some follow up questions though:

  • should we hard-code the chrome option (since the driver is effectively broken without it)?
  • is it fine to keep the chrome option permanently? (I'm guessing non-chrome browsers will ignore that - firefox does ignore the chrome one, for instance)
  • apparently there isn't such an option for firefox/geckodriver, mainly because that firefox setup does not support JWP anymore: The docker image contains firefox 92 and geckodriver 0.29 - geckodriver stops supporting JWP from v0.24 up.

@uuf6429
Copy link
Member

uuf6429 commented Jan 10, 2025

On the other hand, I'm wondering why Selenium is not translating our JWP calls to W3C correctly.

So other potential solutions would be:

  1. figure out and solve selenium's translation mechanism (see the next/below comment)
  2. make our driver send W3C (action) requests, bypassing the instaclick webdriver
  3. use one of those instaclick webdriver forks (essentially bringing back the PR that has been decline)
  4. limit this driver to selenium 2 and below (again I don't get why we keep trying to fix this)

@uuf6429
Copy link
Member

uuf6429 commented Jan 11, 2025

By the way, just now I found the root cause of this problem: SeleniumHQ/selenium#7393 - selenium loses / doesn't translate the {button: 2} part.

You can see the faulty code and a fix (only applied to Selenium 4) here: SeleniumHQ/selenium@085ceed

@aik099
Copy link
Member Author

aik099 commented Jan 11, 2025

  1. use one of those instaclick webdriver forks (essentially bringing back the PR that has been decline)

I've tried to use the lullabot/php-webdriver, but got a Cannot call non W3C standard command while in W3C mode exception, when calling $this->session->window_handle(); code. This suggests, that we shouldn't go that path.

  1. limit this driver to selenium 2 and below (again I don't get why we keep trying to fix this)

I'm trying to make the test suite green -> make a final release -> advocate everybody to use the webdriver-classic-driver driver.

I'm not planning to abandon this driver completely because it does have differences in the setValue method (and maybe people rely on them) compared to the webdriver-classic-driver driver.


Considering all your findings I can now do 2 things:

  1. ignore the right clicking test on Selenium 3
  2. in the driver's rightClick method throw an exception when used on a Selenium 3

.

@uuf6429
Copy link
Member

uuf6429 commented Jan 11, 2025

I'm trying to make the test suite green -> make a final release -> advocate everybody to use the webdriver-classic-driver driver.

I'm not planning to abandon this driver completely because it does have differences in the setValue method (and maybe people rely on them) compared to the webdriver-classic-driver driver.

I'm not saying to abandon it - however the facts are clear:

  1. it doesn't work in selenium 3
  2. anyone using it in selenium 3 is theoretically misusing it
  3. probably no one is using it on selenium 3, because they can't do so effectively

Since you'd be adding a selenium version check anyway, I would just make it throw an exception saying that it won't work with selenium 3.
Users still have a variety of options:

  • stick to selenium 2
  • upgrade to selenium 4 (I wouldn't recommend it though)
  • switch to webdriver-classic-driver

That said, I'm also fine with your proposal. I just don't feel it is worth the trouble.

@aik099
Copy link
Member Author

aik099 commented Jan 11, 2025

The latest release of this driver does work (except for window/frame management) on Selenium 3 and even on Selenium 4 (if desired capabilities are given in a new format with the firstMatch key).

The current code in the master branch restores support for window/frame management for Selenium 3+.

Here is the PR with a proposed implementation: #407 . Please review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants