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

feat: Add 'mobile: calibrateWebToRealCoordinatesTranslation' API #2071

Merged
merged 21 commits into from
Oct 13, 2023

Conversation

mykola-mokhnach
Copy link
Contributor

Based on appium/WebDriverAgent#785

Also removed the deprecated moveTo endpoint

@mykola-mokhnach
Copy link
Contributor Author

@KazuCocoa Could you please help to test the feature on real devices?

lib/commands/web.js Outdated Show resolved Hide resolved
@KazuCocoa
Copy link
Member

sure, I'll check on real device(s)

Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

btw, is it helpful to do this calculation instead of opening /health by default, or before opening the default page /health or safariInitialUrl (in the future)?

lib/commands/web.js Outdated Show resolved Hide resolved
docs/capabilities.md Show resolved Hide resolved
lib/commands/web.js Outdated Show resolved Hide resolved
@KazuCocoa
Copy link
Member

In iPad, the pixelRatio was 1 when it was landscape, but that was 2 in the portrait. It caused a wrong click point in the portrait. The legacy one clicked properly in such a case.

Probably... we may need to get proper pixelRatio...? (I haven't researched yet)

docs/execute-methods.md Outdated Show resolved Hide resolved
@mykola-mokhnach
Copy link
Contributor Author

In iPad, the pixelRatio was 1 when it was landscape, but that was 2 in the portrait. It caused a wrong click point in the portrait. The legacy one clicked properly in such a case.

Probably... we may need to get proper pixelRatio...? (I haven't researched yet)

did you recalibrate after changing the orientation?

@KazuCocoa
Copy link
Member

Yes, it seems like window.devicePixelRatio returned a different ratio depending on the orientation, 1 (landscape) and 2 (portrait) while maybe the landscape's x1 was the expected one as the device. That was iPad mini

@mykola-mokhnach
Copy link
Contributor Author

Yes, it seems like window.devicePixelRatio returned a different ratio depending on the orientation, 1 (landscape) and 2 (portrait) while maybe the landscape's x1 was the expected one as the device. That was iPad mini

This is weird. Let me try to use then the scale value returned by xctest

@KazuCocoa
Copy link
Member

hmmm, confirmed the device had > driver.execute_script 'mobile: deviceScreenInfo' => {"statusBarSize"=>{"width"=>768, "height"=>20}, "scale"=>2}, so the scale was 2. But the scale calculation made the wrong tap coordinate as #2071 (comment) ...
Let me check a few combinations...

@KazuCocoa
Copy link
Member

KazuCocoa commented Oct 9, 2023

I haven't found a better way for across devices yet. e.g. 69b462f but this was not good.

What I have tried so far as testing is:

  • open google.com
  • find element "About" (Bottom of the screen)
  • click it <- if the click opens the about

Screenshot 2023-10-08 at 10 10 25 PM

So far, the existing nativeWebTap handles them properly but based on this PR idea's ones are not yet

@mykola-mokhnach
Copy link
Contributor Author

I am now using a more sophisticated algorithm to measure actual pixel ratio values

@KazuCocoa
Copy link
Member

KazuCocoa commented Oct 9, 2023

hm, the latest one worked only my iPad landscape only. Simulator iPhones, iPad portrait etc did not click the element position, especially the Y was far from the target element, almost center in iPhone 15 simulator.

@mykola-mokhnach
Copy link
Contributor Author

hm, the latest one worked only my iPad landscape only. Simulator iPhones, iPad portrait etc did not click the element position, especially the Y was far from the target element, almost center in iPhone 15 simulator.

strange, the translation did work in my local tests on ios 17 simulator. At least both x and y offsets as well as scale factors were calculated properly. I've changed Math.trunc to Math.round to have better approximations since xctest operates with float coordinates, but otherwise seems good to me.

Could you please provide me your client code, so I could run it locally?

@KazuCocoa
Copy link
Member

Sure, like below. Not the same as #2071 (comment) , but this also did not click the element. With an iPhone 15 iOS 17 simulator, the click did not click the element. (I haven't checked other simulator variations). WDA was already launching on the simulator

server_url = 'http://localhost:4723'
caps = {
  "platformName": "ios",
  "automationName": "xcuitest",
  "platformVersion": "17.0",
  "deviceName": "iPhone 15",
  "browserName": :safari,
  "webDriverAgentUrl": "http://localhost:8100/",
  "nativeWebTap": true,
  "nativeWebTapStrict": true
}
core = Appium::Core.for url: server_url, caps: caps
driver = core.start_driver
driver.get 'https://google.com'
driver.execute_script 'mobile: calibrateWebToRealCoordinatesTranslation'
e = driver.find_element :link_text, 'Making the internet safer for kids and teens'
e.click  # click does not happen

https://gist.github.com/KazuCocoa/7bf791f54317bd464feb29095a2f6088 is the log that I ran with the above with the latest PR code. The last click did not click the expected element.

Current master clicked {"x":196,"y":337}, which clicked the element with the same capability. https://gist.github.com/KazuCocoa/7bf791f54317bd464feb29095a2f6088?permalink_comment_id=4719947#gistcomment-4719947 is the click log with current master.

Screenshot 2023-10-10 at 12 11 12 AM

Btw, I noticed driver.execute_script 'mobile: calibrateWebToRealCoordinatesTranslation' required a few times to run occasionally on a real device since each returned a different result in the same session. Probably the calibration page's loading condition might be affected.

@mykola-mokhnach
Copy link
Contributor Author

Thanks @KazuCocoa

The more I dive into this topic the more complicated it looks. Made it work with Simulator properly. Please check with real devices again

@KazuCocoa
Copy link
Member

Sure, thanks. I'll take a look

lib/commands/web.js Outdated Show resolved Hide resolved
Copy link
Member

@KazuCocoa KazuCocoa left a comment

Choose a reason for hiding this comment

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

lgtm 🚀

@mykola-mokhnach mykola-mokhnach merged commit b3fa78d into appium:master Oct 13, 2023
18 checks passed
@mykola-mokhnach mykola-mokhnach deleted the calibrate branch October 13, 2023 11:14
github-actions bot pushed a commit that referenced this pull request Oct 13, 2023
## [5.7.0](v5.6.0...v5.7.0) (2023-10-13)

### Features

* Add 'mobile: calibrateWebToRealCoordinatesTranslation' API ([#2071](#2071)) ([b3fa78d](b3fa78d))
@github-actions
Copy link
Contributor

🎉 This PR is included in version 5.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants