Skip to content

Conversation

j1z0
Copy link
Contributor

@j1z0 j1z0 commented Mar 15, 2012

ok here is my attempt at getting frames to work in painless manner. Basically I changed the element_finder functionality so that the filter could handled a list of tags. ie. ['frame','iframe'] as opposed to a single tag. This also required some changes to the xpath generation for find by default stuff.

Also I updated all the appropriate functions that have to do with frames to look for both frames and frames. Finally I added a couple of tests.

All the test pass. But I'm would be interested in feedback, especially if there is a way to make the code cleaner... cooler... nicer :)

…s with multiple tags. ie. tag=['frame','ifram']
@emanlove
Copy link
Member

One of the problems I saw with several of the suggested fixes is that they confuse what element_finder means by "tag". element_finder understands "tag" to be both "element type" and what most people think of as "html tag", see _get_tag_and_constraints.

I see the point of having multiple tags when it comes to frames but I am not sure it makes sense to have multiple tags for anything else as one would be searching for one tag or element type. I'm also not sure about falling though to "find by default" and in particular the key attrs search as frames is more of a element type then an attribute of a specific type.

@emanlove
Copy link
Member

I'm also not sure about falling though to "find by default" and in particular the key attrs search as frames is more of a element type then an attribute of a specific type. - emanlove

I'm getting myself all confused here as to the definition of a frame; whether it is an element or an attribute. According to the HTML spec both frame and iframe are elements while the xhtml spec states frame to be an attribute of table. I am still working through some fundamental concepts before I finalize me thoughts on any piece of code.

One thing that I am wondering is whether user will want to differentiate between and frame element and an iframe element. I can see that some might not care but others might want to only match one or the other. Ultimately I could see a Frame utility class similar to Selenium's Select class.

@adwu73
Copy link
Contributor

adwu73 commented Mar 16, 2012

j1z0, I got your point, I am willing to test your fix on my real apps once done.

From my point view, there are no need to differentiate frame and iframe in test cases, this is an implementation details that tester don't need to care.

@j1z0
Copy link
Contributor Author

j1z0 commented Mar 16, 2012

@emanlove Frame and iFrame are element types they are not attributes. Conceptually they are a way to insert one html page inside of another html page.

Regarding the fall through to find by default, that has always been there. However I get your point, perhaps we can create a special strategy in the element_finder _find_frames or something like that. I guess this is probably the same idea as the Frame utility class you mentioned. The advantage there is it would keep the rest of the element_finder.py code cleaner.

Also as it stands now, a tester doesn't need to worry about the differences between frame and iframe, but they can always use the locator argument to specifically limit to a particular tag. something like css=iFrame[#this_one] or whatever. I think that much should remain, i.e. by default you don't have to care, but you can pick if you really need to.

@j1z0 j1z0 closed this Mar 16, 2012
@adwu73
Copy link
Contributor

adwu73 commented Mar 22, 2012

j1z0, have this iframe support issue been fixed in selenium2lib 1.0? tks! Adam

@j1z0
Copy link
Contributor Author

j1z0 commented Mar 22, 2012

the fix is still in the iframe branch, so you could manually pull that down, but it didn't make it into 1.0. I'm still waiting for acceptance on the merge request from the other devs.

Cheers,
Jeremy

On Mar 22, 2012, at 3:31 PM, adwu73 wrote:

j1z0, have this iframe support issue been fixed in selenium2lib 1.0? tks! Adam


Reply to this email directly or view it on GitHub:
#41 (comment)

@adwu73
Copy link
Contributor

adwu73 commented Mar 24, 2012

tks, Jeremy for the update

@emanlove
Copy link
Member

On 03/23/2012 09:51 PM, adwu73 wrote:

tks, Jeremy for the update


Reply to this email directly or view it on GitHub:
#41 (comment)

Adam,

A fix for the iframe issue has been merged into rtomac/master.

Ed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants