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

set targetSdkVersion to 33 #2649

Merged
merged 15 commits into from
Oct 2, 2023
Merged

set targetSdkVersion to 33 #2649

merged 15 commits into from
Oct 2, 2023

Conversation

r10s
Copy link
Member

@r10s r10s commented Sep 6, 2023

this PR updates targetSdkVersion to 33, compiling and building works, however, this kind if updates usually also comes with behaviour changes (see https://developer.android.com/about/versions/13/behavior-changes-all ) that need to be checked carefully:

concrete things that do not work at a first glance:

  • attach audio or gallery (instead of permission request, the fallback "Permission required" info is shown)

  • the "quick image attach bar" is not shown empty as the permissions are granted, but reading photos fails

  • check remaining occurrences of READ_EXTERNAL_STORAGE and WRITE_EXTERNAL_STORAGE: EDIT: done, we keep the permissions in the code, however bypass the check on api33

  • check if writing files (backups, keys, export images, ...) work EDIT: log: OK , key: OK , backup: OK , export attachment (imasge): OK

  • "Privacy": "Runtime permission for notifications": "Android 13 (API level 33) introduces a runtime notification permission: POST_NOTIFICATIONS. This change helps users focus on the notifications that are most important to them."
    also see below

  • add a device message if the user taps "deny notifications"

  • handle "denied notifications" in the system settings

general aspects (unticked items need to be checked, please add a summary-IMPACT next to the items then):

closes #2599

@github-actions
Copy link

github-actions bot commented Sep 6, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@adbenitez
Copy link
Member

I am trying this, for me I had notifications disabled for $reasons (to avoid annoying notifications from my dev/test DC installation) after upgrading I could not enable notifications, in system settings the toggle to enable/disable notifications was disabled, after reverting to target 32 when opening the app I see the popup dialog requesting notifications permissions, maybe something changed there and the way the app is requesting notifications permissions is not compatible with target 33

@r10s r10s marked this pull request as draft October 1, 2023 13:28
@github-actions
Copy link

github-actions bot commented Oct 1, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

github-actions bot commented Oct 1, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

github-actions bot commented Oct 1, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s r10s force-pushed the update-to-api33 branch 2 times, most recently from 95e7e2f to b76fd37 Compare October 2, 2023 09:54
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

r10s added 11 commits October 2, 2023 21:52
not sure, if this affects getSystemService(Context.WIFI_SERVICE),
however, we anyway do not ask for permissions here but just try/catch -
if things do not work, we just cannot get the wifi name

(we're not asking for permission as this would require LOCATION in the past -
and that would scare users that just installed Delta Chat
and select "Add Second Devive".
this is the exact reason why NEARBY_WIFI_DEVICES was introduced, btw.
so in case getting the Wifi name does not work on api33,
we can consider asking for a permission - would still be on startup, however)
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

@r10s r10s requested a review from adbenitez October 2, 2023 21:32
@r10s r10s marked this pull request as ready for review October 2, 2023 21:33
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

To test the changes in this pull request, install this apk:
📦 app-preview.apk

Copy link
Member

@adbenitez adbenitez left a comment

Choose a reason for hiding this comment

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

🚀

@r10s r10s merged commit f18831b into main Oct 2, 2023
2 checks passed
@r10s r10s deleted the update-to-api33 branch October 2, 2023 22:42
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.

Upgrade targetSdkVersion and compileSdkVersion to 33
2 participants