minkphp / webdriver-classic-driver

Mink driver for the W3C WebDriver Classic protocol
MIT License
3 stars 5 forks source link

Replace usage of "arguments[0]" with "{{ELEMENT}}" #9

Closed aik099 closed 9 months ago

aik099 commented 9 months ago

The https://github.com/minkphp/MinkSelenium2Driver/blob/master/src/Selenium2Driver.php uses {{ELEMENT}} to mask the internal element representation of an element in the driver (my understanding). Later it replaces it with arguments[0] when preparing JS code for a WebDriver.

This driver, however, exposes arguments[0] instead.

I think, that this also is a BC break, because developers now can't switch to this driver without replacing {{ELEMENT}} across their test suite first.

Please use the {{ELEMENT}} as in the current Mink's Selenium2 driver (or at least in the public API of the driver).

uuf6429 commented 9 months ago

This driver, however, exposes arguments[0] instead.

This driver avoids the extra step and simply passes valid JS code directly for execution (the invalid syntax warnings was one of the motivations to remove it).

I think, that this also is a BC break,

It can't be so since this is an entirely new package.

because developers now can't switch to this driver without replacing {{ELEMENT}} across their test suite first.

Why is this in any test suite? This is an internal detail of the driver. In Selenium2Driver, executeJsOnElement is private and none of its usages allow modifying the code from a public method (as far as I can see). DriverInterface does not expose a way to execute javascript for an element. In fact, when we remove syn, most of these usages will also be gone.

But maybe I'm missing something - let me know if you have a specific use case in mind.

aik099 commented 9 months ago

You're absolutely correct. The \Mink\WebdriverClassDriver\WebdriverClassicDriver::executeJsOnElement is an internal driver method, that isn't forced to obey {{ELEMENT}} replacement logic.

I must be looking at too much code today, that I haven't noticed that. I honestly thought it was a public method.

My apologies once again.

uuf6429 commented 9 months ago

No worries!