[Bug] iOS - requestPermission fallbackToSettings fix#1729
Merged
jennantilla merged 2 commits intoOneSignal:mainfrom Aug 15, 2024
Merged
[Bug] iOS - requestPermission fallbackToSettings fix#1729jennantilla merged 2 commits intoOneSignal:mainfrom
jennantilla merged 2 commits intoOneSignal:mainfrom
Conversation
3 tasks
Contributor
|
Hi @aharonphytech thank you for submitting this PR! |
jennantilla
approved these changes
Aug 15, 2024
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
One Line Summary
The PR fixes bug in iOS in
requestPermissionmethod where thefallbackToSettingsparameter has no effect.Details
Motivation
In the Objective-C code, the
fallbackToSettingsparameter is of typeBOOL. However, in JavaScript, boolean values (trueandfalse) are passed asNSNumberobjects when bridged to Objective-C. The__NSCFBooleantype seen in the lldb debugger is actually anNSNumberinstance representing a boolean value. This leads to a type mismatch, causing thefallbackToSettingsparameter to always be treated as truthy in the if statement (becauseNSNumberis a non-null object and will always be truthy, no matter which value it encapsulates), resulting in an unintended opening of theOpen Settingsprompt.Scope
This change only affects the
requestNotificationPermissionmethod in the iOS SDK. It does not alter the behavior of any other methods or functionalities.Other
I fixed the problem by changing how the
fallbackToSettingsparameter is handled in the Objective-C method. The issue arose because thefallbackToSettingsparameter was being passed from JavaScript as anNSNumber(which is how boolean values are bridged between JavaScript and Objective-C), but the method in Objective-C was expecting aBOOL.In Objective-C, a
BOOLis a primitive type, whereasNSNumberis an object that can encapsulate a boolean value. Due to this mismatch, the conditionif (fallbackToSettings)was always evaluating to true, because an NSNumber object is always non-null and thus truthy, regardless of whether it represents true or false.To fix this, I ensured that the value passed as
fallbackToSettingsis properly unboxed from theNSNumberobject to aBOOL. I did this by using[fallbackToSettings boolValue], which correctly interprets the NSNumber's encapsulated value as aBOOL. This way, the condition will properly evaluate to true or false based on the actual value passed from JavaScript.Testing
Unit testing
None. This change addresses a specific issue in the interaction between JavaScript and Objective-C, which is difficult to cover with unit tests. Manual testing on actual devices provided sufficient validation for this fix.
Manual testing
Affected code checklist
Checklist
Overview
Testing
Final pass
This change is