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 LoginsPreferences instances being out of sync #499

Merged
merged 4 commits into from
Apr 4, 2022

Conversation

ayoy
Copy link
Collaborator

@ayoy ayoy commented Apr 2, 2022

Task/Issue URL: https://app.asana.com/0/1199230911884351/1202064896664660/f
Tech Design URL:
CC: @samsymons

Description:
I restored Core-Data-backed LoginsPreferences and added LoginsPreferencesModel with @Published properties only to be used by the preferences screen.
While updating it, I decided to rename LoginsPreferences and related symbols to Autofill*.

Steps to test this PR:

  1. Open the app with Autofill autolock enabled
  2. Go to Autofill Preferences and disable autolock
  3. Open Autofill
  4. Verify that Autofill is unlocked

Testing checklist:

  • Test with Release configuration
  • Test proper deallocation of tabs
  • Make sure committed submodule changes are desired

Internal references:

Software Engineering Expectations
Technical Design Template
When ready for review, remember to post the PR in MM

@samsymons samsymons self-assigned this Apr 2, 2022
Copy link
Collaborator

@samsymons samsymons left a comment

Choose a reason for hiding this comment

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

This is looking great, nice work! 👏 I really like this approach to the problem.

There's one last thing we need to restore that was lost during the preferences re-write: changing the Auto-Lock setting needs to be gated behind an authentication check, otherwise someone could disable the check in order to bypass it.

The check was originally located here - once we have that authentication back in, this is good to merge!

@tomasstrba tomasstrba assigned tomasstrba and unassigned samsymons Apr 4, 2022
@@ -20,42 +20,84 @@ import Foundation

final class AutofillPreferencesModel: ObservableObject {

@Published var isAutoLockEnabled: Bool {
@Published var askToSaveUsernamesAndPasswords: Bool {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lots of git diff noise here, but I only updated isAutoLockEnabled and autoLockThreshold and then I moved them below the other 3 properties.

Copy link
Contributor

@tomasstrba tomasstrba left a comment

Choose a reason for hiding this comment

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

LGMT! 👍 💯

@tomasstrba tomasstrba assigned ayoy and unassigned tomasstrba Apr 4, 2022
@ayoy ayoy merged commit 9000386 into develop Apr 4, 2022
@ayoy ayoy deleted the dominik/logins-preferences branch April 4, 2022 10:41
samsymons added a commit that referenced this pull request Apr 10, 2022
# By Dominik Kapusta (2) and Tomas Strba (1)
# Via Tomas Strba
* develop:
  Version 0.22.0
  Fix Website Breakage Reports by allowing unescaped commas in URL query (#498)
  Fix LoginsPreferences instances being out of sync (#499)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
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.

3 participants