Consider etaoin.api/displayed? behavior

[daveyarwood]

I noticed that displayed? returns true for an invisible option element with style display: none.

I looked at the code a little bit, and it seems like the issue might be that the displayed value is not correct.

I noticed in the code that there is a custom implementation for Safari because Safari does not have a native displayed implementation. I tried using that implementation for Chrome and Firefox and it works as expected.

I might be lacking the full context here, but it seems like this custom implementation is more reliable, and Etaoin should just use that for all browsers. If you're interested, I could put together a PR to do that. But I'm also curious if you have any other thoughts about this.

[lread]

Thanks so much @daveyarwood! Personally? No idea! And I don't know what that jwp comment means either:

etaoin/src/etaoin/api.clj

Line 2145 in e4d60c8

(defmethod displayed-el? :default ;;TODO it's only for jwp

Not sure if this helps but here's what the w3c webdriver spec says on the topic.

PR most welcome that includes tests.

[daveyarwood]

From the W3C spec you linked:

An element is in general to be considered visible if any part of it is drawn on the canvas within the boundaries of the viewport.

This definition is confusing with respect to option elements. Are they considered as part of the canvas? I would guess not. But from an end user perspective, I consider an option to be "displayed" if I see it when I open the dropdown. In the current case that I'm running into, I do not see the option element in question in the dropdown (because the style is display: none), however, both Chrome and Firefox consider it to be displayed.

I'm open to other perspectives on this, but I think it makes sense to take a pragmatic approach here and use the visibility and display attributes instead of the displayed attribute, since the former seems more reliable for our purposes.

I'll work on a PR for this.

[lread]

Yeah I guess the fact that they didn't define it in their API is a sign that it was debatable or contentious? I don't know.

Our docstring can describe what displayed? means for Etaoin.

[lread]

Hiya @daveyarwood, I'm learning that we might have been a bit hasty on this one.

I'm ramping up on Etaoin by going over docs and docstrings and tying as much as I can.

Here's something I happened to try:

    <ul>
      <li><a href="#">link 1</a></li>
      <li><a href="#" class="active">link 2 (active)</a></li>
      <li><a href="https://clojure.org">link 3 (clojure.org)</a></li>
      <li style="display:none"><a id="cantseeme" href="#">link 4 (not visible)</a></li>
    </ul>

With the current release of Etaoin (v0.4.6), I get what seems correct to me:

(e/displayed? driver :cantseeme)
;; => false

But with our agreed on change on master I see what seems incorrect to me:

(e/displayed? driver :cantseeme)
;; => true

Watcha think, should we re-open this issue?

[daveyarwood]

Ah, yeah, that seems like a bug. It looks like the issue there is that the element itself is not hidden by display:none, so it's "visible" in that sense, but it has a parent element that is hidden by display:none, so it should be effectively invisible.

[lread]

We can sleep on it for a bit and figure out what we want to do.
I'll re-open this issue so I don't accidentally forget about it.

[daveyarwood]

I don't know if I will have time to work on this soon, but just off-hand, to suggest an approach: if there is a way to enumerate the ancestors of an element, we could recursively check whether each element is displayed until we reach the top and there are no more ancestors to check.

This is probably not good from a performance perspective, but if it's acceptable performance-wise, the correctness might be worth it.

[daveyarwood]

Another idea: generally use the browser-provided displayed property (like we were doing before my change), but if it's an option or some other kind of element where displayed is known not to be accurate, then use this other implementation. That way, at least performance would be good most of the time.

[lread]

Cool, if you are short on time, no pressure to work on it @daveyarwood, I'm sure we'll figure something out.

[lread]

I've done minor poking around the web.
It seems the definition and implementation of "displayed" can be confusing and contentious.

Here's what the Selenium folks say in their docs:

This functionality is mentioned in, but not defined by the w3c specification due to the impossibility of covering all potential conditions. As such, Selenium cannot expect drivers to implement this functionality directly, and now relies on executing a large JavaScript function directly. This function makes many approximations about an element’s nature and relationship in the tree to return a value.

So this can get maybe complex. This selenium unit test can maybe give us some ideas of what we want to think about.

Seems like they might cover:

• element hidden by CSS (what we might at least partially cover)
• element hidden by parent element CSS (what I noticed we don't cover)
• they verify that a hidden element should not allow interaction...
• but that options should be selectable from an invisible select... (?)
• zero size and small elements
• elements hidden/affected by overflow (?)
• opacity (?)

I'm thinking that we should do the minimum that makes sense for us.
But getting a good understanding of what Selenium does would also be interesting.

[lread]

I've marked this both a bug and a feature.
The bug is that what we are/were doing is a bit odd.
The feature is deciding what we should actually do.

[daveyarwood]

I'm running into trouble now as a result of my change in #445. I have a UI test case where I click a button and I expect an element with a particular ID to become invisible. The trouble is: while that element does become invisible, it's because the display style of that element's parent was set to none.

In order to get my UI tests passing again, I'd love to do something about this if possible, even if it isn't an ideal long-term solution.

I played around with this for a while just now by adding a test case for a nested inner div that should be effectively invisible because its parent is invisible:

        <div id="hidden-outer-div" style="display: none;">
            <div id="hidden-inner-div">This is effectively hidden</div>
        </div>

    (-> (e/invisible? {:id :hidden-outer-div}) is)
    (-> (e/invisible? {:id :hidden-inner-div}) is)

As expected, the hidden-inner-div tast case fails on master because of my change in #445.

If I revert my change, then the hidden <option> test case fails because of the issue I raised here.

But I was able to get all of the test-visible test cases passing by using the approach I noted above in this comment: basically, revert to what we were doing before my change in #445, unless the tag is option, in which case, do the pragmatic thing I switched us to because it's more accurate in the case of option elements. (This still isn't perfect because if that option element is the child of a hidden parent element, it could still be erroneously reported to be displayed. But at least now the problem is restricted to option elements.)

If you're on board with this as a short term fix, I'd be happy to raise a PR! Let me know your thoughts.

[lread]

Heya @daveyarwood thanks for following up!

After looking at Selenium's approach, I've learned that the definition of "displayed" can include a lot of perspectives!

I think for us, for now, we can stick to our current approach of CSS controlled visibility via display:none and visibility:hidden. We can expand this, if needed, in the future to include other perspectives of "displayed" (likely even optionally).

This means we would not (at least initially) include:

• checks for opacity: 0 element
• checks for collapsed <details>
• checks for 0 sized elements
• etc, etc.

What are our options for implementing the parent elements check?

1. JavaScript
2. W3C WebDriver calls
3. Can XPath be employed somehow to search for us here? Dunno.

Whatever approach we choose, we should make sure it works across all of our supported browsers: chrome, firefox, edge and safari.

[daveyarwood]

I agree. The browser native displayed property doesn't seem to be reliable, so going further in this direction where we check various properties seems good to me.

I haven't started looking into approaches for checking the ancestors of an element. I think that would be a better fix for the immediate issue, but in the short term, are you open to a "band-aid" level fix like the one I mentioned in my last comment? I have the fix ready and could make a PR soon.

[lread]

@daveyarwood I think I'd rather we take a stab at deciding what behaviour we think makes good general sense and implement that. If we get it right, users of Etaoin would not have to adapt to changes we make in this area.

[daveyarwood]

Alright, that sounds reasonable to me 👍

Unfortunately, I don't think I have the bandwidth to work on a more robust fix myself, but I'm happy to continue the conversation here about approaches if that would be helpful.

[lread]

Yeah, your continued feedback is very much appreciated!

[daveyarwood]

Juuust in case you might be interested in my hotfix 🙂 , I've pushed it to a branch, and you can see the diff here: master...daveyarwood:etaoin:pragmatic-displayed-impl-v2 The change is pretty small -- just reintroducing the browser native displayed property implementation, and using it for all elements except <option>, which will continue to use the pragmatic approach because the displayed property isn't accurate in that case. I also added a test case for an "effectively hidden" element that is not itself hidden, but its parent is.

At the moment, I'm having to switch the repo we use at work to point at my personal fork instead of this repo, so that we can get our UI test cases to pass again. If we could get this change into etaoin master, then I could at least get us pointed at the proper Etaoin repo again, which would be helpful for us.

Better still will be when we have a robust displayed? implementation in a release of Etaoin, but I figure that this band-aid level fix would at least improve the current state of the master branch.

Thoughts?

[lread]

Thanks @daveyarwood, if we were confident your hotfix was on the long-term path we wanted to take, I'd be all for it.

I'll take a look at this issue sometime soon.

[lread]

I'm attempting to learn what WebDriver implementations do today wrt displayed-ness.

Can confirm that as of this writing, safaridriver still has not implemented the GET session/{session id}/element/{element id}/displayed REST endpoint.

It seems that chromedriver and firefox geckodriver (via marionette) both actually make use of Selenium atoms. Selenium atoms are JavaScript snippets that perform specific tasks.
So if we answer what does Selenium do for displayed-ness we might also get an answer for chrome and firefox.

(Far as I can tell, msedgedriver is basically a fork of chromedriver?).

I do see a note in the Selenium source expressly stating that option elements are a special case.

Options and Optgroup elements are treated as special cases: they are
considered shown iff they have a enclosing select element that is shown.

Why an expressly hidden option element is reported as displayed is on my todo list to learn more about.

I'm also poking around to see differences between WebDriver implementation behaviors.
So far I see that transparent elements (opacity: 0;) are seen as displayed by geckodriver but as not displayed by chromedriver and msedgedriver. I do see there is an option to ignoreopacity over at Selenium, so maybe firefox decided to do so but chrome and edge went the other way.

And I'm sure others have grappled with this issue, so far I noticed:

• Move away from calling Element Is Displayed endpoint for W3C-compliant drivers webdriverio/webdriverio#2962

[lread]

I've just verified that Selenium itself considers hidden and invisible options as displayed.

  <select id="visibility">
    <option value="regular" class="regular">Regular</option>
    <option value="disabled" disabled="disabled" class="disabled">Disabled</option>
    <option value="hidden" style="visibility: hidden;" class="hidden">Hidden</option>
    <option value="invisible" style="display: none;" class="invisible">Invisible</option>
  </select>

(source)

My hacky little Java app to verify (the transparent item is a sanity test, it is a select element with opacity 0, see linked source to have a peek if you are curious).

package com.dlread.seleniumplay;
import org.openqa.selenium.By;
import org.openqa.selenium.WebDriver;
import org.openqa.selenium.chrome.ChromeDriver;

public class Play {

    public static void main (String args[]) {
        WebDriver driver = new ChromeDriver();
        driver.get("file:///Users/lee/proj/oss/SeleniumHQ/selenium/common/src/web/selectPage.html");
        System.out.println ("regular displayed: " + driver.findElement(By.cssSelector("[class='regular']")).isDisplayed());
        System.out.println ("hidden displayed: " + driver.findElement(By.cssSelector("[class='hidden']")).isDisplayed());
        System.out.println ("invisible displayed: " + driver.findElement(By.cssSelector("[class='invisible']")).isDisplayed());
        System.out.println ("transparent displayed: " + driver.findElement(By.id("transparent")).isDisplayed());
        driver.close();
    }
}

When run, outputs:

regular displayed: true
hidden displayed: true
invisible displayed: true
transparent displayed: false

I still have not dug up the rationale for this behavior, but I'm sure my aha moment is coming.

[lread]

I've asked the question on the selenium user group:
https://groups.google.com/g/selenium-users/c/1gvP3WktYLU

Hopefully, someone will help us out with an explanation.

[lread]

No answers to my question on the Selenium User's Group yet.
'Tis the way sometimes in the world of open source.
I remain curious if there is a good reason for this behavior (there could be, I just don't know!).

I've done some more digging and my (perhaps flawed) understanding is this:

• Selenium has been around in one form or another since 2004. That's a long time.
• This option behavior has been around (maybe?) since 2010. That's also a long time.
• Selenium played a big role in the W3C WebDriver spec.
• Selenium makes its JavaScript atoms available to W3C WebDriver implementers. Some make use of them.
• But... maybe in slightly different ways with slightly different versions?
• Selenium itself does not call a WebDriver implementation displayed REST endpoint, it instead uses its JavaScript atom to answer the question of displayed-ness.

So what shall we do?

The W3C WebDriver spec recommends (but does not require) that a WebDriver implement displayed-ness by calling the Selenium's JavaScript atom (without ignoring opaqueness styling BTW) and expose the result on the displayed REST endpoint. But not all WebDriver implementations do this, and when they do, they don't necessarily all do it in the same way.

Where are we at now?

Etaoin implemented a naive displayed? behavior for safari but otherwise delegated to the displayed W3C WebDriver implementation endpoint.

@daveyarwood noticed the oddity with the option element (the subject of this issue).
We all agreed to call our naive safari implementation for all WebDriver implementations and not delegate to the displayed REST endpoint.

We then realized that the safari implementation was more naive than we'd like.

Where do we go?

[rejected] Option 1: Do nothing

Leave our naive displayed behavior change as is.
I feel this is too naive.

Option 2: Reverse new naive displayed implementation

Continue to delegate to the displayed REST endpoint (except for safari)
Document that the implementation is naive for Safari and odd regarding option elements.
This does resolve the issue through code, but does make the Etaoin user aware of a behavior.

Option 3: Automatically tweak new naive displayed implementation to special case option elements.

Call displayed REST endpoint when available.
Use our naive implementation for safari and option elements.

I think this is your most recent proposal, @daveyarwood.
This would address this issue for a use case.
It only addresses option elements that are hidden through CSS styling on the option element itself.

Option 4: Call the Selenium atom

Do what the W3C WebDriver spec recommends WebDriver implementations do, and call the Selenium JavaScript atom to determine displayed-ness.

If I've understood correctly, this is what Selenium does.
This would provide a consistent displayed-ness result across WebDriver implementations.

(To look into: other WebDriver implementation operations might rely internally on displayed-ness. We do want to remain a lightweight wrapper atop W3C WebDriver. If we have one definition of displayed-ness and a WebDriver implementation has a different definition, this could get confusing. I expect that Selenium wraps displayed-ness checks in all the right places for consistency?).

But this does not resolve the raised issue. Sub-options
a) document option element oddity
b) automatically tweak to overcome option element oddity
c) allow Etaoin users to optionally overcome option element oddity

Option ?

[lread]

The Selenium folks fixed a broken link to a video in their docs.

The video does not cover our option element mystery but does cover some of the challenges of determining displayed-ness.

[lread]

Heya @daveyarwood, I've been letting this issue block a release.
This is probably not a great idea.

I'll likely roll our current changes to displayed? back sometime in the next days, and cut a release.

[daveyarwood]

Sounds good to me. I'm all for not blocking a release!