discussion: support for `Element::is_*` functions family about thirtyfour

[stevepryde]

Not strictly related to this discussion but following up on my above comments...
I've managed to get the new ElementQuery working. See the thirtyfour_query crate for details.
https://crates.io/crates/thirtyfour_query

[stevepryde]

Just to rein in the scope of this issue, I think the goal is to add the following methods to WebElement:

• is_visible
• is_present
• is_clickable

(is_enabled is already there)

All of these would return a bool.

They likely all require some kind of javascript. For is_present it may be possible to do get_attribute or similar and just use the response to figure out whether the element is still present (i.e. if you get a StaleElementReference then it's no longer present). I'm not sure how to do the others. Perhaps these can be copied from other selenium libraries.

If someone has experience with these, and can provide something that should work reliably in 99% of cases, a PR would be most welcome ;)

[stevepryde]

The is_enabled method is already implemented as it is part of the W3C WebDriver spec.
The is_present one is a bit tricky since in order to get an element to call on, the element had to already be present (else selenium would not have found it). The selenium selectors already effectively wait for an element to become present - that's what they do. However you may wish to wait for an element to become "not present" again. I think that is fairly easy once you have good finder machinery available (more on that later).

You are correct about the others - they are not easy to do. My suggestion is to not rely on the other methods. It is possible (and potentially more reliable) to use selenium without them.

For example, instead of is_displayed() you may be able to use some other query to determine whether an element is present or not. For example, in Angular you can just wait until the class no longer contains ng-hide. Of course this is easier if you have access to the code for the web app you are trying to automate - then you can add whatever attributes you need in order to expose the state of an element in the DOM. If you don't have access to the code it becomes much more difficult but perhaps you can come up with some javascript that works specific to your app.

In my experience, even the is_displayed method found in other selenium libraries is not 100% reliable in all cases. There are lots of false positives, because actually determining when an element is visible to the user is really, really difficult.

With all of that said, I believe your real intention here is to have more powerful element selector logic available, which is something I have been thinking about. I'm not yet sure whether to add that in here, or in its own crate. It probably makes sense to add it here, but it would expand the scope of this project somewhat.

I will add a new issue to scope out the feature I am thinking of in more detail.

[stevepryde]

One thing your Waiter idea offers that is outside the scope of ElementQuery is to be able to wait for a condition involving an element you already have. For example you have an element and you want to wait until it becomes disabled.

I assume this would be nicer with async closures(?) but for now the interface seems quite verbose to me and not really ideal for supplying ad-hoc closures in your code. This is why I ended up going for the builder interface because this hides the complexity and the user code reads pretty well.

Because of this, I'm thinking it would be nicer to just do a similar trait for waiting on element conditions specifically, and follow the same pattern as ElementQuery.

That would allow something like this:

element.wait().until().enabled().await?;

which I think is pretty nice.

And just like with ElementQuery you could chain various conditions together, and even replace until() with until_not() or while(). If you want custom predicates, that can be done the same as ElementQuery::with_filter() (like the is_visible() function in my previous comment).

Calling .await at the end there without a dedicated run() method might be possible using a trick like this: https://blog.yoshuawuyts.com/async-finalizers/
A sync version would need a dedicated run() method though (although probably named something better than "run").

One thing I really like that came out of ElementQuery is that anyone can build their own traits and extend the functionality just like ElementQuery does. I added the ability to store configuration (anything that implements Serialize + Deserialize) in WebDriver and then retrieve it for use in the trait - so that you can set it once in WebDriver and it can be accessed from any WebElement - because it gets stored in the WebDriverSession which every element has a reference to.

[audioXD]

No problem @stevepryde I found it while reading the Java code for selenium. There I found that it calls a webdriver command instead of javascript code.

[stevepryde]

Ok that also seems to be catching and ignoring StaleElementReferenceException as well (and NoSuchElementException for the is_visible() case)

[zhiburt]

I haven't been able to look through everything profoundly but looks promising/great.

---

I paste (just for fun) bellow an public API design patchworks which I was thinking about now/initially.

let waiter = Waiter::new(&mut driver, Duration::from_secs(5));
let query = waiter.until(|driver| driver.element("a").await?.enabled());

let waiter = Waiter::new(&mut driver, Duration::from_secs(5));
let query = waiter.until(Visible(By::Id("content")));

let waiter = Waiter::new(Duration::from_secs(5));
waiter.until(|driver| driver.element("a").await?.enabled());

driver.query(waiter).await;

[stevepryde]

That's an interesting idea. I think I'll explore that a little more. There is some overlap with the query interface but also potential to do a little more as well. It would likely require boxing the closure that you pass in.

I just had a quick attempt and came up with this (which works):

    let waiter = Waiter::new(&driver, Duration::from_secs(5));
    waiter
        .until(Box::new(move |driver| {
            Box::pin(
                async move { driver.find_element(By::Id("search-form")).await?.is_enabled().await },
            )
        }))
        .await?;

The closure signature looks like this:

type WaiterPredicate =
    Box<dyn for<'a> Fn(&'a WebDriver) -> Pin<Box<dyn Future<Output = WebDriverResult<bool>> + 'a>>>;

It needs to return WebDriverResult so that you can use ? within the closure.
If you wanted to return an element rather than bool, you'd need a different method, with a different closure sig.

However, this functionality is already possible with the ElementQuery interface from thirtyfour_query, like this:

let elem_form = driver.query(By::Id("search-form")).and_enabled().first().await?;

You could supply your own is_visible() method like this:

pub fn is_visible<'a>(elem: &'a WebElement) -> Pin<Box<dyn Future<Output = bool> + 'a>> {
    Box::pin(async move {
        let mut args = ScriptArgs::new();

        args.push(elem.clone());
        match elem
            .session
            .execute_script_with_args(
                r##"let style = window.getComputedStyle(arguments[0]);
                      return style.display;"##,
                &args,
            )
            .await
        {
            Ok(ret) => {
                let r: String = ret.convert().unwrap_or(String::new());
                r != "none"
            }
            Err(_) => false,
        }
    })
}

And then you can include it as a filter anywhere you want like this:

let elem_form =
        driver.query(By::Id("search-form")).with_filter(Box::new(is_visible)).first().await?;

The above code works right now (I just tested it). Btw the above Javascript is something I found via a quick Google search.
https://stackoverflow.com/questions/19669786/check-if-element-is-visible-in-dom

If we find that something like this works pretty well then we can include it in ElementQuery so then you could just do:

let elem_form =
        driver.query(By::Id("search-form")).and_visible().first().await?;

I'm happy to just copy the javascript verbatim from the selenium project if that's deemed "official" enough. Personally I've found it a bit hit or miss though 🤷‍♂️

[audioXD]

What is this section of the standard that I have found W3C element-displayedness isn't this native is_displayed() handling?

Sorry i can't make a pull request since i don't know how the library works yet. :(

[stevepryde]

Thanks @audioXD I had not seen that section before.

I've run into issues with this before, especially when Safari switched over to the W3C spec - they dropped support for this: https://developer.apple.com/documentation/webkit/macos_webdriver_commands_for_safari_12_and_later

It does seem to be supported by Chrome however, so I can add that one in pretty easily.

[audioXD]

Can we do a checklist of things to implement. Otherwise this issue is never going to get closed.
Like:

• is_visible
• is_present
• is_clickable

[stevepryde]

I was going to tackle all three now (for the exact reason that I don't want this issue hanging around forever ;-) )

In the python lib, is_clickable() is simply the equivalent of is_visible() and is_enabled().
Is this ok? Do we even need this?

As for is_present() - I don't currently have a good solution for this. The caller can do this more easily by simply re-running the query that found the element in the first place - and I think this kind of thing makes more sense in thirtyfour_query rather than here.

[audioXD]

In the Java version I found for is_clickable()

public static ExpectedCondition<WebElement> elementToBeClickable(final By locator) {
    return new ExpectedCondition<WebElement>() {
      @Override
      public WebElement apply(WebDriver driver) {
        WebElement element = visibilityOfElementLocated(locator).apply(driver);
        try {
          if (element != null && element.isEnabled()) {
            return element;
          }
          return null;
        } catch (StaleElementReferenceException e) {
          return null;
        }
      }

      @Override
      public String toString() {
        return "element to be clickable: " + locator;
      }
    };
  }

public static ExpectedCondition<WebElement> visibilityOfElementLocated(final By locator) {
    return new ExpectedCondition<WebElement>() {
      @Override
      public WebElement apply(WebDriver driver) {
        try {
          return elementIfVisible(driver.findElement(locator));
        } catch (StaleElementReferenceException | NoSuchElementException e) {
          // Returns null because the element is no longer or not present in DOM.
          return null;
        }
      }

      @Override
      public String toString() {
        return "visibility of element located by " + locator;
      }
    };
  }

So if the elment is present and is enabled

[audioXD]

Yes I was about to mention the exact thing of calling a non mutable method and checking if there is a no reference error.

[stevepryde]

My preference is for these WebElement methods to not be too clever in ignoring specific errors, but rather do that level of behaviour in the thirtyfour_query implementation.

These methods on their own should return whatever info would be applicable to a single explicit call, in which case I think you probably do want to know if an element is stale etc.

[stevepryde]

There's a related component here which is the ability to wait on these conditions. This has implications for both thirtyfour_query but also there's a need for the ability to wait for an existing element to meet some condition. Ideally this will share the polling behaviour in thirtyfour_query so I think it makes sense to implement it there.