Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

fix hybrid element position determination #1906

Merged
merged 3 commits into from
Sep 19, 2021

Conversation

wswebcreation
Copy link
Contributor

When switched to the webview of a hybrid app the elements on the screenshot were not aligned with the expected position. See screenshot

Android:
image

iOS:
image

This PR will fix that by determining the position of the first webview on in the hybrid app, see below

Android:
image

iOS:
image

This will also fix the issue that Android hybrid apps got stuck when switching to the webview. The nativeWebScreenshot was needed to get a full screen screenshot instead of the ChromeDriver Webview screenshot

when switched to the webview of a hybrid app the elements on the screenshot were not aligned with the expected position. This will fix that by determining the position of the first webview on in the hybrid app
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 16, 2021

CLA Signed

The committers are authorized under a signed CLA.

const chromeCustomCaps = {
const {platformName = ''} = caps;
const androidCustomCaps = {
// @TODO: remove when this is defaulted in the newest Appium 1.8.x release
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.8.x? must be a quite old one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was code I copied over, not sure if we still need it but didn't want to change that in this PR

@jlipps
Copy link
Member

jlipps commented Sep 16, 2021

Given that this is being targeted to the old AD repo, I don't see that it will ever be published. This PR should be opened against the inspector repo instead. (Unless @KazuCocoa is specifically publishing old versions of AD, which I think would be confusing now that we have fully made the switch).

@KazuCocoa
Copy link
Member

yea, the reason why I've maintained this branch was the inspector blocked https connection. That was already resolved in a recent update (HTTP libraries and webdriverio update). So it would be great to create the same change to the inspector repo.
And release the inspector first. (This PR itself can keep here)
if we still have some blocking issue, I'm okay to update this branch

afaik, after appium/appium-inspector#118 , cloud vendors also can use appium-inspect on their env.
(some of them probably still need to update their capabilities. I already handle it in appium/appium-inspector#128)

@wswebcreation
Copy link
Contributor Author

I totally understand @jlipps and @KazuCocoa

But this is a fix for a customer who is using Sauce Labs which is currently not yet supported with the new one (or is it now).
It would be great if we can make a patch release. I'll add this in the new repo next week or something, when I have some more time

@jlipps
Copy link
Member

jlipps commented Sep 16, 2021

@wswebcreation the new one will support sauce labs as soon as I merge the other PR. I think just need someone to sign the CLA for me to do that. so I would be able to publish that other one sooner than this one probably.

@wswebcreation
Copy link
Contributor Author

Which CLA do you mean @jlipps ?

I’ve build the app now on my local machine and send it to the customer so we’re good now

@KazuCocoa
Copy link
Member

I see.
I can create a patch version as 1.21.0-1.

1.22.0 version will be the separate inspector version, so the patch will reduce confusion, I think.

@wswebcreation
Copy link
Contributor Author

A patch would be great if that is possible

@KazuCocoa KazuCocoa merged commit 0ac98ec into appium:1.21 Sep 19, 2021
@KazuCocoa
Copy link
Member

@wswebcreation wswebcreation deleted the fix-hybrid-element-positions branch September 20, 2021 05:03
@wswebcreation
Copy link
Contributor Author

Thanks @KazuCocoa !!!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
4 participants