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

Get rid of SYSTEM_ALERT_WINDOW permission in Android manifest #3574

Closed
gnprice opened this issue Jul 26, 2019 · 0 comments
Closed

Get rid of SYSTEM_ALERT_WINDOW permission in Android manifest #3574

gnprice opened this issue Jul 26, 2019 · 0 comments
Labels
a-Android upstream: RN Issues related to an issue in React Native

Comments

@gnprice
Copy link
Member

gnprice commented Jul 26, 2019

Splitting from #3075:

  • Test an install flow for any scary mentions of permissions. E.g., our manifest says we use the SYSTEM_ALERT_WINDOW permission.
    ...
  • Remove the SYSTEM_ALERT_WINDOW permission from the output manifest in release builds; it's only needed in debug builds, for displaying the RN debug menu.

In fact, we've had logic to supposedly do exactly that since
2cc6a52 in 2017; with a fix in b5ac051 of 2018, and a refactor in a0f33ec of this week.

But a quick look at the actual manifest of 25.8.122, the latest beta, finds it's right there.

In fact... it looks like it was pulled in by React Native itself! Dates to the very beginning, reported as facebook/react-native#5886, and fixed as facebook/react-native#23504.

Which went into RN v0.59... which means it was fixed by the upgrade, #3399 . And, indeed, it's gone when I do a build now.

The issue was pretty confusing because RN redundantly had the permission in its own manifest fragment that gets merged into the output, and in the manifest in the app template. So anything we did to make it conditional in our own source manifest had no effect, and could only have had an effect if we'd used tools:remove -- because it never needed to be there at all, until just recently when it was removed (by that fix PR) from RN's own manifest fragment.

@gnprice gnprice added a-Android upstream: RN Issues related to an issue in React Native labels Jul 26, 2019
@gnprice gnprice closed this as completed Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-Android upstream: RN Issues related to an issue in React Native
Projects
None yet
Development

No branches or pull requests

1 participant