robotframework / OldSeleniumLibrary

Deprecated Selenium library for Robot Framework
Apache License 2.0
13 stars 3 forks source link

New `Element Should (Not) Be Visible` keywords #144

Closed spooning closed 9 years ago

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 15 Oct 2010

Sometimes, it's necessary to assert that a given element is visible, or invisible, depending on the context. This can be done in a user keyword, but depending on what you want to do, this keyword can become quite ugly. Also, a user keyword never provides integrated logging etc.

It would be nice to have keywords to check or assert element visibility, like:

Element Should Be Visible Element Should Not Be Visible

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 15 Oct 2010

This needs some clarification:

1) Is element visible if it's not shown on the browser window because the window is too small?

2) How should Element Should Not Be Visible behave when the element doesn't exist at all?

3) How well Selenium supports this?

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 15 Oct 2010

Good thoughts. I'd say:

1) "Visible" means "logically visible" as in "it doesn't have display:none or other properties on it" - we could look into how Selenium checks visibility in order to explore their interpretation of "visible"

2) I think that it should be a precondition that the element exists - why should you check visibility on an element that isn't there? I think the keyword should fail if the element doesn't exist. If anyone has an example of a case where it makes sense to not make the keyword fail if the element isn't there, I'd love to take that as additional input :)

3) I think that if I do some research for 1) by looking into Selenium inner workings, I'll probably find out how well this is implemented.

Pekka, what's your opinion on the questions?

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 15 Oct 2010

I agree with Robert about 1) and 2). Just wanted to make this sure. And want to remind that these need to be mentioned in the kw's documentation.

Are there other ways to hide elements in HTML than setting display:none? If not, or if this is what people use 98% of times, implementing this keyword should be relatively easy even if Selenium wouldn't have any special support.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 15 Oct 2010

Ways that I know to make elements invisible:

If anyone knows additional ways, I'm happy to hear about those.

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 15 Oct 2010

I don't think we can check this from CSS because:

I understood that Selenium has isVisible method. If that's true, implementing this keyword ought to be trivial. Us implementing this otherwise is probably too much work.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 16 Oct 2010

Yes, Selenium offers isVisible and, automatically, also verifyVisible. I think we should rely on that and create keywords that wrap isVisible in a nice way. For example, if you do:

Call Selenium Api isVisible //body

you will get "OK,true" as the result. Thus, the keyword should split the result string and handle its parts accordingly.

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 17 Oct 2010

Instead of using the isVisible method in the Selenium RC API, this keyword should use is_visible in Selenium Python API. The Python API is what all other keywords use too, and the methods there already handle the return value from the RC API. Notice that you can also call the Python API with the Call Selenium API keyword.

This keyword should also handle the given locator so that both name and id are supported without an xpath. This is what other similar keywords do too.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 28 Oct 2010

Implemented and pushed, complete with tests. I'd be happy about a review of r7587883b7a.

spooning commented 9 years ago

Originally submitted to Google Code by @pekkaklarck on 29 Oct 2010

Looks great!

I was wondering should the locator be converted with self._parse_locator like most other keywords using locators do, but after looking at the code and discussing with Janne, I realized that in this case that would be a no-operation. The _parse_locator method is needed in more complicated cases where we want it to be possible to use something else than id or name (or xpath=, dom=, css=) as locator. These extra attributes are context-sensitive and keywords operating any element (like these ones) cannot use them anyway.

spooning commented 9 years ago

Originally submitted to Google Code by spielman... on 29 Oct 2010

Nice. I'd say this is done :)