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

[🐛 Bug]: ExpectedCondition::elementToBeClickable does not consider CSS pointer-events: none #14427

Open
rr-it opened this issue Aug 22, 2024 · 12 comments

Comments

@rr-it
Copy link

rr-it commented Aug 22, 2024

What happened?

The docs mention

  • static ExpectedCondition<WebElement> elementToBeClickable(By locator)
  • static ExpectedCondition<WebElement> elementToBeClickable(WebElement element)

An expectation for checking an element is visible and enabled such that you can click it.
See https://www.selenium.dev/selenium/docs/api/java/org/openqa/selenium/support/ui/ExpectedConditions.html#elementToBeClickable(org.openqa.selenium.By)

So elementToBeClickable should check if an element is clickable.

Elements that have CSS pointer-events: none set are not clickable. This should be considered in the check for clickability.

See https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events#values

How can we reproduce the issue?

The following <button>-element is not clickable:

HTML:

<button class="disabled"></button>

CSS:

.disabled {
    pointer-events: none;
}

But elementToBeClickable only checks if an element is there and enabled.
It does not check for the CSS property pointer-event: none - which might make an element non-clickable.

/**
* An expectation for checking an element is visible and enabled such that you can click it.
*
* @param locator used to find the element
* @return the WebElement once it is located and clickable (visible and enabled)
*/
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;
}
};
}
/**
* An expectation for checking an element is visible and enabled such that you can click it.
*
* @param element the WebElement
* @return the (same) WebElement once it is clickable (visible and enabled)
*/
public static ExpectedCondition<WebElement> elementToBeClickable(final WebElement element) {
return new ExpectedCondition<WebElement>() {
@Override
public WebElement apply(WebDriver driver) {
WebElement visibleElement = visibilityOf(element).apply(driver);
try {
if (visibleElement != null && visibleElement.isEnabled()) {
return visibleElement;
}
return null;
} catch (StaleElementReferenceException e) {
return null;
}
}
@Override
public String toString() {
return "element to be clickable: " + element;
}
};
}

Adding element.getCssValue("pointer-events") != "none" to the condition might give the expected result.

Relevant log output

IMO not relevant here

Operating System

IMO not relevant here

Selenium version

IMO not relevant here

What are the browser(s) and version(s) where you see this issue?

IMO not relevant here

What are the browser driver(s) and version(s) where you see this issue?

IMO not relevant here

Are you using Selenium Grid?

No response

Copy link

@rr-it, thank you for creating this issue. We will troubleshoot it as soon as we can.


Info for maintainers

Triage this issue by using labels.

If information is missing, add a helpful comment and then I-issue-template label.

If the issue is a question, add the I-question label.

If the issue is valid but there is no time to troubleshoot it, consider adding the help wanted label.

If the issue requires changes or fixes from an external project (e.g., ChromeDriver, GeckoDriver, MSEdgeDriver, W3C), add the applicable G-* label, and it will provide the correct link and auto-close the issue.

After troubleshooting the issue, please add the R-awaiting answer label.

Thank you!

@shbenzer
Copy link
Contributor

Just because a pointer-event is set to none doesn't mean an element isn't clickable.

To use your example of a disable button: I can click it, but nothing should happen. If I were writing tests for security permissions, for example, I would want to be able to simulate clicking said button when it is enabled and when it is disabled, then assert that the result of clicking the button when enabled does not occur when the button is disabled.

@rr-it
Copy link
Author

rr-it commented Aug 22, 2024

@shbenzer AFAIK your mentioned case does already not work for buttons "disabled" via pointer-events: none.

To use your example of a disable button: I can click it, but nothing should happen.

If you trigger a click on a button like <button style="pointer-events: none">…</button> an exception ElementClickInterceptedException might get thrown:

[ElementClickInterceptedException] element click intercepted:
Element <button type="button" class="dropdown-item disabled">...</button> is not clickable at point (121, 269).
Other element would receive the click: <li>...</li>

Here the underlying element in the background receives the click instead.


Just because a pointer-event is set to none doesn't mean an element isn't clickable.

This contradicts what https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events#none says:

pointer-events: none: The element on its own is never the target of pointer events.


ExpectedCondition::elementToBeClickable does already not return "really disabled" elements.
What I like to change here is, that ExpectedCondition::elementToBeClickable handles "pointer-events: none-disabled" elements the same way - as those are already not clickable either.

E.g. a "really disabled" button via attribute disabled:

<button disabled>Test</button>

Also see:

@shbenzer
Copy link
Contributor

shbenzer commented Aug 23, 2024

While I still believe that elements should be clickable when they're disabled, you do make a good point. I just did some testing in Chrome in Vue.js on a button that is disabled and not only do I similarly get the same ElementClickInterceptedException you described when trying to click a disabled element, but element.is_enabled() also returns true for the disabled button.

<button type="button class="v-btn v-btn--disabled v-btn--has-bg theme--light v-size--small" disabled="disabled" style="" class="v-btn__content"><span>I'm a disabled button</span></button>

I think this might be evidence of a larger issue, and more than just the EC::elementToBeClickable needs to be updated. I think at least 3 things need to be updated to handle new disabled elements and pointer-events: none:

  • ExpectedCondition elementToBeClickable(By locator)
  • ExpectedCondition elementToBeClickable(WebElement element)
  • WebElement.is_enabled()

@rr-it
Copy link
Author

rr-it commented Aug 23, 2024

IMO reading the docs WebDriver element enabled we might check what's the result of
/session/{session id}/element/{element id}/enabled (WebElement.is_enabled())
in the following cases:

Enabled:

  • A.1: simple button <button>Test</button>
  • A.2: simple button inside form <form><button>Test</button></form>
  • B.1: simple button with type <button type="button">Test</button>
  • B.2: simple button with type inside form <form><button type="button">Test</button></form>

Disabled by attribute disabled:

  • A.1_d-attr1: simple button <button disabled>Test</button>
  • A.1_d-attr2: simple button <button disabled="">Test</button>
  • A.1_d-attr3: simple button <button disabled="disabled">Test</button>
  • A.2_d-attr1: simple button inside form <form><button disabled>Test</button></form>
  • A.2_d-attr2: simple button inside form <form><button disabled="">Test</button></form>
  • A.2_d-attr3: simple button inside form <form><button disabled="disabled">Test</button></form>
  • B.1_d-attr1: simple button with type <button type="button" disabled>Test</button>
  • B.1_d-attr2: simple button with type <button type="button" disabled="">Test</button>
  • B.1_d-attr3: simple button with type <button type="button" disabled="disabled">Test</button>
  • B.2_d-attr1: simple button with type inside form <form><button type="button" disabled>Test</button></form>
  • B.2_d-attr2: simple button with type inside form <form><button type="button" disabled="">Test</button></form>
  • B.2_d-attr3: simple button with type inside form <form><button type="button" disabled="disabled">Test</button></form>

Disabled by pointer-events: none:

  • A.1_d-pointer: simple button <button style="pointer-events: none">Test<button>
  • A.2_d-pointer: simple button inside form <form><button style="pointer-events: none">Test</button></form>
  • B.1_d-pointer: simple button with type <button type="button" style="pointer-events: none">Test</button>
  • B.2_d-pointer: simple button with type inside form <form><button type="button" style="pointer-events: none">Test</button></form>

Check all these cases with different browser engines:

As the 'element enabled' info might come directly from the browser engine.

@shbenzer
Copy link
Contributor

shbenzer commented Aug 23, 2024

Only thing that needed to be upgraded was isEnabled(). Thank you for pointing this out @rr-it

@rr-it
Copy link
Author

rr-it commented Aug 23, 2024

I'd like to keep a difference between is enabled and is clickable.
So not in any case is enabled equals is clickable.

So my conclusion:

@shbenzer
Copy link
Contributor

Is_clickable checks if an element is enabled before clicking - by upgrading is_enabled to handle pointer-events:none, is_clickable should handle it correctly

@rr-it
Copy link
Author

rr-it commented Aug 23, 2024

Is_clickable checks if an element is enabled before clicking - by upgrading is_enabled to handle pointer-events:none, is_clickable should handle it correctly

Actually someone might want to check for elements that are 'is_enabled' but are not 'is_clickable' at the same time.

I do not want to change any meaning of neither 'is_enabled' nor 'is_clickable'.
I only want to fix 'is_clickable', so that it fulfills its meaning.
I do not want to change the behaviour of 'is_enabled', as this would change the meaning of 'is_enabled'.


Button A: is_enabled: no and is_clickable: no

<button disabled>Button A</button>

  • trigger by keyboard: no
    • <-- element not enabled
  • click by pointer device (mouse): no
    • <-- element not enabled

Button B: is_enabled: yes and is_clickable: no

(Here the current implementation needs fixing as it states: is_enabled: yes and is_clickable: yes)

<button style="pointer-events: none">Button B</button>

  • trigger by keyboard: yes
    • <-- element is enabled
  • click by pointer device (mouse): no
    • <-- element is enabled
    • <-- but: pointer-events are disabled

@shbenzer
Copy link
Contributor

shbenzer commented Aug 23, 2024

Instances where an element is clickable require it to be of the following element types:

TagName.BUTTON
TagName.INPUT
TagName.OPTGROUP
TagName.OPTION
TagName.SELECT
TagName.TEXTAREA

These tags are intended to be interacted with, in some way, by the mouse (at minimum by selecting them for keyboard interaction in the case of TagName.TEXTAREA).

While, yes, if one of these tags have pointer-events: none as an attribute, one might still be able to interact with it through the keyboard, to me that would be an exception to accepted practices. The buttons are effectively disabled to the average user. They won't be able to be targeted with anything except pure keyboard navigation (e.g. Key.TAB).

EC::ToBeClickable will accurately tell you the element can't be clicked
Is_Enabled() will accurately tell you that the button is effectively disabled

@diemol, do you disagree?

@rr-it
Copy link
Author

rr-it commented Aug 23, 2024

About this comment:

Is_Enabled() will accurately tell you that the button is effectively disabled

@shbenzer If we go on with your implemenation, it will only tell that a button is disabled for mouse. It won't tell anymore if it is enabled or disabled for keyboard.

I would not call a button "effectively disabled" if that button is disabled for mouse only, but is still enabled for keyboard.


These tags are intended to be interacted with, in some way, by the mouse (at minimum by selecting them for keyboard interaction in the case of TagName.TEXTAREA).

  • It should be possible to use any web-interface by keyboard only (WAI-ARIA).
  • It should be totally possible to use this very framework here in your own tests to find buttons that are implemented in an unusual way e.g. "usable by keyboard only and not by mouse" so you can fix them in your code. Therefore it must be possible to discern is_enabled: yes and is_clickable: no.

@shbenzer
Copy link
Contributor

That is a good point, I hadn't considered that accessibility standards might require a site to be keyboard-only accessible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants