calabash / calabash-ios

Calabash for iOS
Other
1.81k stars 369 forks source link

Query visibility broken in 0.9.146 (compared to 0.9.144) #152

Closed mgrebenets closed 11 years ago

mgrebenets commented 11 years ago

Hi.

So I have the app I'm testing. The app is using open source ECSlidingViewController controller to implement sliding sidebar.

The latest 0.9.146 update breaks visibility for the app when the sidebar is opened. I first noticed it in 0.9.145.pre2. I can see all the elements, inspect them with Accessibility Inspector, touch with the mouse, but can't query them. The query thinks they are not visible, so it basically breaks all the Cucumber tests.

However, no visibility issues occur while using 0.9.144 version.

So I came up with a very simple sample iOS app using ECSlidingViewController. The app has same visibility issues that I see in its "big brother". Here's the project on GitHub that helps to reproduce the problem.

Just a note, I'm not even sure the sliding view controller is a cause, I used it since the original app uses it and has visibility issues.

jmoody commented 11 years ago

in your sample project, can you annotate the screenshots with what view you are expecting to be visible?

mgrebenets commented 11 years ago

I'll do more when get back to laptop, but an obvious thing is Toggle button (UIBarButtonItem) on the toolbar. It has "Toggle" label. I can clearly click it and inspect with accessibility inspector in simulator. I can query this button with label "view" in 144 and get nothing in 146.

jmoody commented 11 years ago

my guess is that this behavior is caused by this change: subviews of the main window should mask the views on the topmost controller which i requested.

i cloned your project and added accessibilityIdentifiers to views of the two controllers:

# with the slide view open
> flash("view marked:'content view'") =>
nil

> flash("all view marked:'content view'") =>
[
    [0] "<UIView: 0x7537140; frame = (0 0; 768 960);  ...snip...>"
]

flash("all view marked:'content view' descendant label" )
[
    [0] "<UIButtonLabel: 0x7533ed0; ...snip...>",
    [1] "<UILabel: 0x7536fd0; frame = (65 208; 639 156); text = 'kadsljf' ...snip...>"
]

then i colored the backgrounds of the view controller views:

doing this clearly shows that the sliding view controller view is applied over the NLCCContentViewController view.

the solution is to use the all option/directive in (some) of your queries or maybe present the sliding view under the other view (no idea how ECSlidingViewController works).

be aware that the all option/directive will cause query to return results that include hidden views or views with alpha == 0.0

mgrebenets commented 11 years ago

OK, I understand the sliding view issue, I suspected there's something odd with this open source component, it hasn't been updated for quite a while and our devs have also hacked it a bit. I'll talk to devs on this one, think they'll want to look into it and fix. Until then I'll stick with 0.9.144 since it works fine for the tests we have so far, and will look into the use of all for queries dealing with "hidden" elements.

Here's the thing I want to understand, which of the two has a bug, 0.9.144 or 0.9.146? Let's say the view covers another view, but then doesn't handle touches and gestures and delegates them all the way down the responsibility chain to the underlying view. Whatever the reason, third party component issue or just bad design, but that's the app I have to test. So technically, the "hidden" view below handles touches, gestures, is visible both to user and accessibility inspector. Shouldn't it then be visible to the native UI automation tools and to Calabash queries as well?

Should I expect future version to behave the way 0.9.144 did or is current behavior the right one?

jmoody commented 11 years ago

"Here's the thing I want to understand, which of the two has a bug, 0.9.144 or 0.9.146?"

both? neither? i am interested to hear what karl has to say, but at this point in its development, i feel like calabash has covered most of the common use cases and has work arounds for the edge cases. not a very satisfying answer, i know.

when i am confronted with a case like yours, i typically try to dig into the calabash sources (server and client) to figure out exactly why calabash is behaving like it is. also not very satisfying. i can empathize. i have been on your side of the equation many times.

krukow commented 11 years ago

Hi, I just got into this thread now.

On this issue it is important to understand that Calabash uses a heuristic to determine if a view is visible or not. This problem is actually harder than it might seem at first (there are many ways to make a view invisible)

Being a heuristic means that it works well in most common cases, but there will be cases where it fails. The heuristic can always be improved though, and I welcome improvements either as pull requests, or as suggestions like yours. (Thanks for the excellent work on creating a project to reproduce the issue - I wish more people would file bugs like you and @jmoody !).

I've not yet had the time to look at the details, but if what Joshua says is correct, then I already know why Calabash considers this view "invisible": if a view is overlaid on top of this view, Calabash will usually deem the view "invisible". This could probably be improved by taking the alpha into consideration also.

For reference, the future version will be like 0.9.146. The reason is that you have the power of disabling the view-visibility filter using the "all" construct that Joshua mentioned (i.e., you have an easy workaround for this case).

I'll keep this issue open until we can analyze it further. Comments and questions welcome.

mgrebenets commented 11 years ago

I got something new. I don't know all the logic behind ECSlidingViewController, but one thing it does is putting so called topViewSnaphot on top of content view (right side) when the sidebar is opened. Don't know what's the rationale for that, but this snapshot is the same size as the screen and sits on top of the view hierarchy. However, it covers only content on the right side.

all the console output is for 0.9.146

query("all view").last
{
          "class" => "UIView",
           "rect" => {
        "center_x" => 658,
               "y" => 20,
           "width" => 768,
               "x" => 274,
        "center_y" => 522,
          "height" => 1004
    },
          "frame" => {
             "y" => 0,
         "width" => 768,
             "x" => 0,
        "height" => 1004
    },
         "UIType" => "UIView",
    "description" => "<UIView: 0x991c660; frame = (0 0; 768 1004); autoresize = LM+W+RM+TM+H+BM; gestureRecognizers = <NSArray: 0x991c800>; layer = <CALayer: 0x991c7d0>>"
}

This snapshot view is a UIView but it's contents is another view snapshot

topViewSnapshot.layer.contents = (id)[UIImage imageWithUIView:self.topView].CGImage;

And it's alpha is 1.0

query("all view", :alpha).last
1

0.9.144 can "see through" this snapshot, while 0.9.146 is "blinded" by it

label "view"
[
    [0] "slider view",
    [1] "panView",
    [2] "topViewSnapshot"
]

Of course forcing all offers a workaround

label "all view"
[
    [ 0] "slider view",
    [ 1] "panView",
    [ 2] nil,
    [ 3] nil,
    [ 4] "content view",
    [ 5] "toolbar",
    [ 6] nil,
    [ 7] nil,
    [ 8] "Toggle",
    [ 9] "Toggle",
    [10] nil,
    [11] "Toggle",
    [12] "kadsljfkadsjfkasd;fajs;fj\ndsa;kfjasd;fjads;j\nkldsfj;asdjf;adskjfas;jfd;asjf\nds;klafj;asdfj;adsjfas;f",
    [13] nil,
    [14] nil,
    [15] nil,
    [16] "Root View Controller",
    [17] nil,
    [18] "topViewSnapshot"
]

While this open source slider view might be not the best example of iOS coding (I don't get the snapshot thing completely so far), I've seen similar tricks in other apps and open source components as well.

krukow commented 11 years ago

Yes, I think you're right that it is not that uncommon. I'd welcome any suggestion on how to improve the heuristic.

krukow commented 11 years ago

Closing issue - visibility detection is a heuristic and the is a workaround using 'all'