Skip to content

Conversation

@mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Apr 14, 2022

Session::execute and Session::execute_async methods must convert the web driver element to object the same way as Container::element and Container::elements methods do.

@robocoder
Copy link
Member

Thanks for the PR. This looks like a compat-buster on the 1.x branch since php-webdriver has historically been a (largely) thin wrapper around the W3C and legacy WebDriver protocols.

Can you provide an example of the method call that demonstrates (args and return value) why this is needed?

@mvorisek mvorisek marked this pull request as draft April 14, 2022 20:47
@mvorisek mvorisek marked this pull request as ready for review April 14, 2022 21:17
@mvorisek
Copy link
Contributor Author

W3C and legacy WebDriver protocols

Yes, it is supported offcially, see facebook impl. https://github.com/php-webdriver/php-webdriver/blob/8ffa927b270e932449e8015abf4d38bb0eff24b7/lib/Remote/RemoteWebDriver.php#L666

My question is, can this PR target 1.x? LOC will be low, no BC break.

@mvorisek
Copy link
Contributor Author

mvorisek commented Apr 14, 2022

Example usage:

// expecting:
// a) $wdSession with working \WebDriver\Session driver
// b) loaded webpage with at least one <div> element with some ID attribute

$element = $wdSession->element('xpath', '//div'); // returns instance of \WebDriver\Element

$jsResult = $wdSession->execute([
    'script' => 'return (function (elem) { return [elem, elem.id]; })(arguments[0])',
    'args' => [$element],
]);

// will dump array with:
// [0] (new) instance of \WebDriver\Element to demonstrate conversion from php to webdriver and back
// [1] ID of given element extracted by JS
var_dump($jsResult);

@robocoder
Copy link
Member

Thanks for fleshing that out.

@robocoder robocoder merged commit c632618 into instaclick:1.x Apr 14, 2022
@robocoder
Copy link
Member

Thanks. This has been tagged as 1.4.12. I'll try to port to the master branch later tonight.

@robocoder
Copy link
Member

See 8bb204d

@mvorisek
Copy link
Contributor Author

Thank you ❤️

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.

2 participants