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

touch actions *do* work in webview #4005

Closed
wants to merge 2 commits into from
Closed

Conversation

lukeis
Copy link
Contributor

@lukeis lukeis commented Nov 7, 2014

No description provided.

@jlipps
Copy link
Member

jlipps commented Nov 7, 2014

@Jonahss weren't you investigating this recently? is this the fix we want?

@lukeis
Copy link
Contributor Author

lukeis commented Nov 8, 2014

works for me locally :) and it's the only way i've been able to get a swipe working when running against MobileSafari.

Side note... why don't you support switching to the "Native App" context when running against MobileSafari?

@jlipps
Copy link
Member

jlipps commented Nov 8, 2014

Side note... why don't you support switching to the "Native App" context when running against MobileSafari?

we don't?

@lukeis
Copy link
Contributor Author

lukeis commented Nov 10, 2014

@jlipps sorry, mistaken about that last bit

@Jonahss
Copy link
Member

Jonahss commented Nov 10, 2014

So touch actions work in webviews in iOS. The problem is that in Android, if you perform a touch action on an element (rather than on x,y coordinates) the code tries to find the web element while in the NATIVE context, doesn't find it, so the touch action fails.

I haven't checked this since @0x1mason's recent changes to touch actions though, so it might work now.

@0x1mason
Copy link

@Jonahss I didn't make any changes relating to contexts, so I doubt my code will make any difference.

@paymand
Copy link
Member

paymand commented Nov 13, 2014

Looks like we should be able to break it down for Android and iOS, allow it on iOS and raise a notYetImplemented on Android?

@Jonahss
Copy link
Member

Jonahss commented Nov 13, 2014

I'm okay with that.

On Thu, Nov 13, 2014 at 7:34 AM, Payman Delshad notifications@github.com
wrote:

Looks like we should be able to break it down for Android and iOS, allow
it on iOS and raise a notYetImplemented on Android?


Reply to this email directly or view it on GitHub
#4005 (comment).

@jlipps
Copy link
Member

jlipps commented Nov 13, 2014

they should work on android too---a while ago I thought I pushed a change that allowed you to scope back out to NATIVE_APP when working with an android webview.

@Jonahss
Copy link
Member

Jonahss commented Nov 13, 2014

They don't work when you supply an element reference. They do work when you
supply {x,y} coordinates.

On Thu, Nov 13, 2014 at 11:26 AM, Jonathan Lipps notifications@github.com
wrote:

they should work on android too---a while ago I thought I pushed a change
that allowed you to scope back out to NATIVE_APP when working with an
android webview.


Reply to this email directly or view it on GitHub
#4005 (comment).

@lukeis
Copy link
Contributor Author

lukeis commented Nov 13, 2014

So, I guess the question is do you want to guard against the cases that don't work now or let them error in an inconsistent way for now until they are supported?

@jlipps
Copy link
Member

jlipps commented Nov 13, 2014

Can we not just error with NoSuchElementException?

@lukeis
Copy link
Contributor Author

lukeis commented Nov 13, 2014

is that the only case?

@0x1mason
Copy link

@Jonahss

They don't work when you supply an element reference. They do work when you supply {x,y} coordinates.

How hard is it to fix this?

@jlipps
Copy link
Member

jlipps commented Nov 13, 2014

How hard is it to fix this?

It is impossible to fix. Chromedriver elements and native Android elements live in two hermetically compartmentalized worlds. You can't use a webview element with a native method and vice versa.

@lukeis
Copy link
Contributor Author

lukeis commented Nov 13, 2014

in other projects I know of ;) when getting an element, we would get the center location of it and then use that (effectively as if the user passed in the x,y)

@0x1mason
Copy link

@jlipps

It is impossible to fix. Chromedriver elements and native Android elements live in two hermetically compartmentalized worlds. You can't use a webview element with a native method and vice versa.

I was thinking more workaround. Can't you derive the x,y coordinates from the webview element and send them to the native context?

@0x1mason
Copy link

Ha, I see @lukeis had the same thought.

@jlipps
Copy link
Member

jlipps commented Nov 14, 2014

yep, we could do that

@Jonahss
Copy link
Member

Jonahss commented Nov 14, 2014

Yeah it's not a complex change. I was hoping we could resolve it that way :)

On Thu, Nov 13, 2014 at 4:10 PM, Jonathan Lipps notifications@github.com
wrote:

yep, we could do that


Reply to this email directly or view it on GitHub
#4005 (comment).

@0x1mason
Copy link

@Jonahss Created a ticket and milestoned for 1.3.4
#4075

@jlipps
Copy link
Member

jlipps commented Nov 14, 2014

meanwhile, any reason we shouldn't merge this since it's at least a partial fix?

@Jonahss
Copy link
Member

Jonahss commented Nov 14, 2014

Mmmm. It's going to make some Android webview touch actions fail with a bad error, but sure, we can merge it.
Just might confuse some users. But hey, let them use everything that works, right?

@0x1mason
Copy link

@Jonahss @jlipps If it's going to create confusion, I think we should wait. Less confusion is awesome for our users and saves them headaches. It also means we can put more time into completing the feature rather than triaging questions about unexpected behavior.

@jlipps
Copy link
Member

jlipps commented Nov 19, 2014

If it's going to create confusion, I think we should wait. Less confusion is awesome for our users and saves them headaches.

I agree. I'm not sure which would cause more confusion though:

  1. I'm using webviews! I'm using TouchActions! Oh wait, I get a not yet implemented error. That doesn't make sense...
  2. I'm using webviews! I'm using TouchActions! I have a specific web element I want to interact with! Oh wait, I get an unknown element error. That doesn't make sense...

If we agree it's (2) then we should leave this unmerged. If we agree it's (1) then we should merge this. I don't have a strong opinion either way. Happy to punt to a future release

@jlipps
Copy link
Member

jlipps commented Nov 20, 2014

@Jonahss @lukeis @0x1mason please weigh in on the above so we can merge or close this

@lukeis
Copy link
Contributor Author

lukeis commented Nov 20, 2014

I'm 👍 for option 1

@lukeis
Copy link
Contributor Author

lukeis commented Nov 20, 2014

oops, edited ;)

@appium-ci
Copy link

Can one of the admins verify this patch?

2 similar comments
@appium-ci
Copy link

Can one of the admins verify this patch?

@appium-ci
Copy link

Can one of the admins verify this patch?

@BhaskarVj
Copy link

press(), longPress() are working fine with Native App on Android...But moveTo(),tap() aren't working.
I'm using element,not with co-ordinates...
Appium 1.3.4
Windows 7
Android 4.4.2, Java Bindings 2.1.0
Please suggest me in a better way.Thank you in advance :)

@appium-ci
Copy link

Can one of the admins verify this patch?

@0x1mason
Copy link

@bhaskar-appium If you have an implementation question, please try asking at https://discuss.appium.io/. You may want to include the code in question since it's difficult to evaluate the problem without it.

@appium-ci
Copy link

Can one of the admins verify this patch?

@0x1mason
Copy link

I think the short-term solution to this issue in Android is to implement touch actions with chrome-remote-interface, perhaps as a stopgap. Over the long-term, we should probably ditch the ChromeDriver server as a separate process. We might be able to use CD as a library and run a CD-based http listener from bootstrap. That would solve our touch action problems, greatly simplify CD instance management, improve debugging, and generally increase our control over implementation.

@lukeis
Copy link
Contributor Author

lukeis commented Feb 10, 2015

gonna close this PR, not seemingly getting around to implementing this.

@lukeis lukeis closed this Feb 10, 2015
@0x1mason
Copy link

@lukeis We'll try to get this in for the next release, v1.3.6.

@0x1mason 0x1mason reopened this Feb 10, 2015
@appium-ci
Copy link

Can one of the admins verify this patch?

@lukeis
Copy link
Contributor Author

lukeis commented Feb 10, 2015

@0x1mason why reopen? unless you're using this to track the issue?

I meant to say "I'm not seemingly getting around to implementing this correctly"

@0x1mason
Copy link

@lukeis Perhaps I misread. I thought it was just missing tests. I'll go ahead an reclose.

@0x1mason 0x1mason closed this Feb 10, 2015
@doronzRumble
Copy link

can i understand that since this issue got closed, it wont be implemented as a feature for one of the upcoming releases?
is there a workaround for this to enable swiping in webapp?

@lukeis
Copy link
Contributor Author

lukeis commented Dec 27, 2015

@doronzRumble you can switch to Native and perform the action (given the coordinates of the webview items that you can fetch before switching to native)

@buekera
Copy link

buekera commented Jan 12, 2016

@lukeis Yes we can do that but it more a "hacky" way in my opinion. I don't like working on native context when I do everything else on the webview.

@0x1mason I think you forgot to implement it, we are on version 1.4.16 now...

@jlipps
Copy link
Member

jlipps commented Jan 12, 2016

@Choonz this was closed as 'won't merge' for the time being. we might implement the feature in a future version of appium though

@buekera
Copy link

buekera commented Jan 13, 2016

@jlipps Sounds good for me. Is there an estimation when this will come in?

I really have trouble clicking on elements in my webview and I guess I'm not the only one.

@jlipps
Copy link
Member

jlipps commented Jan 13, 2016

@Choonz as a workaround you can always get into the native context and do the tapping based on coordinates.

@buekera
Copy link

buekera commented Jan 14, 2016

@jlipps No I actually can't switch to the native context because we want to use the same tests inside a browser, there is no native context.

@jlipps
Copy link
Member

jlipps commented Jan 14, 2016

@Choonz so when you call the getContexts command there is only one thing in it?

@buekera
Copy link

buekera commented Jan 20, 2016

@jlipps No, I can see the native context and the webview context. Working inside the native context is no option for us though so switching back and forth will not work for us. Otherwise I would switch to native context to perform the tap.

I already got it working on webview context by using the JSExecutor and trigger the touchend event. It's an ugly way to do it though but it's working for now.

@jlipps
Copy link
Member

jlipps commented Jan 20, 2016

ok glad you found a workaround; however i'm curious why the native context is no option for you?

@buekera
Copy link

buekera commented Feb 16, 2016

@jlipps We decided to use the native context instead of JSExecutor. I now use the touch actions of appium to tap on x, y coodinates. The problem I ran into is converting the webview coordinates of an element to the native coordinates. It seems to be a little bit tricky due to the fact that the small android menu on the top has different heights (depends on the devices resolution). Is there any solution to this problem?

@jlipps
Copy link
Member

jlipps commented Feb 16, 2016

@Choonz does that menu not have height attributes?

@buekera
Copy link

buekera commented Feb 17, 2016

@jlipps By menu I mean the system menu provided by android itself. I didn't find a way yet to access it and then read attributes from it. I hope you know about which menu I am talking about. :D

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.

None yet

10 participants