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

Bugfix FXIOS-8309 [v122.1] System theme resetting bug (backport #18429) #18456

Merged
merged 2 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 10 additions & 28 deletions BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
return userDefaults.bool(forKey: ThemeKeys.PrivateMode.isOn)
}

public var isSystemThemeOn: Bool {
public var systemThemeIsOn: Bool {
return userDefaults.bool(forKey: ThemeKeys.systemThemeIsOn)
}

public var automaticBrightnessIsOn: Bool {
return userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)
}

// MARK: - Initializers

public init(
Expand All @@ -64,8 +68,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
self.mainQueue = mainQueue
self.sharedContainerIdentifier = sharedContainerIdentifier

migrateDefaultsToUseStandard()

self.userDefaults.register(defaults: [
ThemeKeys.systemThemeIsOn: true,
ThemeKeys.NightMode.isOn: NSNumber(value: false),
Expand Down Expand Up @@ -93,7 +95,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
// the system theme is off
// OR night mode is on
// OR private mode is on
guard isSystemThemeOn,
guard systemThemeIsOn,
!nightModeIsOn,
!privateModeIsOn
else { return }
Expand All @@ -104,9 +106,9 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
public func setSystemTheme(isOn: Bool) {
userDefaults.set(isOn, forKey: ThemeKeys.systemThemeIsOn)

if isOn {
if systemThemeIsOn {
systemThemeChanged()
} else if userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn) {
} else if automaticBrightnessIsOn {
updateThemeBasedOnBrightness()
}
}
Expand All @@ -118,8 +120,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
}

public func setAutomaticBrightness(isOn: Bool) {
let currentState = userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)
guard currentState != isOn else { return }
guard automaticBrightnessIsOn != isOn else { return }

userDefaults.set(isOn, forKey: ThemeKeys.AutomaticBrightness.isOn)
brightnessChanged()
Expand Down Expand Up @@ -181,9 +182,7 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
}

private func brightnessChanged() {
let brightnessIsOn = userDefaults.bool(forKey: ThemeKeys.AutomaticBrightness.isOn)

if brightnessIsOn {
if automaticBrightnessIsOn {
updateThemeBasedOnBrightness()
} else {
systemThemeChanged()
Expand All @@ -201,23 +200,6 @@ public final class DefaultThemeManager: ThemeManager, Notifiable {
}
}

private func migrateDefaultsToUseStandard() {
guard let oldDefaultsStore = UserDefaults(suiteName: sharedContainerIdentifier) else { return }

if let systemThemeIsOn = oldDefaultsStore.value(forKey: ThemeKeys.systemThemeIsOn) {
userDefaults.set(systemThemeIsOn, forKey: ThemeKeys.systemThemeIsOn)
}
if let nightModeIsOn = oldDefaultsStore.value(forKey: ThemeKeys.NightMode.isOn) {
userDefaults.set(nightModeIsOn, forKey: ThemeKeys.NightMode.isOn)
}
if let automaticBrightnessIsOn = oldDefaultsStore.value(forKey: ThemeKeys.AutomaticBrightness.isOn) {
userDefaults.set(automaticBrightnessIsOn, forKey: ThemeKeys.AutomaticBrightness.isOn)
}
if let automaticBrighnessValue = oldDefaultsStore.value(forKey: ThemeKeys.AutomaticBrightness.thresholdValue) {
userDefaults.set(automaticBrighnessValue, forKey: ThemeKeys.AutomaticBrightness.thresholdValue)
}
}

// MARK: - Notifiable

public func handleNotifications(_ notification: Notification) {
Expand Down
3 changes: 3 additions & 0 deletions BrowserKit/Sources/Common/Theming/ThemeManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ public protocol ThemeManager {
var currentTheme: Theme { get }
var window: UIWindow? { get set }

var systemThemeIsOn: Bool { get }
var automaticBrightnessIsOn: Bool { get }

func changeCurrentTheme(_ newTheme: ThemeType)
func systemThemeChanged()
func setSystemTheme(isOn: Bool)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
// file, You can obtain one at http://mozilla.org/MPL/2.0/

import Foundation
import Common

class ThemeSetting: Setting {
private weak var settingsDelegate: GeneralSettingsDelegate?
private let profile: Profile
private let themeManager: ThemeManager

override var accessoryView: UIImageView? {
return SettingDisclosureUtility.buildDisclosureIndicator(theme: theme)
Expand All @@ -18,20 +20,25 @@ class ThemeSetting: Setting {
}

override var status: NSAttributedString {
if LegacyThemeManager.instance.systemThemeIsOn {
if themeManager.systemThemeIsOn {
return NSAttributedString(string: .SystemThemeSectionHeader)
} else if !LegacyThemeManager.instance.automaticBrightnessIsOn {
} else if !themeManager.automaticBrightnessIsOn {
return NSAttributedString(string: .DisplayThemeManualStatusLabel)
} else if LegacyThemeManager.instance.automaticBrightnessIsOn {
} else if themeManager.automaticBrightnessIsOn {
return NSAttributedString(string: .DisplayThemeAutomaticStatusLabel)
}

return NSAttributedString(string: "")
}

init(settings: SettingsTableViewController,
settingsDelegate: GeneralSettingsDelegate?) {
settingsDelegate: GeneralSettingsDelegate?,
themeManager: ThemeManager = AppContainer.shared.resolve()
) {
self.profile = settings.profile
self.settingsDelegate = settingsDelegate
self.themeManager = themeManager

super.init(
title: NSAttributedString(
string: .SettingsDisplayThemeTitle,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ThemeManagerMiddleware: ThemeManagerProvider {
store.dispatch(ThemeSettingsAction.receivedThemeManagerValues(currentThemeState))
case ThemeSettingsAction.toggleUseSystemAppearance(let enabled):
self.toggleUseSystemAppearance(enabled)
store.dispatch(ThemeSettingsAction.systemThemeChanged(self.legacyThemeManager.systemThemeIsOn))
store.dispatch(ThemeSettingsAction.systemThemeChanged(self.themeManager.systemThemeIsOn))
case ThemeSettingsAction.enableAutomaticBrightness(let enabled):
self.toggleAutomaticBrightness(enabled)
store.dispatch(
Expand All @@ -55,15 +55,14 @@ class ThemeManagerMiddleware: ThemeManagerProvider {

// MARK: - Helper func
func getCurrentThemeManagerState() -> ThemeSettingsState {
ThemeSettingsState(useSystemAppearance: legacyThemeManager.systemThemeIsOn,
ThemeSettingsState(useSystemAppearance: themeManager.systemThemeIsOn,
isAutomaticBrightnessEnable: legacyThemeManager.automaticBrightnessIsOn,
manualThemeSelected: themeManager.currentTheme.type,
userBrightnessThreshold: legacyThemeManager.automaticBrightnessValue,
systemBrightness: getScreenBrightness())
}

func toggleUseSystemAppearance(_ enabled: Bool) {
legacyThemeManager.systemThemeIsOn = enabled
themeManager.setSystemTheme(isOn: enabled)
}

Expand All @@ -76,10 +75,10 @@ class ThemeManagerMiddleware: ThemeManagerProvider {
themeManager.setAutomaticBrightness(isOn: enabled)
}

func updateManualTheme(_ theme: ThemeType) {
let isLightTheme = theme == .light
func updateManualTheme(_ newTheme: ThemeType) {
let isLightTheme = newTheme == .light
legacyThemeManager.current = isLightTheme ? LegacyNormalTheme() : LegacyDarkTheme()
themeManager.changeCurrentTheme(isLightTheme ? .light : .dark)
themeManager.changeCurrentTheme(newTheme)
}

func updateUserBrightness(_ value: Float) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,12 @@ class LegacyThemeManager {
}
}

var systemThemeIsOn: Bool {
didSet {
UserDefaults.standard.set(
systemThemeIsOn,
forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue
)
}
private var systemThemeIsOn: Bool {
return UserDefaults.standard.bool(forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue)
}

private init() {
UserDefaults.standard.register(defaults: [LegacyThemeManagerPrefs.systemThemeIsOn.rawValue: true])
systemThemeIsOn = UserDefaults.standard.bool(forKey: LegacyThemeManagerPrefs.systemThemeIsOn.rawValue)

NotificationCenter.default.addObserver(self,
selector: #selector(brightnessChanged),
Expand Down
14 changes: 13 additions & 1 deletion firefox-ios/Client/Telemetry/TelemetryWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -161,14 +161,17 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
// If the setting exists at the key location, use that value. Otherwise record the default
// value for that preference to ensure it makes it into the metrics ping.
let prefs = profile.prefs

// FxA Account Login status
GleanMetrics.Preferences.fxaLoggedIn.set(profile.hasSyncableAccount())

// Record New Tab setting
if let newTabChoice = prefs.stringForKey(NewTabAccessors.HomePrefKey) {
GleanMetrics.Preferences.newTabExperience.set(newTabChoice)
} else {
GleanMetrics.Preferences.newTabExperience.set(NewTabAccessors.Default.rawValue)
}

// Record `Home` setting, where Firefox Home is "Home", a custom URL is "other" and blank is "Blank".
let homePageSetting = NewTabAccessors.getHomePage(prefs)
switch homePageSetting {
Expand All @@ -181,45 +184,54 @@ class TelemetryWrapper: TelemetryWrapperProtocol, FeatureFlaggable {
default:
GleanMetrics.Preferences.homePageSetting.set(homePageSetting.rawValue)
}

// Notifications
GleanMetrics.Preferences.tipsAndFeaturesNotifs.set(UserDefaults.standard.bool(forKey: PrefsKeys.Notifications.TipsAndFeaturesNotifications))
GleanMetrics.Preferences.syncNotifs.set(UserDefaults.standard.bool(forKey: PrefsKeys.Notifications.SyncNotifications))

// Save logins
if let saveLogins = prefs.boolForKey(PrefsKeys.LoginsSaveEnabled) {
GleanMetrics.Preferences.saveLogins.set(saveLogins)
} else {
GleanMetrics.Preferences.saveLogins.set(true)
}

// Show clipboard bar
if let showClipboardBar = prefs.boolForKey("showClipboardBar") {
GleanMetrics.Preferences.showClipboardBar.set(showClipboardBar)
} else {
GleanMetrics.Preferences.showClipboardBar.set(false)
}

// Close private tabs
if let closePrivateTabs = prefs.boolForKey("settings.closePrivateTabs") {
GleanMetrics.Preferences.closePrivateTabs.set(closePrivateTabs)
} else {
GleanMetrics.Preferences.closePrivateTabs.set(false)
}

// Tracking protection - enabled
if let tpEnabled = prefs.boolForKey(ContentBlockingConfig.Prefs.EnabledKey) {
GleanMetrics.TrackingProtection.enabled.set(tpEnabled)
} else {
GleanMetrics.TrackingProtection.enabled.set(true)
}

// Tracking protection - strength
if let tpStrength = prefs.stringForKey(ContentBlockingConfig.Prefs.StrengthKey) {
GleanMetrics.TrackingProtection.strength.set(tpStrength)
} else {
GleanMetrics.TrackingProtection.strength.set("basic")
}

// System theme enabled
let themeManager = DefaultThemeManager(sharedContainerIdentifier: AppInfo.sharedContainerIdentifier)
GleanMetrics.Theme.useSystemTheme.set(themeManager.isSystemThemeOn)
GleanMetrics.Theme.useSystemTheme.set(themeManager.systemThemeIsOn)

// Installed Mozilla applications
GleanMetrics.InstalledMozillaProducts.focus.set(UIApplication.shared.canOpenURL(URL(string: "firefox-focus://")!))
GleanMetrics.InstalledMozillaProducts.klar.set(UIApplication.shared.canOpenURL(URL(string: "firefox-klar://")!))

// Device Authentication
GleanMetrics.Device.authentication.set(AppAuthenticator().canAuthenticateDeviceOwner)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ class MockThemeManager: ThemeManager {
var currentTheme: Theme = LightTheme()
var window: UIWindow?

var systemThemeIsOn: Bool { return true}

var automaticBrightnessIsOn: Bool { return false}

func getInterfaceStyle() -> UIUserInterfaceStyle {
return .light
}
Expand Down