Add visionOS support to IdentityCore iOS target #1365
Conversation
Merged dev to branch
merge dev to branch
…/visionOS_support merge iOS17 test changes from another branch
…port merge xcode 15.2 branch to confirm pipelines
merge dev to branch
merge dev to branch
merge dev to branch
…upport merge pipelinefix branch to main visionOS branch
merge dev to branch
![azure-pipelines[bot]](https://avatars.githubusercontent.com/in/9426?s=60&v=4){kind=small}
| @@ -8820,6 +8820,7 @@ | |||
| ENABLE_BITCODE = NO; | |||
There was a problem hiding this comment.
This pull request does not update changelog.txt.
Please consider if this change would be noticeable to a partner or user and either update changelog.txt or resolve this conversation.
merge dev to branch
merge dev to branch
| @@ -237,10 +243,11 @@ - (void)dismiss | |||
| return safariController; | |||
| } | |||
| #else | |||
| MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Couldn't create session on macOS. Safari allowed flag %d", safariAllowed); | |||
| MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Couldn't create session on macOS or visionOS. Safari allowed flag %d", safariAllowed); | |||
There was a problem hiding this comment.
if target OS is Vision, we are returning nil at line 230, so is this log change correct ?
There was a problem hiding this comment.
That's true, I added the visionOS logic above later. I'll add a log line up there too
| @@ -92,7 +92,9 @@ + (BOOL)completeCertAuthChallenge:(NSURL *)endUrl | |||
|
|
|||
| + (void)resetHandler | |||
| { | |||
| #if TARGET_OS_IPHONE && !MSID_EXCLUDE_SYSTEMWV | |||
There was a problem hiding this comment.
why we added this extra check for system webview here ?
There was a problem hiding this comment.
This is a suggested change from Teams based on testing with the system webview disabled. I can see that all other instances of s_certAuthInProgress being updated are wrapped in this macro
| return [self systemWebviewFromConfiguration:configuration | ||
| useAuthenticationSession:useSession | ||
| allowSafariViewController:allowSafariViewController | ||
| context:context]; | ||
| #else |
There was a problem hiding this comment.
are we returning nil only if it is system webview for all targets ?
There was a problem hiding this comment.
Yes, any partner regardless of platform may choose to avoid system webview entirely. These !MSID_EXCLUDE_SYSTEMWV changes are added in a few places where setting it was not working. Other changes that add #if defined TARGET_OS_VISION && TARGET_OS_VISION are meant to specifically remove SafariViewController because it is completely removed on visionOS
In this specific case the [systemWebviewFromConfiguration] method itself is wrapped in #if !MSID_EXCLUDE_SYSTEMWV, but the call to it here was not. It's safer to wrap the call itself as well so we don't call a non-existent method
Proposed changes
This PR adds visionOS support to the IdentityCore iOS target, so that IdentityCore iOS compiles on iOS, iPadOS, and visionOS. It also adds code changes needed to compile on visionOS. The biggest change is that visionOS no longer supports SafariViewController, so we need to ensure that all references to it are behind OS macros in order for the target to build.
Type of change
Risk
Additional information