From 43e00b79f5ff5fb7aa9a98971d464e2200509915 Mon Sep 17 00:00:00 2001 From: "roux g. buciu" <11182210+adudenamedruby@users.noreply.github.com> Date: Mon, 29 Jan 2024 11:31:03 -0500 Subject: [PATCH] Bugfix FXIOS-8309 [v122.1] System theme resetting bug (#18429) * fix issue * improvements because they're nice * fix mock --- .../Common/Theming/DefaultThemeManager.swift | 38 +++++-------------- .../Sources/Common/Theming/ThemeManager.swift | 3 ++ .../Settings/Main/General/ThemeSetting.swift | 15 ++++++-- .../ThemeSettings/ThemeMiddleware.swift | 11 +++--- .../LegacyThemeManager.swift | 10 +---- .../Client/Telemetry/TelemetryWrapper.swift | 14 ++++++- .../ClientTests/Mocks/MockThemeManager.swift | 4 ++ 7 files changed, 48 insertions(+), 47 deletions(-) diff --git a/BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift b/BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift index a64b47951940..88c88a0d951e 100644 --- a/BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift +++ b/BrowserKit/Sources/Common/Theming/DefaultThemeManager.swift @@ -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( @@ -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), @@ -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 } @@ -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() } } @@ -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() @@ -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() @@ -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) { diff --git a/BrowserKit/Sources/Common/Theming/ThemeManager.swift b/BrowserKit/Sources/Common/Theming/ThemeManager.swift index 6d8834748737..03c4c5d62af7 100644 --- a/BrowserKit/Sources/Common/Theming/ThemeManager.swift +++ b/BrowserKit/Sources/Common/Theming/ThemeManager.swift @@ -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) diff --git a/firefox-ios/Client/Frontend/Settings/Main/General/ThemeSetting.swift b/firefox-ios/Client/Frontend/Settings/Main/General/ThemeSetting.swift index 519fecaa0456..8a896d0bff86 100644 --- a/firefox-ios/Client/Frontend/Settings/Main/General/ThemeSetting.swift +++ b/firefox-ios/Client/Frontend/Settings/Main/General/ThemeSetting.swift @@ -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) @@ -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, diff --git a/firefox-ios/Client/Frontend/Settings/ThemeSettings/ThemeMiddleware.swift b/firefox-ios/Client/Frontend/Settings/ThemeSettings/ThemeMiddleware.swift index 68a131e40d08..1654b39416c2 100644 --- a/firefox-ios/Client/Frontend/Settings/ThemeSettings/ThemeMiddleware.swift +++ b/firefox-ios/Client/Frontend/Settings/ThemeSettings/ThemeMiddleware.swift @@ -31,7 +31,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( @@ -58,7 +58,7 @@ class ThemeManagerMiddleware: ThemeManagerProvider { func getCurrentThemeManagerState(windowUUID: WindowUUID?) -> ThemeSettingsState { // TODO: [8188] Revisit UUID handling, needs additional investigation. ThemeSettingsState(windowUUID: windowUUID ?? WindowUUID.unavailable, - useSystemAppearance: legacyThemeManager.systemThemeIsOn, + useSystemAppearance: themeManager.systemThemeIsOn, isAutomaticBrightnessEnable: legacyThemeManager.automaticBrightnessIsOn, manualThemeSelected: themeManager.currentTheme.type, userBrightnessThreshold: legacyThemeManager.automaticBrightnessValue, @@ -66,7 +66,6 @@ class ThemeManagerMiddleware: ThemeManagerProvider { } func toggleUseSystemAppearance(_ enabled: Bool) { - legacyThemeManager.systemThemeIsOn = enabled themeManager.setSystemTheme(isOn: enabled) } @@ -79,10 +78,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) { diff --git a/firefox-ios/Client/Frontend/Theme/LegacyThemeManager/LegacyThemeManager.swift b/firefox-ios/Client/Frontend/Theme/LegacyThemeManager/LegacyThemeManager.swift index a363c5c28d6c..897474770bf1 100644 --- a/firefox-ios/Client/Frontend/Theme/LegacyThemeManager/LegacyThemeManager.swift +++ b/firefox-ios/Client/Frontend/Theme/LegacyThemeManager/LegacyThemeManager.swift @@ -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), diff --git a/firefox-ios/Client/Telemetry/TelemetryWrapper.swift b/firefox-ios/Client/Telemetry/TelemetryWrapper.swift index 36ace8c40f69..09f420c2328b 100644 --- a/firefox-ios/Client/Telemetry/TelemetryWrapper.swift +++ b/firefox-ios/Client/Telemetry/TelemetryWrapper.swift @@ -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 { @@ -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) diff --git a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockThemeManager.swift b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockThemeManager.swift index ef42991c137b..c16257506607 100644 --- a/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockThemeManager.swift +++ b/firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockThemeManager.swift @@ -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 }