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

The Change Passcode and Require Passcode options are not grayed out when Passcode is OFF #6151

Closed
abodea opened this issue Feb 24, 2020 · 5 comments · Fixed by #6250
Closed
Labels
Bug 🐞 This is a bug with existing functionality not behaving as expected Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval Contributor OK This is a good issue for contributors interested in helping the project P3 Issues that would be nice to have for the current release

Comments

@abodea
Copy link
Member

abodea commented Feb 24, 2020

Steps to reproduce

  1. Launch Firefox.
  2. Reach settings and then Face ID & Passwords.
  3. Turn the passcode OFF.
  4. Observe the Change Passcode and Require Passcode.

Expected behavior

The Change Passcode and Require Passcode options should be grayed out while the Passcode is OFF.

Actual behavior

The Change Passcode and Require Passcode options are not grayed out when the Passcode is OFF.

Device information

  • Device: iPad Pro (13.3)
  • Fenix version: v23.0(17297)

unnamed

@abodea abodea added the Bug 🐞 This is a bug with existing functionality not behaving as expected label Feb 24, 2020
@athomasmoz athomasmoz added the P3 Issues that would be nice to have for the current release label Feb 27, 2020
@athomasmoz athomasmoz added the Contributor OK This is a good issue for contributors interested in helping the project label Feb 27, 2020
@nishant2718
Copy link
Contributor

nishant2718 commented Mar 10, 2020

I see this issue in prod on iPhone iOS 13.3 as well. I'll start looking into it.

@dnarcese dnarcese added the Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval label Mar 11, 2020
@vphong
Copy link
Contributor

vphong commented Mar 13, 2020

I think I've pinpointed the problem on this.

When a passcode is added or removed, we call this selector using NotificationCenter.

@objc func refreshSettings(_ notification: Notification) {
updateTitleForTouchIDState()
settings = generateSettings()
tableView.reloadData()
}

but generateSettings() does not do anything with the notification.

override func generateSettings() -> [SettingSection] {
if let _ = KeychainWrapper.sharedAppContainerKeychain.authenticationInfo() {
return passcodeEnabledSettings()
} else {
return passcodeDisabledSettings()
}
}

I'm not totally sure about the best way to expose/send the notification to generateSettings(), so I hesitate to work on a fix. Any thoughts?

@KrystynaKruchkovska
Copy link
Contributor

There is an open request already, textLabel?.textColor = UIColor.theme.tableView.rowText set after correct one.

@vphong
Copy link
Contributor

vphong commented Mar 15, 2020

Apologies, you're right! #6250 does fix the problem.

@dnarcese
Copy link
Contributor

Fixed in #6250

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🐞 This is a bug with existing functionality not behaving as expected Contributor Fix A contributor has fixed this issue. It might still be opened as we're waiting for final QA approval Contributor OK This is a good issue for contributors interested in helping the project P3 Issues that would be nice to have for the current release
Projects
None yet
6 participants