Add ability to check if a NodeElement is disabled

[evangelion1204]

Hi,

I would like to implement a method called isDisabled for the NodeElement to check if an element is disabled or not. This can come quite handy because in AjaxRequest you often disable Buttons/Elements.
This could be done quite analog to isChecked. Is this new function wished and how could it be named? isDisabled or isEnabled - cause you normally add the attribute disabled to an element isDisabled seems more logical, from the properties isEnabled.

Thanks,

Michael

[aik099]

👍

[aik099]

@stof , any thoughts about the correct naming?

[stof]

I'm not sure this method is really needed. It is a matter of checking the disabled attribute (isChecked is different as its state will change when manipulating the form)

[aik099]

But you can change disabled state from JS and then getAttribute('disabled') will return value from HTML attribute that was present on initial page load.

Maybe we should introduce getNodeProperty method (will do same as $('element').prop('name')) does. And change the isChecked to use that. Because right now we have getAttribute for retrieving HTML attributes of DOM node, but we don't have any method for retrieving JS properties of a node. For BrowserKit driver it will fallback to getAttribute.

[evangelion1204]

As far as I know disabled is not an attribute, the initial value is set via the attribute but it is a property, some browser may change it if the property changes. But normally this should not be the case. A workaround via attribute is possible but may result in wrong checks.

getNodeProperty would be even better, cause it would work with every property.

[aik099]

Then in the NodeElement we can create method isEnabled which will just call !...->getDriver()->getNodeProperty($this->getXpath(), 'disabled').

[stof]

The issue with getNodeProperty is that we cannot implement it in BrowserKitDriver. The fallback to the HTML attribute is only true for a few DOM properties. It is far from being a general case

[aik099]

Then we don't implement it in MinkBrowserKitDriver. Since getNodeProperty uses JS to work then it will only be implemented in JS-aware drivers. I think we can live with that.

Yes, idea with fallback was wrong one.

[stof]

@aik099 but isDisabled is something which could make sense in MinkBrowserKitDriver

[aik099]

Now I'm confused. First you say that no such thing as isDisabled is needed because it can be checked via ->getAttribute('disabled'). Now you say that suddenly MinkBrowserKitDriver needs it as well.

Then we're back at initial subject of discussion: how to name the method on the driver: isDisabled or isEnabled. JS-aware drivers will use JS for it and MinkBrowserKitDriver will do just getAttribute('disabled').

And getNodeProperty will be JS-aware drivers only feature then.

[stof]

no, I'm saying that if you decide to include it, you cannot implement it using a generic getNodeProperty which would need not work in non-JS drivers

[evangelion1204]

I think the problem is that the line ...->getDriver()->getNodeProperty($this->getXpath(), 'checked'). is in the NodeElement not the Driver itself, the getNodeProperty function will not be implemented in non JS-aware drivers and not work anymore.

[aik099]

no, I'm saying that if you decide to include it, you cannot implement it using a generic getNodeProperty which would need not work in non-JS drivers

Then I can see the light at the end of a tunnel. As told in https://github.com/Behat/Mink/issues/521#issuecomment-42435990 we just do 2 separate methods and all drivers will be happy. In turn the getNodeProperty can be used to implement isDisabled and isChecked for all JS-based drivers.

[aik099]

I think the problem is that the line ...->getDriver()->getNodeProperty($this->getXpath(), 'checked'). is in the NodeElement not the Driver itself, the getNodeProperty function will not be implemented in non JS-aware drivers and not work anymore.

You're wrong. Latest code from NodeElement:

public function isChecked()
{
    return (Boolean) $this->getSession()->getDriver()->isChecked($this->getXpath());
}

[stof]

Then I can see the light at the end of a tunnel. As told in #521 (comment) we just do 2 separate methods and all drivers will be happy. In turn the getNodeProperty can be used to implement isDisabled and isChecked for all JS-based drivers.

Selenium1 and Selenium2 at least have a native method to check isChecked. I don't think making them use a custom JS evaluation rather than the driver method is a good idea.