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

Fix getText() to work with elements outside viewport #327

Closed

Conversation

mvorisek
Copy link
Contributor

@mvorisek mvorisek commented Jan 12, 2021

Original impl. using $element->text() is defined as "Returns the visible text for the element." [1].

This PR fixes the implementation to makegetText() working with elements outside viewport. The new impl. is inspired by getHtml() method defined a few lines below. Thus this fix should be consistent and there should be no downside.

See atk4/ui#1538 - when small window (like 1280x720) is given, the tests fail with the original implementation. With this PR, the issue is fixed.

[1] https://github.com/instaclick/php-webdriver/blob/v1.0.8/lib/WebDriver/Element.php#L33

@oleg-andreyev
Copy link
Contributor

oleg-andreyev commented Jan 12, 2021

It's an expected behavior https://www.w3.org/TR/webdriver/#get-element-text

I don't think it's a responsibility of this package. You just need to make sure that text is in a viewport (same as your user)

@mvorisek
Copy link
Contributor Author

It's an expected behavior https://www.w3.org/TR/webdriver/#get-element-text

I don't think it's a responsibility of this package. You just need to make sure that text is in a viewport (same as your user)

For W3C driver conformant yes, but not for general element itself. As discussed in the PR desc, getHtml() always works and getText() should too - this PR fixes it.

@mvorisek mvorisek changed the title Fix getText() to work for elements outside viewport Fix getText() to work with elements outside viewport Jan 13, 2021
@aik099
Copy link
Member

aik099 commented Jan 23, 2021

The Selenium Server (used by this driver) only operates page elements, that the user can see. For that reason, there can't be a Mink/Behat test, that tests parts of the page, invisible to the user.

Because of the above fact, Selenium Server API doesn't have a method to retrieve HTML of an element (user can't see it on the page anywhere) and we have to develop a JavaScript-based solution, which doesn't respect element visibility as you've noticed.

There are no tests for getText/getHTML asserting that they're accessing visible parts of the page because only the Selenium driver is capable of that.

I'm not certain what direction we should take with this:

  1. make getText operate on invisible elements (as getHTML does)
  2. change getHTML to also operate on only visible elements

// @stof

Related PR: #328

@aik099 aik099 requested a review from stof January 23, 2021 17:22
@mvorisek
Copy link
Contributor Author

mvorisek commented Jan 23, 2021

I'm not certain what direction we should take with this:

1. make `getText` operate on invisible elements (as `getHTML` does)

2. change `getHTML` to also operate on only visible elements

@aik099 Thank you for getting into it. I strongly belive we should go with the 1st option because:

a) some browsers like Firefox operate on invisible elements, some browsers like Chrome does not (and only in some modes, like --headless). With this PR, the behaviour for the getText is consistent.
b) other methods like getHTML already operate on invisible elements, thus with this PR, the behaviour is then consistent across methods.

@mvorisek
Copy link
Contributor Author

Hi @stof, @aik099, guys, can you please merge this PR to allow easier/more consistent Behat testing?

Thanks in advance!

@aik099
Copy link
Member

aik099 commented Jul 17, 2021

See atk4/ui#1538 - when small window (like 1280x720) is given, the tests fail with the original implementation. With this PR, the issue is fixed.

@mvorisek , the case you're facing (website doesn't fit in a small browser window and therefore it looks like the user isn't seeing the text) is expected behavior.

How you can hope to pass the test when text is invisible to the user?

P.S.
After thinking more about your proposition I still think, that operations outside of the viewport (which was made via JS just to conform with the DriverInterface really) isn't something, that this driver should ever do. I hope you'll understand.

@mvorisek
Copy link
Contributor Author

@aik099 I think of it this way - DOM has no viewport, if a DOM element is queried for a text with getText, a text of the element should be returned no matter if presented in a viewport or not. Secondly, similar functon getHTML does it already.

@Chekote
Copy link
Contributor

Chekote commented Aug 24, 2021

@mvorisek I think you're not using the right tool for the job. Selenium is for user testing. You should not be interacting with or making assertions against anything that the user cannot interact with or see.

You'll notice this being enforced as the Selenium server has evolved. In older versions, you used to be able to click elements that were covered by other elements, but with newer versions, you will now get an exception. If a user cannot do it, then Selenium won't allow you to do it either.

@aik099
Copy link
Member

aik099 commented Sep 4, 2021

Closing then.

@aik099 aik099 closed this Sep 4, 2021
@mvorisek mvorisek deleted the fix_gettext_outside_viewport branch October 16, 2022 16:57
@mvorisek mvorisek restored the fix_gettext_outside_viewport branch May 3, 2023 08:15
@mvorisek mvorisek deleted the fix_gettext_outside_viewport branch May 3, 2023 08:15
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.

4 participants