appium / WebDriverAgent

A WebDriver server for iOS and tvOS
Other
1.18k stars 370 forks source link

Proposing a definitive solution for the visibility attribute. #308

Open JonGabilondoAngulo opened 4 years ago

JonGabilondoAngulo commented 4 years ago

Hi,

Again I'm in trouble because an element has visibility false, when it should be true. I'd like to propose something to reduce the recurring problem of this 'simple' visible attribute.

Visibility is terribly complicated if we follow the WebDriver specification, which associated visibility to "displayedness". It turns this subject into "user visual perception", therefore we get into trouble.

We are trying to figure out if an element is visible and if it is hittable, using hundreds of lines of code, and relying on un-existing contracts between XCUITest and programmers, i.e. NO API. (I'm talking about - (BOOL)fb_isVisible).

In this case the visibility is wrong because additional attribute FB_XCAXAIsVisibleAttribute is false.

NSNumber *isVisible = self.additionalAttributes[FB_XCAXAIsVisibleAttribute]; if (isVisible != nil) { return isVisible.boolValue; }

This is private API, XCUITest doesn't care about that value.

If I disable the use of FB_XCAXAIsVisibleAttribute, It still returns visibility false because the accessibility element at the element mid point is not recognised.

XCAccessibilityElement *hitElement = [FBActiveAppDetectionPoint axElementWithPoint:midPoint];

Again, not happy with area clipping we want to rely on hit tests.

I think it is a pity so much effort (including YYCache) for such critical mistakes that can ruin tests, hours and hours of QA engineers. I think we should leave "displayedness" behind, we will never get it right.

I propose to simplify it by using XCElementSnapshot visibleFrame clipped to screen bounds. visibleFrame is as well private API, so XCUITest doesn't have a contract for correctness, but let's start from there.

mykola-mokhnach commented 4 years ago

I propose to simplify it by using XCElementSnapshot visibleFrame clipped to screen bounds. visibleFrame is as well private API, so XCUITest doesn't have a contract for correctness, but let's start from there.

Thanks for the proposal. We also has this idea. Unfortunately I can provide at least two examples where such approach does not work - out-of-screen table cells and remote/IPC views like Share dialog or web views

JonGabilondoAngulo commented 4 years ago

Glad to hear the idea has already been explored. I anticipated unreliable geometry informations.

About the table cells out of screen. From my experience the cells do show coordinates outside of the screen, but some of their children they have coordinates relative to the cell what could be mistaken as coordinates within the screen. Current clipping code in fb_isVisible already solves those children as invisible because the parent cell is out of screen.

I have modified fb_isVisible that tells "is element in within the screen". See example of Files App Table view with cell out of screen:

https://gist.github.com/JonGabilondoAngulo/07ccfc6d551debbee1cfc70a4b2aa20b#file-screen-shot-2020-04-02-at-21-50-16-png

This is the code for the visibility:

It requires a fix to mark as invisible those Windows without content that are so common. Besides that, the results are very good. The attribute it represents is not "isVisible" as we know it but "isOnScreen".

What is different ? Cells Yellow and Blue (under tab bar) are "true" for onScreen, which are "false" for isVisible.

Still why do I prefer this results ?

  1. I'm not using private unreliable API for visibility or hit test accessibility element.
  2. If I need to know if the cell is visible to the eye I know I have to check if it is under Tab bar (or not. Maybe I have a tabbar with opacity 0.3 that lets the taps go through...)
  3. If I'm searching by X/Y, I will arrive to TabBar and will get visual preference over the cell (if I want to).

I'm familiar with XCP/IPC UI. I've been experimenting with "Stocks" and the web view it shows. I'll be glad if you could point me, if you want, to some cases that in your opinion make this approach not viable.

Thanks.

mykola-mokhnach commented 4 years ago

I was talking about the dialog from https://github.com/appium/appium/issues/11324

I'm glad that the solution you've found works for you. Although, I don't think we could replace the mainstream one with it - there are too many corner cases :(

JonGabilondoAngulo commented 4 years ago

I saw appium/appium#11324 It seems they were giving relative coordinates in that iOS 11. iOS 13 does not have that problem. There are many geometry inaccuracies in XCUITest I'm afraid. They could cause visibility, location, tap errors ...

This case actually helps my point. Why visibility attribute is calculated wrong. If it is due to to a bad 'rect' is due to a defect in API from XCUITest that can be filed to Apple. If the defect is due to us using private API trying to achieve displayedness then there is none to blame but us.

XCUITest doesn't give visibility (it does give hittable and exists), I'm not sure they want to provide it. Although with hittable they get into the same troubles, and indeed they fail in some cases. Hittable is wrong sometimes.

Displayedness is a very "natural" aspiration, everybody would agree to it, until we understand its complexity, then anyone would agree to avoid it. It depends on opacities, visualisation flags, event propagations, masks, ghosts ..

isVisible in software is different than displayedness, which is a visual perception issue. It takes visibility into an unreliable API. And this is something to think deeply. An API that has to be labeled for the sake of the users, as unreliable or inaccurate is a serious matter. I don't recall to get across an API that was labeled unreliable. I understand approximation, inaccurate in numbers, but approximation in boolean is just nonsense.

I think visibility has to be given a thought, or warned to users, or abolished, or an alternative attribute that is achievable reliable, and when it fails is because a clear reason that can be fixed.

As I write I realise that this goes beyond an "issue" for this list to a fundamental analysis of "visibility", that should go higher into Appium or even WebDriver.

Anyway, I think it could as well stay within our XCUITest efforts without "scalating". We could turn visibility into something that is reliable in my opinion.

nadavva commented 4 years ago

Interesting reflection, I have many problems with visibility as well, I’d like to find something more reliable

nanoscopic commented 2 years ago

To start, I agree that what WDA is doing in regard to this ( and many other things... ) is very complicated and buggy. There is too much reliance on private APIs that do not return consistent results and are slowly being phased out or completely broken as Apple makes updates.

The closest method I see for visibility test is to use the latest calls to figure out the coordinates of an element, and then to make the call to determine the element at that point. If the same element doesn't come back, then it isn't visible.

Note that the element returned by elementAtPoint is not a XCUIElement... It can still be reliably used though. The headers are also out of date within WDA which makes everything much more confusing.

If you use the latest headers and latest Xcode, you can consistently and accurately fetch the coordinates of elements on screen. I don't see how this could be readily fixed within WDA though as it has too many years of layered changes and hacky attempts to fix things, such as the mentioned YYCache.

Avoid caching old elements. It is terrible and just leads to bugs. You can make a single quick call to get a snapshot of all elements on screen, and that should be used generally... What WDA does weirdly is make many calls repeatedly instead of just using a single snapshot and navigating the Snapshot...

One of the first things I did to WDA myself was to completely delete YYCache as it introduces more bugs then it solves.