Remove Alert Handling in iOS

[ghost]

This patchset removes support for Appium auto-accepting or dismissing of alerts, specifically the useLocationServices alert on iOS.

[bootstraponline]

I think this test would pass if you only changed bootstrapSettings.useLocationServices to bootstrapSettings.autoAcceptAlerts instead of deleting the else.

[ghost]

If the else stays then the alert is dismissed with "Don't Allow" chosen.

[bootstraponline]

Ok. Makes sense.

[bootstraponline]

👍

[jlipps]

Thanks @Dandover. In general I'm happy with this approach.

However this will auto accept all alerts. What we want is something that will auto accept alerts that block execution of a script, which so far has just been location services. If you want to expand that list, fine... but this will auto-accept even random alerts that happen during test execution. Not sure what I think about that.

Also, I'd like to see this remain backwards compatible with the useLocationServices: true option. I.e., if someone has a test that sets that desired cap and true value, I'd like for it to continue to work for a while as we deprecate it.

[ghost]

OK sounds like it's off to a good start @jlipps but let's put a little more thought into it and make sure we get it right.

I'm not sure which app you had in mind, but location is just one of the many blocking alert types. In iOS 7 there are now OS alerts for access requests to each privacy category: microphone, bluetooth, photos, reminders, calendars, contacts, and location. So depending on your application code you will get alerts for any/all of these data categories if/when your app makes a request for that data and the alerts will each block test execution. I don't see the distinction between these alerts triggered by apps requesting access to private data vs. the alerts triggered by apps for any other purpose. The app does something, it or the OS pops up an alert, test handles it (or not). I don't think there is such a thing as a random alert.

So just to make sure, you propose we keep the default for Appium to deny location services to applications (click "Don't Allow") unless useLocationServices: true is passed as a cap, in which case Appium will allow location services (click "OK"). And then the way to keep Appium from interacting with location services (or any future alert) would be to add another cap that is autoAcceptAlerts: false?

Not sure what the plan for deprecation would be, but if useLocationServices sticks around, I'd propose we rename to allowLocationServices to match the implementation and be clear that true == allow and false == don't allow. But that would break backwards compat I suppose?

There have been several threads now on appium-discuss from folks who are trying to test their application alerts and getting tripped up by this: Roopesh, myself, Vic, Ling-Yi in the past few months, so I'm hoping we can come up with something that works long term.

Final thoughts?

[ghost]

@bootstraponline or @jlipps or @penguinho thoughts on defaults? assume we should doc this too...

[jlipps]

IIRC the whole point of having to do something special for location services was that it popped up before instruments really had control of the app. In other words, we couldn't use the normal alert handling functions.

When I introduced the 'useLocationServices' desired cap, I thought I was working around this limitation. In fact, I'm not sure I was. I think all it does is automatically say 'yes' to the location services dialog when it can interact with it. I'm not sure but I think it doesn't solve the case of the popup preventing instruments from automating in the first place (which was solved by delaying the popup by 8 seconds).

If that's correct, then my vote would just be to remove all handling whatsoever and let the client do it based on the app logic.

If on the other hand we can demonstrate that having some setting to automatically handle these alerts allows appium automation to continue when it couldn't otherwise (via the alert methods), then I say let's keep discussing this.

does that make sense to everyone?

[bootstraponline]

does that make sense to everyone?

yes

If that's correct, then my vote would just be to remove all handling whatsoever and let the client do it based on the app logic.

👍

[ghost]

Ahh, thanks for the back story. In that case my vote would also be to remove all the alert handling and let the test/client do it. I'll update this pull request.

[ghost]

OK, updated. This brings Appium more inline with UIAutomation alert handling on iOS.

[bootstraponline]

I think we should remove uiautoHandleAlerts entirely.

[ghost]

Just to make sure we're clear - you want to remove the config option totally and have Appium's alert handler tell UIAutomation that the client will always handle the alert (return true) in onAlert? You don't want to provide a way to let the client rely on UIAutomation's default alert handler (return false) in onAlert.

[bootstraponline]

I thought the idea is to have the client always handle the alert? /cc @jlipps

[jlipps]

Just to make sure we're clear - you want to remove the config option totally and have Appium's alert handler tell UIAutomation that the client will always handle the alert (return true) in onAlert? You don't want to provide a way to let the client rely on UIAutomation's default alert handler (return false) in onAlert.

Correct. We should always return true, so that UIAutomation always lets the client handle the alert.

[ghost]

OK, done.

[jlipps]

where do we call this? shouldn't we remove that too?

[ghost]

Good call. Done.

[bootstraponline]

The code looks good to me.