Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix android 14 crash with implicit intent #208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

devtyty
Copy link

@devtyty devtyty commented Jan 17, 2024

No description provided.

@contactsimonwilson
Copy link

Thanks for this. I have some users reporting issues on Android 14 and will use this with patch-package to get by for now

@sssajjad007
Copy link

its work for me

@GaelCO
Copy link

GaelCO commented Feb 6, 2024

Hi,
I was making the same modifications on my end when I saw your PR.
I agree with the changes on the RNBackgroundActionsTask.java class. But for this, I think compileSDKVersion need to pass at least to 34 to assure have UPSIDE_DOWN_CAKE constant.

For the foreground type, an action is not always of type shortService. I try to found a way to set a dynamically foregroundService

@contactsimonwilson
Copy link

Hi, I was making the same modifications on my end when I saw your PR. I agree with the changes on the RNBackgroundActionsTask.java class. But for this, I think compileSDKVersion need to pass at least to 34 to assure have UPSIDE_DOWN_CAKE constant.

For the foreground type, an action is not always of type shortService. I try to found a way to set a dynamically foregroundService

Agreed with the service type. I'm using specialUse for my app with this patch. If using expo, this could easily be dynamic with a config plugin

@GaelCO
Copy link

GaelCO commented Feb 6, 2024

The foreground type could be pass by parameter since API Level 29.
I'm going to try a solution with adding type(s) in options.
/!\ Some types need permission on manifest. I consider that they should not be added to the library and that it is the responsibility of each application.

@devtyty
Copy link
Author

devtyty commented Feb 6, 2024

The foreground type could be pass by parameter since API Level 29. I'm going to try a solution with adding type(s) in options. /!\ Some types need permission on manifest. I consider that they should not be added to the library and that it is the responsibility of each application.

Right! Thanks for your feedback, i think we should set compileSDKVersion to 34 for library and foreground type to be is the responsibility of each application.

Maybe i will update this patch

@GaelCO
Copy link

GaelCO commented Feb 7, 2024

After some test and read the document, it seems to be impossible to set the foregroundType directly in the startForeground method without declare it in the service tag (manifest). If we had all the possible tags, we must declare all the corresponding permissions. Imho, It's not an option
This parameter is just useful if a service could have different type and precise when it is called.

I don't see any other solution than indicating in the documentation that the service must be redeclared in the application manifest with the correct foregroundType.

@devtyty
Copy link
Author

devtyty commented Feb 19, 2024

After some test and read the document, it seems to be impossible de set the foregroundType directly in the startForeground method without declare it in the service tag (manifest). If we had all the possible tags, we must declare all the corresponding permissions. Imho, It's not an option This parameter is just useful if a service could have different type and precise when it is called.

I don't see any other solution than indicating in the documentation that the service must be redeclared in the application manifest with the correct foregroundType.

I updated compileSdk to 34 and remove foregroundSeviceType in lib's manifest.
We should to declare in self application with permission related.

Refer

To startForeground with dynamic type, i think lib must be to chang with param is permission type corresponding

@MarkNaArea
Copy link

Worked for me!

@marcosinigaglia
Copy link

Hi, is there any news with respect to this PR? Need help with anything?

@rolandvar
Copy link

Worked for me too!

@ansarikhurshid786
Copy link

i am going to use it now. seem working for you guys

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.

None yet

8 participants