braze-inc / braze-swift-sdk

Braze SDK for the Apple ecosystem, including: iOS, macOS, iPadOS, visionOS, tvOS
https://www.braze.com
Other
50 stars 19 forks source link

[Bug]: Fail to present InAppMessage with multiple UIWindows #82

Closed aclima93 closed 10 months ago

aclima93 commented 11 months ago

Platform

iOS

Platform Version

iOS 17.0

Braze SDK Version

6.6.1

Xcode Version

15.0

Computer Processor

Apple (M1)

Repro Rate

100% of the time

Steps To Reproduce

Have an app with two or more windows for the same scene, all windows set to UIWindow.Level.normal. Assume the currently presented window is not the first entry for UIWindowScene.windows

Expected Behavior

Attempting to display an InAppMessage will result in it being presented in the currently active key window.

Actual Incorrect Behavior

Attempting to display an InAppMessage will result in "Unable to present message - unable to find the app root view controller." and fail to present the IAM in the currently active key window.

Verbose Logs

No response

Additional Information

Hypothesis

Internally the following chain of events seems to be the cause of this behavior:

Proposal

Changing the activeWindow filtering logic to take into account isKeyWindow

+ (UIWindow *)selectApplicationWindow:(NSArray<UIWindow *> *)windows {
  // Dynamically gets ABKInAppMessageWindow class as it is part of AppboyUI
  Class ABKInAppMessageWindow = NSClassFromString(@"ABKInAppMessageWindow");

  // Holds all windows excluding any `ABKInAppMessageWindow`
  NSMutableArray<UIWindow *> *filteredWindows = [NSMutableArray array];

  // Holds all `UIWindowLevelNormal` windows excluding any `ABKInAppMessageWindow`
  NSMutableArray<UIWindow *> *normalLevelWindows = [NSMutableArray array];

  for (UIWindow *window in windows) {
    // Ignores ABKInAppMessageWindow
    if (ABKInAppMessageWindow && [window isKindOfClass:[ABKInAppMessageWindow class]]) {
      continue;
    }
    // Assumes that the application window has a windowLevel set to
    // UIWindowLevelNormal and is the `keyWindow`
    if (window.windowLevel == UIWindowLevelNormal) { 
      if (window.isKeyWindow) {
        return window;
      }
      [normalLevelWindows addObject:window];
    }

    [filteredWindows addObject:window];
  }

  // Fallback to the first UIWindowLevelNormal window in hierarchy, 
  // or even the first window in hierarchy
  return normalLevelWindows.firstObject ?? filteredWindows.firstObject;
}

would probably fix it, while also making sure that in the event that no keyWindow is found, it will still fallback to the first window with UIWindowLevelNormal. (apologies in advance, my ObjC is rusty, so there's probably room for improvement)

hokstuff commented 11 months ago

Hi @aclima93,

Thanks for filing a detailed report around this issue! We will investigate this issue internally to try to repro the behavior and will keep you updated.

For some insight into the logic, the method Braze.UIUtils.activeRootViewController gets the rootViewController of the object returned from snippet below:

    static var _activeWindow: () -> UIWindow? = {
      {
        let windows: [UIWindow]
        if #available(iOS 13.0, tvOS 13.0, *) {
          windows = activeWindowScene?.windows ?? UIApplication.shared.windows
        } else {
          windows = UIApplication.shared.windows
        }

        return
          windows
          .filter { $0.windowLevel == .normal }
          .filter { !($0 is BrazeInAppMessageWindowType) }
          .first
      }
    }()

However, it appears that this snippet is either returning nil or returning a UIWindow with a nil rootViewController. Let us know if there's anything that jumps out at you based on your integration.

Thank you!

aclima93 commented 11 months ago

That snippet seems to reflect the ObjC code that I can inspect in this repository quite well, so the problem should be the same: if there are multiple windows in the current window scene and the window marked with isKeyWindow isn't the first one on the activeWindowScene?.windows array, Braze.UIUtils will return the wrong window and the IAM will get displayed in the wrong window (or not at all, if that window doesn't have a rootViewController set-up).

PS: could you share where i might find that swift snippet within the SDK? ABKUIUtils is the closest thing i've found in this repository.

hokstuff commented 11 months ago

I see, that makes sense. We will try to incorporate the isKeyWindow call to the filter chained commands in the snippet above within _activeWindow() and verify that it functions as expected when grabbing the rootViewController.

PS: could you share where i might find that swift snippet within the SDK? ABKUIUtils is the closest thing i've found in this repository.

We actually don't open-source all of our source files in our codebase, and the Swift snippet above is not publicly accessible (it is compiled into the binaries that we ship). For the file ABKUIUtils, that is a part of our BrazeUICompat package that is solely used as a tool for the migration from our legacy Appboy_iOS_SDK to the Swift SDK

aclima93 commented 11 months ago

@hokstuff Thank you!

aclima93 commented 11 months ago

@hokstuff would you be able to provide an estimate of when this fix might be released?

lowip commented 11 months ago

Hi @aclima93,

This fix is slated for our next release which is planned for the coming days or early next week. We'll update this thread once it's available.

Best,

jerielng commented 10 months ago

Hey @aclima93, we've released 7.2.0, which should resolve this issue now. Thank you! I'll go ahead and close this issue but feel free to reach back out if you have further questions.