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

WebElement.isEnabled() should check for actual element state rather than looking at disabled attribute in the dom #1348

Closed
mekdev opened this Issue Dec 4, 2015 · 7 comments

Comments

Projects
None yet
4 participants
@mekdev
Copy link

mekdev commented Dec 4, 2015

This is a feature request for the nodejs bindings - WebElement.prototype.isEnabled()

/**
2124     * Schedules a command to query whether the DOM element represented by this
2125     * instance is enabled, as dicted by the {@code disabled} attribute.
2126     * @return {!webdriver.promise.Promise.<boolean>} A promise that will be
2127     *     resolved with whether this element is currently enabled.
2128     */
2129    webdriver.WebElement.prototype.isEnabled = function() {
2130      return this.schedule_(
2131          new webdriver.Command(webdriver.CommandName.IS_ELEMENT_ENABLED),
2132          'WebElement.isEnabled()');
2133    };

Scenario

Call isEnabled() on a web element that is disabled, but the element does not have a disabled attribute.

image

Page object

BasePage: Internal ui library, where the native selenium bindings are called

/**
 * Safe enabled status getter for element enabled status to check for negative states
 * @param locator
 * @param timeout
 * @returns {!promise.Promise.<T>}
 */
BasePage.prototype.isEnabled = function(locator, timeout) {
    timeout = timeout || WAIT_TIME_NOT_PRESENT;
    var defer = Promise.defer();
    var driver = this.driver;
    // Explicitly wait for the element to be visible
    var element = driver.findElement(locator);
    driver.wait(Until.elementIsVisible(element),timeout).then(function () {
        Logger.debug('Element is visible : ' + locator);
        // If its located check of if it is enabled
        element = driver.findElement(locator);
        driver.wait(Until.elementIsEnabled(element),timeout).then(function() {
            driver.findElement(locator).isEnabled().then(function(isEnabled) {
                Logger.verbose('Element : ' + locator + ' isEnabled() : '+ isEnabled);
                defer.fulfill(isEnabled);
            }).thenCatch(function (err)  /* error call back*/{
                Logger.verbose('Element is NOT enabled : ' + locator);
                defer.fulfill(false);
            });
        }).thenCatch(function (err) /* error call back*/ {
            Logger.verbose('Element is NOT enabled after waiting, error : ' + err.stack);
            defer.fulfill(false);
        });
    }).thenCatch(function (err) /* error call back*/ {
        Logger.verbose('Element is NOT enabled after waiting, error : ' + err.stack);
        defer.fulfill(false);
    });
    return defer.promise;
};

SignupPage: Returns the promise of the element state

/**
 * Gets the enabled status of the Save button
 * @returns {!webdriver.promise.Promise.<boolean>|!promise.Promise.<T>}
 */
SignupPage.prototype.isCreateButtonEnabled = function() {
    this.waitForDisplayed(By.css(CREATE_ACCOUNT_SUBMIT_BUTTON));
    return this.isEnabled(By.css(CREATE_ACCOUNT_SUBMIT_BUTTON));
};

Usage in mocha

        signup.isCreateButtonEnabled().then(function (isEnabled){
            assert.equal(isEnabled, false, 'Create button should be disabled');
        });

On asserting, the value coming back is true even when the element is actually disabled.
On reading the docs in detail it seems like we are looking for disabled attribute instead of checking actual state. See attached debug stack.

Expected behavior : the returned value to be false

image

@ddavison

This comment has been minimized.

Copy link
Member

ddavison commented Dec 4, 2015

well met @mekdev , we met at SeConf :) go ahead and submit a pull request - looks like you got the code all here.. be sure to read the CONTRIBUTING.md file prior

@AutomatedTester

This comment has been minimized.

Copy link
Member

AutomatedTester commented Dec 4, 2015

Before you submit a PR, how are we supposed to guess that element is disabled?

Looking at the markup there is nothing to suggest that it is disabled other than you have another element over it to take the events...

@ddavison

This comment has been minimized.

Copy link
Member

ddavison commented Dec 4, 2015

It looks like you are using the disabled class to determine it's disabled. Selenium shouldn't care about this class.. it's an arbitrary class probably defined by the UI library you are using.

It will however detect the disabled="disabled" attribute

@mekdev

This comment has been minimized.

Copy link
Author

mekdev commented Dec 4, 2015

@ddavison, yes I remember :) happy to see you here again.

I am opened to help out but I am not sure if the problem is just related to the nodejs bindings or is it the current intended behavior deeper down the selenium stack.

After looking at the internals we are calling this command.

/**
 * Schedules a command to query whether the DOM element represented by this
 * instance is enabled, as dicted by the {@code disabled} attribute.
 * @return {!webdriver.promise.Promise.<boolean>} A promise that will be
 *     resolved with whether this element is currently enabled.
 */
webdriver.WebElement.prototype.isEnabled = function() {
  return this.schedule_(
      new webdriver.Command(webdriver.CommandName.IS_ELEMENT_ENABLED),
      'WebElement.isEnabled()');
};

In the commands file it is

  IS_ELEMENT_ENABLED: 'isElementEnabled',

@AutomatedTester - Hi Dave, I am not sure what you mean by "guess that the element is disabled".

It looks like you are using the disabled class

Yes, the ui is using disabled class and not the disabled attribute.
Are you guys saying that the dev folks should just use the attribute instead and thats the only way selenium knows that the element is in a disabled state ?

@AutomatedTester

This comment has been minimized.

Copy link
Member

AutomatedTester commented Dec 4, 2015

@mekdev

The .disabled class is meaningless. It is a construct of some framework that you are using but doesn't actually disable the element. See the following css below

.disabled {
  opacity: 0.1
}

This doesn't actually disable anything, it sets the opacity to 0.1 but I could still click on it and events would still fire. If there is a disabled attribute on the element then the browser wouldn't queue up events to be fired.

I am saying that you should use the what the browser understands, and is cross-platform, as disabled and that is the attribute.

@jleyba

This comment has been minimized.

Copy link
Contributor

jleyba commented Dec 4, 2015

Only thing I can add to this is I'm open to changing the jsdoc for what isEnabled() does. The actual behavior, though, is dictated by the W3C spec:

https://w3c.github.io/webdriver/webdriver-spec.html#is-element-enabled

@mekdev

This comment has been minimized.

Copy link
Author

mekdev commented Dec 4, 2015

Thanks guys! I will take it back to the ui team and ask them to not use the .disabled class.

@jleyba I can help with that, we are using the nodejs binding heavily.
The jsdoc might be a good starter for me to help out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.