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: Accept the deprecated 3 as an alias for 2 #3410

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

Hocuri
Copy link
Collaborator

@Hocuri Hocuri commented Nov 4, 2024

fix #3408 by automatically changing a 3 to 2.

3 is a deprecated alias for 2, so all UIs now need to handle both values. I think it wasn't a good idea to deprecate 3 in the first place, but it's not worth it going back now.

I tested this PR like this:

  • Without this PR:
    • Set certificate checks to "Accept invalid Certificates" in the settings -> imap_certificate_checks is 2
    • Import a backup from Desktop with "Accept invalid Certificates"
    • Open "Password and Account" in the settings -> the app crashes
  • With this PR:
    • Open "Password and Account" in the settings -> the app doesn't crash anymore
    • -> imap_certificate_checks is 3
    • Save the settings
    • -> imap_certificate_checks is 2

@Hocuri Hocuri requested review from r10s and adbenitez November 4, 2024 16:50
@@ -384,7 +384,6 @@
android:layout_width="0dp"
android:layout_height="48dp"
android:entries="@array/pref_dc_certck_entries"
android:entryValues="@array/pref_dc_certck_values"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was supposed to map "Accept invalid certificates" to 3. This didn't actually work, though, so the behavior doesn't actually change here.

@r10s
Copy link
Member

r10s commented Nov 4, 2024

did a PR fixing the documentation for the CFFI as well, deltachat/deltachat-core-rust#6176

Copy link

github-actions bot commented Nov 4, 2024

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

@r10s r10s mentioned this pull request Nov 5, 2024
@r10s
Copy link
Member

r10s commented Nov 5, 2024

so that we can sleep better, i created #3412 :)

@r10s
Copy link
Member

r10s commented Nov 5, 2024

I think it wasn't a good idea to deprecate 3 in the first place, but it's not worth it going back now.

i agree, lesson learned. we should try harder to keep things stable for UI. we had a similar hickup with DC_QR_BACKUP2 recently.

@r10s r10s added the bug label Nov 5, 2024
@Hocuri
Copy link
Collaborator Author

Hocuri commented Nov 5, 2024

Merging since it fixes a crash

@Hocuri Hocuri merged commit 9679b22 into main Nov 5, 2024
2 checks passed
@Hocuri Hocuri deleted the hoc/fix-certificate-checks branch November 5, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash when opening server configuration for accounts imported from DC desktop
3 participants