Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow safari alert with block/allow buttons to be handled #1185

Merged
merged 2 commits into from
Mar 24, 2020

Conversation

imurchie
Copy link
Contributor

@imurchie imurchie commented Mar 24, 2020

Add "Block"/"Allow" alerts to those recognized.
Simulator Screen Shot - appiumTest-C33F9E2A-D126-499E-B7EA-7EB124163596-iPhone X - 2020-03-24 at 09 08 28
Simulator Screen Shot - appiumTest-C33F9E2A-D126-499E-B7EA-7EB124163596-iPhone X - 2020-03-24 at 09 10 30
Simulator Screen Shot - appiumTest-C33F9E2A-D126-499E-B7EA-7EB124163596-iPhone X - 2020-03-24 at 09 09 49

This should fix appium/appium#8582 and appium/appium#14091

@@ -113,18 +113,21 @@ helpers.getAlert = async function getAlert () {
}

// determine that the name of the button is what is expected
Copy link
Contributor

Choose a reason for hiding this comment

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

why cannot we handle this logic inside WDA? https://github.com/appium/WebDriverAgent/blob/f23062fab0228f2835d4324b2a814aa1649d115a/WebDriverAgentLib/Categories/XCUIApplication%2BFBAlert.m#L16 is the method. Just let me know if there are any other conditions to detect alerts and I'll modify the stuff correspondingly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The WDA logic has always failed for Safari alerts. All the info is in this function, so if you want to translate it over, great!

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, I'll try to do that today. Would you assist me with testing?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, I'll need the page source output for the cases these alerts are visible, also on iPad

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can help with testing. There is also a test suite for Safari alerts within appium-xcuitest-driver (https://github.com/appium/appium-xcuitest-driver/blob/master/test/functional/web/safari-alerts-e2e-specs.js). A test for the popup is added in this PR.

Sources...
iPhone: https://gist.github.com/imurchie/421b17339a32e64f965e25c779fdd200
iPad: https://gist.github.com/imurchie/99043004e30f66ff2b1986bc808d0064

@imurchie imurchie merged commit 99b8cda into appium:master Mar 24, 2020
@imurchie imurchie deleted the isaac-popups branch March 24, 2020 16:19
khanayan123 pushed a commit to khanayan123/appium-xcuitest-driver that referenced this pull request May 10, 2021
…um#1185)

* feat: allow safari alert with block/allow buttons to be handled

* tests: go back to original page in windows tests
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.

Check for alerts during XCUITest-based webview/safari tests
2 participants