Skip to content

Replaced Element Tap Quiescence Check for iOS 13#278

Merged
mykola-mokhnach merged 4 commits intoappium:masterfrom
Dan-Maor:element_tap_quiescence
Feb 3, 2020
Merged

Replaced Element Tap Quiescence Check for iOS 13#278
mykola-mokhnach merged 4 commits intoappium:masterfrom
Dan-Maor:element_tap_quiescence

Conversation

@Dan-Maor
Copy link
Copy Markdown
Collaborator

@Dan-Maor Dan-Maor commented Feb 2, 2020

We've encountered a case in which using XCTest's native [XCUIElement tap] delays the command execution by 60 seconds compared to the custom implementation, which is not being used for iOS 13. After the 60 seconds delay a message stating "App animations complete notification not received, will attempt to continue" message is displayed in the device's log and the interaction succeeds.

Turns out that the function uses XCTest's quiescence validation internally (the same one being used for launching applications), and that it appears to not play nice with animations after interacting with elements.

From what I could gather, XCTest's animations check uses the XCAXClient method notifyWhenNoAnimationsAreActiveForApplicationinternally, which registers for a user testing notification titled "AnimationsNonActive", and then injects an accessibility event (2043) to the accessibility element of the application. This in turn would trigger the required user testing notification to be posted from the tested application.

It appears that for some reason attempting to check for animations after trying to find elements during an animation causes the application to not send the user testing notification required to finish the quiescence check, even if the check was initiated after the application has truly idled.

This PR disables the animation check before using [XCUIElement tap] and validates the readiness of the object to be tapped using fb_waitUntilFrameIsStable, like with the custom implementation.

It should be noted that when using Xcode 11, in some instances this approach is slightly slower than XCTest's implementation. I did not notice a speed discrepancy on Xcode 10.

@KazuCocoa
Copy link
Copy Markdown
Member

ah...
it is out of my curiosity.
Does this slowness happen when reduceMotion on, too?
(it is available in caps and settings API for only on a simulator. On a real device, we must enable the preference in settings > accessibility by hand)

@Dan-Maor
Copy link
Copy Markdown
Collaborator Author

Dan-Maor commented Feb 2, 2020

The issue is still happening when reduceMotion is enabled.

Copy link
Copy Markdown
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.

Thanks.
This change looked good to me

/**
Set the usage of animation check in quiescence validation (defaults to YES).
*/
+ (void)setEnableAnimationCheck:(BOOL)value;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What about like below naming?

  • setAnimationCheckEnabled: (BOOL)isEnabled
  • setEnableAnimationCheck: (BOOL)enable

@mykola-mokhnach
Copy link
Copy Markdown

mykola-mokhnach commented Feb 2, 2020

wow, thanks for the interesting investigation Mr @Dan-Maor
I saw similar reports at appium/appium#13503 (comment) and I'm afraid this problem is not only relevant for tap, but also for other similar XCUIElement primitives.

frame = newFrame;
return isSameFrame;
}];
return result;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is this change necessary?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Calling self.frame retrieves a snapshot of the element every time.

From my testing saving the result of the frame instead of calling self.frame every time the property is needed is slightly faster (5% faster in my simple stress scenario); I've experimented with ways to make the possible performance impact lower, also tested using fb_snapshotwithattributes with only the frame as a parameter, but it didn't make much of a difference from just calling self.frame.

Saving the result into a variable was just for printing execution times though, I'll remove it.

Comment thread WebDriverAgentLib/Categories/XCUIElement+FBTap.m
Copy link
Copy Markdown
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 with Mykola's comment

@mykola-mokhnach mykola-mokhnach merged commit aa93d66 into appium:master Feb 3, 2020
@Dan-Maor Dan-Maor deleted the element_tap_quiescence branch February 3, 2020 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants