Skip to content

chore: [SDK-5307] remove stale compatibility code for iOS below 15 - #1756

Open
nan-li wants to merge 2 commits into
nan/sdk-5346from
nan/sdk-5307
Open

nan-li wants to merge 2 commits into
nan/sdk-5346from
nan/sdk-5307

Conversation

@nan-li

@nan-li nan-li commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Remove the runtime guards and reflection that only existed for iOS 12 through 14, and for iOS 15 on Mac Catalyst. SDK-5307.

Details

Motivation

#1749 raised the deployment target to iOS 15 and #1757 raises Mac Catalyst to match, so every @available(iOS 13.0, *), @available(iOS 14.0, *) and @available(iOS 15.0, *) check is always true, and the performSelector and NSInvocation calls that avoided linking newer APIs at compile time have no reason left to exist.

Scope

Stacked on #1757. The first commit needs only the iOS floor; the second needs the Catalyst floor too.

  • The two Objective-C lifecycle observers and OSRemoteLoggingController register UIScene observers with the UIScene* notification constants instead of string names.
  • OSBundleUtils.isAppUsingUIScene and the ephemeral authorization check lose their guards and else branches.
  • UIApplication+OneSignal and OSMessagingController read the key window's scene through the typed UIWindowScene API. A nil key window still reads as active, as it did through performSelector.
  • The attachment downloader calls NSFileHandle writeData:error: directly and drops the pre-13 writeData: fallback.
  • createActionForButton: builds UNNotificationActionIcon and calls actionWithIdentifier:title:options:icon: directly, and drops the pre-15 fallback that built the action without an icon. A button without an icon key still passes a nil icon.
  • OneSignalCoreMocks and the dev app lose the matching #available checks, and the dev app's willPresentNotification always uses the banner and list options.
  • Not in this PR: the keyWindow reads, the Xcode-era AUTH_STATUS_EPHEMERAL and provisional defines, and the commented-out blocks in the legacy UnitTests target.

Testing

Unit testing

No new tests. The removed branches were unreachable at the iOS 15 floor, and the existing lifecycle, in-app messaging and notification settings tests cover the paths that remain.

Manual testing

Reduced test plan run serially on an iPhone 17 Pro Max simulator with Xcode 27.0: 353 tests, 0 failures. build-for-testing produces the same warning set as main. With #1757 underneath, OneSignalFramework builds for Mac Catalyst on Xcode 27.0 at the ios15.0-macabi target. The dev app builds against the resulting frameworks. Not run on a device.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

@nan-li
nan-li changed the base branch from main to nan/sdk-5346 September 24, 2026 16:27
The deployment target is iOS 15 since #1749, so every
`@available(iOS 13.0, *)` and `@available(iOS 14.0, *)` check is always
true, and the reflection that avoided linking iOS 13 APIs at compile
time is no longer needed.

- Register the UIScene lifecycle observers with the UIScene notification
  constants instead of string names, in the lifecycle observer, the
  notifications manager and the remote logging controller.
- Drop the iOS 13 guard in `OSBundleUtils.isAppUsingUIScene` and the
  iOS 14 guard around the ephemeral authorization status.
- Read the key window's scene through the typed `UIWindowScene` API in
  `UIApplication+OneSignal` and `OSMessagingController` instead of
  `performSelector`.
- Call `NSFileHandle writeData:error:` directly in the attachment
  downloader instead of through `NSInvocation`, and drop the pre-13
  `writeData:` fallback.
- Remove the matching `#available` checks in OneSignalCoreMocks and the
  iOS 13 and 14 guards in the dev app.
With the Mac Catalyst floor at 15 alongside iOS, the `@available(iOS
15.0, *)` check around notification action buttons is always true, so
`createActionForButton:` can call `UNNotificationActionIcon` and
`actionWithIdentifier:title:options:icon:` directly instead of going
through `NSClassFromString`, `performSelector` and `NSInvocation`. The
pre-15 fallback that built the action without an icon goes with it. A
button without an icon key still passes a nil icon, as before.
@nan-li nan-li changed the title chore: [SDK-5307] remove iOS 12 through 14 runtime guards and reflection chore: [SDK-5307] remove compatibility code for iOS 12 through 14 Sep 24, 2026
@nan-li
nan-li marked this pull request as ready for review September 24, 2026 16:31
@nan-li nan-li changed the title chore: [SDK-5307] remove compatibility code for iOS 12 through 14 chore: [SDK-5307] remove stale compatibility code for iOS below 15 Sep 24, 2026
@nan-li
nan-li requested a review from a team September 25, 2026 16:59

@fadi-george fadi-george left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Potential issues:

  • writeData:error: returns a BOOL, might be cleaner to check that instead of fileHandleError
  • OSRemoteLoggingControllerTests still posts "UISceneDidEnterBackgroundNotification" by string, could use UIScene.didEnterBackgroundNotification to match
  • NSClassFromString for UNTextInputNotificationResponse / UNPushNotificationTrigger in UNUserNotificationCenter+OneSignalNotifications.m could go too if you want the cleanup to be complete

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.

2 participants