Skip to content
This repository has been archived by the owner on Dec 26, 2019. It is now read-only.

A Short Explanation: Why there are problems with landscape orientation #856

Open
mykola-mokhnach opened this issue Jan 22, 2018 · 3 comments

Comments

@mykola-mokhnach
Copy link
Contributor

Facebook decided to close Wiki from editing by third-parties, so I just leave it here.

Bug Description

There are many complains about interaction issues in non-default screen orientation mode. All these problems are coming from a single source. XCTest has a known bug when it fails to transform element coordinates properly in non-portrait orientation. It means that if https://github.com/facebook/WebDriverAgent/blob/52512a7fb39a697fc727d372c8765a43c74f6d37/WebDriverAgentLib/Commands/FBOrientationCommands.m#L101 was called with orientation value different from UIDeviceOrientationPortrait then interface elements, which belong to some windows from the current hierarchy, will change their frames, while others won't.

The Problem

Usually, it is the case, that if the particular window belongs to a system service then its descendants will have "correct" frame coordinates and if it a custom application then the4 coordinates will be "corrupted". Under "corrupted" I assume that click or other actions will be executed not in the place, where we expect it to be executed (or simply offscreen) if sent to such element. This also includes the case when coordinates are calculated based on the global screen size, since the "real" screen dimensions constructed by XCTest are not the same as returned to you.

The described problem also affects, for example, screenshots. The are incorrectly rotated in non-portrait mode (the bottom side of the picture is not the bottom of the screen).

Also, it is known, that the problem shows itself differently in iOS9 and iOS10+.

The Solution

The WDA implementation from Facebook tries to workaround the problem by adjusting coordinates based on the current interface orientation value. It assumes the coordinates should be always fixed if the orientation is different from the default one. See

https://github.com/facebook/WebDriverAgent/blob/52512a7fb39a697fc727d372c8765a43c74f6d37/WebDriverAgentLib/Commands/FBElementCommands.m#L457

and

https://github.com/facebook/WebDriverAgent/blob/52512a7fb39a697fc727d372c8765a43c74f6d37/WebDriverAgentLib/Categories/XCUIElement%2BFBTap.m#L37

Here you have the methods, which "fix" the coordinates:

https://github.com/facebook/WebDriverAgent/blob/52512a7fb39a697fc727d372c8765a43c74f6d37/WebDriverAgentLib/Utilities/FBMathUtils.m#L41
https://github.com/facebook/WebDriverAgent/blob/52512a7fb39a697fc727d372c8765a43c74f6d37/WebDriverAgentLib/Utilities/FBMathUtils.m#L56

So Why It Still Fails?

This works in most of the cases, but sometimes, due to the fact described in The Problem section, element coordinates might not be inverted. And since we try to "fix" them anyway, this leads to "missclicks". Also, Apple still changes this behaviour in major XCTest releases (perhaps, it's even going to be fixed sometimes ;-) ).

Is There Any Solution?

In the Appium fork we've applied more "advanced" logic to detect the fact of coordinates corruption in non-default screen orientations. This logic relies on the fact that if an element has "corrupted" frame then it's window is inverted relatively to the application's window frame. You can check

https://github.com/appium/WebDriverAgent/blob/02887c171699821314c1928642303c36f037ae8a/WebDriverAgentLib/Utilities/FBBaseActionsSynthesizer.m#L57

https://github.com/appium/WebDriverAgent/blob/02887c171699821314c1928642303c36f037ae8a/WebDriverAgentLib/Utilities/FBMathUtils.m#L86

for more details. We still don't know whether the particular element coordinates are OK if global coordinates are used. Also, this trick does not work with upside-down orientation. That is why the solution would be to avoid using global coordinates, but rather rely on some particular parent container UI element (or the window itself, which is always a child of Application element), while calculating offsets.

How Can I Help?

We are welcome to contributions, that can help us to solve the described problems or find better/faster ways to workaround them.

@shynkevich-alex
Copy link

any progress?

DeviLeo pushed a commit to DeviLeo/WebDriverAgent that referenced this issue Mar 1, 2018
…pplication orientation is landscape and tapX > application portrait width or tapY > application portrait height.

Summary:
 If SDK >= 11, the tap coordinate based on application is not correct when
 the application orientation is landscape and
 tapX > application portrait width or tapY > application portrait height.
 Pass the window element to the method [FBElementCommands gestureCoordinateWithCoordinate:element:]
 will resolve the problem.
 More details about the bug, please see the following issues:
 facebookarchive#705: facebookarchive#705
 facebookarchive#798: facebookarchive#798
 facebookarchive#856: facebookarchive#856
 Notice: On iOS 10, if the application is not launched by wda, no elements will be found.
 See issue facebookarchive#732: facebookarchive#732
DeviLeo pushed a commit to DeviLeo/WebDriverAgent that referenced this issue Mar 1, 2018
Summary:
  If SDK >= 11, the tap coordinate based on application is not correct when
  the application orientation is landscape and
  tapX > application portrait width or tapY > application portrait height.
  Pass the window element to the method [FBElementCommands gestureCoordinateWithCoordinate:element:] will resolve the problem.
  More details about the bug, please see the following issues: facebookarchive#705 facebookarchive#798 facebookarchive#856
  Notice: On iOS 10, if the application is not launched by wda, no elements will be found. See issue facebookarchive#732
@GWesley
Copy link

GWesley commented Mar 9, 2018

All tried, not work on iPhone7 plus 11.3

@vikramvi
Copy link

vikramvi commented Mar 9, 2018

Facebook should show a bit more Sympathy & Empathy to thousands of struggling automation engineers across globe.

I'm sure they have enough money to allocate dedicated resources for this critical project.

@mykola-mokhnach Thanks for your endless patience with FB :)

facebook-github-bot pushed a commit that referenced this issue May 25, 2018
Summary:
If SDK >= 11, the tap coordinate based on application is not correct when
  the application orientation is landscape and
  tapX > application portrait width or tapY > application portrait height.
  Pass the window element to the method [FBElementCommands gestureCoordinateWithCoordinate:element:] will resolve the problem.
  More details about the bug, please see the following issues: #705, #798, #856.
  Notice: On iOS 10, if the application is not launched by wda, no elements will be found. See issue #732.
Closes #878

Differential Revision: D7947931

Pulled By: marekcirkos

fbshipit-source-id: 4f8ef1484761b79c69f67df40293eea70bc08984
mykola-mokhnach referenced this issue in appium/WebDriverAgent Jun 6, 2018
* Remove TableView scrolling integration tests

Summary: We have duplicated scrolling tests for regular tableViews and simple scrollviews. It was usefull to have them, but now testing scrolling on scrollviews covers both cases anyway and we will waste less time testing.

Reviewed By: antiarchit

Differential Revision: D6543548

fbshipit-source-id: 35e717c43208b8f138a706798d9aa8a9e34b302a

* Use fb_tapWithError in alert integration tests

Summary:
Some alert integration tests are flaky on our CI. I suspect this happens due to failing `tap` command, which might be failing to tap on animated view. This know XCTest issue.
Fortunately we have `fb_tapWithError` that handels are common pitfalls.

Reviewed By: antiarchit

Differential Revision: D6543616

fbshipit-source-id: f12ea7da67058253deff53fa980ebf2e55a93899

* Fix rotation integeation tests

Summary: Rather then depending on XCUITest information on orientation, we should test what is application perception

Reviewed By: antiarchit

Differential Revision: D6543511

fbshipit-source-id: 192e50d0202f96bd6119921c9353a72789b33ad4

* Kill testExtrasIconContent

Summary:
This test is super device/simulator specific, that was used to interate on visibility improvements.
Right now it does not bring much value anyway.

Reviewed By: antiarchit

Differential Revision: D6543627

fbshipit-source-id: ffbda3b0b5ecde6a632e88030e84fae5af614347

* Use Xcode 9.2 and iOS 11.2 on Travis

Summary: It is time to move on.

Reviewed By: antiarchit

Differential Revision: D6692223

fbshipit-source-id: 81913167551191b2f9e177ff1ead943f6fc25752

* Fix scrolling on big scroll offsets

Summary:
`XCUICoordinate` seems to have a bug that will return half of offset in case coordinate referance point goes off screen. I suspect this has something to do with retina displays.
Instead of using Apple's implementation of `screenPoint` I implemented our own.

Reviewed By: antiarchit

Differential Revision: D6702218

fbshipit-source-id: b01ef061732281cdf8c6707de38a11b6e31b9c28

* Run testInvisibleDescendantWithXPathQuery test on scroll page

Summary:
With all visibility changes tested pageindicator is no longer considered invisible. For the time being this is ok with us.
This diff uses different invisible elements to run same test of search for invisible elements

Reviewed By: antiarchit

Differential Revision: D6702252

fbshipit-source-id: a2bd5198d14f05cb1b33afe76a6b5bda7e34a847

* Do not run testSpringBoardIcons on iPads

Summary: This test was not intended to run on iPad, so we should turn it off so that we can make Travis green again :).

Reviewed By: antiarchit

Differential Revision: D6726707

fbshipit-source-id: db92f23b2dc755e7c1464069c04b1838717b8ff4

* Fix resolving XCUIApplication snapshots

Summary:
With new xcode it looks like, calling `query`, `resolve` on XCUIApplication will in some sense break it. Not sure exactly what goes wrong, but final result is that it's missing some children and description of that object is empty.
Instead we can use new API for fetching snapshots for debugging.

Reviewed By: lawrencelomax

Differential Revision: D6723102

fbshipit-source-id: a33d9caf03eef4450f3879996493493c2cb257b4

* Use visibleFrame to calculate scrolling vector

Summary:
Using views frame for calculating scroling vector might be buggy when application is doing tricky stuff with scolling views eg. nesting scrollview within scrollview.
In that case frame of the second scrollview will be huge and make scrolling imposible as we will try to use points of the screen. Instead we should visible frame.

Reviewed By: antiarchit

Differential Revision: D6819626

fbshipit-source-id: 27062f593485b4ec55ae8bd1a9c9a32a89630b92

* fb_isVisible using interfaceOrientation instead of Device orientation.

Summary:
[XCUIDevice sharedDevice].orientation may get values like FaceUp, which cause wrong calculation of the app frame size, in landscape.

With devices in Lab, standing portrait, this problem is less common. Working on development with devices on the table it happens more often causing a great deal of confusion.

From my tests, using app interfaceOrientation gives always the reality of the App.
Closes facebookarchive#875

Reviewed By: marekcirkos

Differential Revision: D7067153

Pulled By: antiarchit

fbshipit-source-id: c6aa8f8823d0f43e5072a190c34df931d0d6b8b8

* Fix testIconsFromSearchDashboard integration test

Summary:
It might be there is more than one `IntegrationApp` icon on the dashboard while integration tests are running on Travis. Addresses https://travis-ci.org/facebook/WebDriverAgent/jobs/349725814
Closes facebookarchive#883

Differential Revision: D7289614

Pulled By: marekcirkos

fbshipit-source-id: e5060888f2a752c039fcf815984e24579a3cef06

* Allow to override typing frequency for single typing request

Reviewed By: antiarchit

Differential Revision: D7615943

fbshipit-source-id: 9cb82dce23c02970d0c29613c7de4ebca0838361

* Include RoutingHTTPServer in copy to fix ISSUE #902

Summary:
Include RoutingHTTPServer in copy to fix ISSUE #902
Closes facebookarchive#915

Differential Revision: D7947944

Pulled By: marekcirkos

fbshipit-source-id: ca60238fc3f715f753f084493aec5afa4524b21b

* Fix the incorrect tap coordinate in landscape mode.

Summary:
If SDK >= 11, the tap coordinate based on application is not correct when
  the application orientation is landscape and
  tapX > application portrait width or tapY > application portrait height.
  Pass the window element to the method [FBElementCommands gestureCoordinateWithCoordinate:element:] will resolve the problem.
  More details about the bug, please see the following issues: #705, #798, #856.
  Notice: On iOS 10, if the application is not launched by wda, no elements will be found. See issue #732.
Closes facebookarchive#878

Differential Revision: D7947931

Pulled By: marekcirkos

fbshipit-source-id: 4f8ef1484761b79c69f67df40293eea70bc08984

* Update flow-bin for WebDriverAgent's Inspector

Differential Revision: D8220228

fbshipit-source-id: cf08317ab2e50282373576fedb7c2b26b47969ad

* Handle infinitive numbers in elements rect

Summary:
In case testmanagerd has problems in calculating frames it may return 'inf' instead which does not behave well with JSON encoding.
Therfore checing for infs when formating JSON reponse

Reviewed By: antiarchit

Differential Revision: D7067158

fbshipit-source-id: 180400186af49f1353d93e9eb61d7306b8a1a0f6

* force touch support

Summary:
A couple of commands which make possible to force touch given element or point by coordinates. In fact – copy of the tap extension. Baked with integration tests.

Updated with mykola-mokhnach improvements in  #79
Closes facebookarchive#917

Differential Revision: D8220249

Pulled By: marekcirkos

fbshipit-source-id: 2a14ab5759894577a1f5e40f20b4a6d79e519419

* Support getting the element cache size, clearing the element cache

Summary:
Every time a user finds an element using the `element` or `elements` route, a new `XCUIElement` object is added to the session cache. The elements are never removed from the cache.

A very naïve script which goes to the home screen and queries for the 'Phone' element 1000 times in a row, will cause memory consumption (as measured by Xcode) of the WebDriverAgent to grow to approximately 126 MB. Net, it appears that every `XCUIElement` object consumes about 100KB of memory.

For sessions which persist for a long period of time (say, 30 minutes or more), this can eventually lead to an out of memory situation in which the WebDriverAgent crashes.

This PR tries to provide an 'escape hatch' for applications which want long-running sessions, by allowing them to:
- query the amount of elements in the cache
- clear the element cache
by adding two custom routes.

We're looking at other approaches to keep the cache size under control as well - e.g. by checking the element cache for duplicates and adding an option to remove 'stale' elements (elements which no longer exist); we'll try to submit PRs for that as well.

As usual, let me know what you think & happy to discuss further.
Closes facebookarchive#896

Differential Revision: D8254682

Pulled By: marekcirkos

fbshipit-source-id: a200a570d9b4e71a6f2171419b39126dd9affca3

* Fix fb_framelessFuzzyMatchesElement method

Summary:
In XCUIElementTypeTable,attribute values of many XCUIElementTypeCell
instances are null.When fb_scrollToVisibleWithNormalizedScrollDistance
function is called by XCUIElementTypeCell instance,it is unable to
accurately slide the specified UI element.
Closes facebookarchive#879

Differential Revision: D8255752

Pulled By: marekcirkos

fbshipit-source-id: 930f478417823ae0c0f79d1699d61af616f3c781

* Tune nullability

* comment out flaky tests

* Optimize force touch endpoint
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants