Skip to content

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

Merged
nan-li merged 4 commits into
mainfrom
nan/sdk-5307
Sep 28, 2026
Merged

nan-li merged 4 commits into
mainfrom
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 raised 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

  • 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, branches on its BOOL result, 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.
  • The notification center category references UNTextInputNotificationResponse and UNPushNotificationTrigger directly and reads userText through the typed class.
  • 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, the setInspectable: invocation in OneSignalWebView and OSInAppMessageView (Xcode-era reflection under a 16.4 guard that stays), the literal com.apple.UNNotification*ActionIdentifier strings, and the commented-out blocks in the legacy UnitTests target.

Testing

Unit testing

New OneSignalAttachmentHandlerTests calls createActionForButton: with a system icon, a template icon and no icon key, and checks the identifier, title, foreground option and whether icon is set. The old testCreateActionForButtonsWithIcon is commented out with the rest of UnitTests.m, so this path had no live test.

Coverage of the other edits is narrower than it looks. No test host declares UIApplicationSceneManifest, so the existing lifecycle tests run the UIApplication branches only; the scene branches in the two Objective-C observers and in applicationIsActive do not execute under test. OSRemoteLoggingControllerTests injects scene mode and covers the remote logging controller's scene branch, now posting through the UIScene constant. The notification settings tests cover the ephemeral change.

Manual testing

Reduced test plan run serially on an iPhone 17 Pro Max simulator with Xcode 27.0: 356 tests, 0 failures. build-for-testing produces the same warning set as main. OneSignalFramework and OneSignalInAppMessages both build for Mac Catalyst on Xcode 27.0 at the ios15.0-macabi target, which between them compile every touched source file. 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
@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

Base automatically changed from nan/sdk-5346 to main September 28, 2026 16:19
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.
…otifications class lookups

Review follow-ups from #1756.

- The attachment downloader branches on the BOOL that
  `writeData:error:` returns instead of testing the error object,
  which is only defined when the call returns NO.
- `OSRemoteLoggingControllerTests` posts the scene background
  notification through the `UIScene` constant, matching the controller.
- The notification center category references
  `UNTextInputNotificationResponse` and `UNPushNotificationTrigger`
  directly and reads `userText` through the typed class instead of
  `NSClassFromString` and key-value coding.
`OneSignalAttachmentHandlerTests` calls `createActionForButton:` with a
system icon, a template icon and no icon key, and checks the identifier,
title, foreground option and whether `icon` is set. The method had no
live test since the old `testCreateActionForButtonsWithIcon` was
commented out with the rest of UnitTests.m.
@nan-li

nan-li commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

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

Thanks. All three are in 6318189

Also rebased onto main after #1757 and added OneSignalAttachmentHandlerTests for the icon paths. Verified on a device with an image attachment and two icon buttons.

@nan-li
nan-li merged commit 513c0ae into main Sep 28, 2026
11 of 14 checks passed
@nan-li
nan-li deleted the nan/sdk-5307 branch September 28, 2026 18:54
nan-li added a commit that referenced this pull request Oct 1, 2026
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