Skip to content
This repository has been archived by the owner on May 10, 2024. It is now read-only.

Commit

Permalink
Fix #6953: Fix filter list toggle states (#7150)
Browse files Browse the repository at this point in the history 
* Fix #6953: Fix filter list toggle states

* Fix #6953: Save filter list settings before filter lists are downloaded
  • Loading branch information
@cuba
cuba authored Mar 27, 2023
1 parent f3e4c44 commit 2b97203
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 64 deletions.
79 changes: 38 additions & 41 deletions Sources/Brave/Frontend/Settings/BraveShieldsAndPrivacySettingsController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
let p3aUtilities: BraveP3AUtils

private let cookieConsentNoticesRowUUID = UUID()
private var currentCookieConsentNoticeBlockingState: Bool
private let blockSwitchToAppNoticesRowUUID = UUID()
private var cancellables: Set<AnyCancellable> = []

init(
Expand All @@ -40,11 +40,6 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
self.feedDataSource = feedDataSource
self.historyAPI = historyAPI
self.p3aUtilities = p3aUtilities

self.currentCookieConsentNoticeBlockingState = FilterListResourceDownloader.shared.isEnabled(
for: FilterList.cookieConsentNoticesComponentID
)

super.init(style: .insetGrouped)
}

Expand All @@ -68,32 +63,28 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
// Listen to changes on filter lists so we know we need to reload the section
FilterListResourceDownloader.shared.$filterLists
.sink { filterLists in
let filterList = filterLists.first(where: { $0.componentId == FilterList.cookieConsentNoticesComponentID })

Task { @MainActor in
let isEnabled = FilterListResourceDownloader.shared.isEnabled(for: FilterList.cookieConsentNoticesComponentID)

guard filterList?.isEnabled == isEnabled else {
assertionFailure("The two should be in sync")
return
// Check if we need to update the cookie consent switch
if let filterList = filterLists.first(where: { filterList in
filterList.componentId == FilterList.cookieConsentNoticesComponentID
}) {
// Toggle only if it's needed or an endless loop might be created
self.toggleSwitchIfNeeded(
on: filterList.isEnabled, section: self.shieldsSection,
rowUUID: self.cookieConsentNoticesRowUUID.uuidString
)
}

guard isEnabled != self.currentCookieConsentNoticeBlockingState else { return }
self.currentCookieConsentNoticeBlockingState = isEnabled

guard let sectionIndex = self.dataSource.sections.firstIndex(where: { $0.uuid == self.shieldsSection.uuid }) else {
assertionFailure("Should exist")
return
}

guard let rowIndex = self.shieldsSection.rows.firstIndex(where: { $0.uuid == self.cookieConsentNoticesRowUUID.uuidString }) else {
assertionFailure("Should exist")
return
// Check if we need to update the block mobile notifications switch
if let filterList = filterLists.first(where: { filterList in
filterList.componentId == FilterList.mobileAnnoyancesComponentID
}) {
// Toggle only if it's needed or an endless loop might be created
self.toggleSwitchIfNeeded(
on: filterList.isEnabled, section: self.otherSettingsSection,
rowUUID: self.blockSwitchToAppNoticesRowUUID.uuidString
)
}

// Reload the section
self.shieldsSection.rows[rowIndex] = self.makeCookieConsentBlockingRow()
self.dataSource.sections[sectionIndex] = self.shieldsSection
}
}
.store(in: &cancellables)
Expand Down Expand Up @@ -225,17 +216,17 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
}()

private var blockMobileAnnoyancesRow: Row {
let mobileAnnoyancesComponentID = FilterList.mobileAnnoyancesComponentID
let filterListDownloader = FilterListResourceDownloader.shared

return Row(
text: Strings.blockMobileAnnoyances,
accessory:
.view(SwitchAccessoryView(
initialValue: filterListDownloader.isEnabled(for: mobileAnnoyancesComponentID),
valueChange: { value in
FilterListResourceDownloader.shared.enableFilterList(for: mobileAnnoyancesComponentID, isEnabled: value)
})), cellClass: MultilineSubtitleCell.self, uuid: "blockMobileAnnoyances"
return .boolRow(
uuid: self.blockSwitchToAppNoticesRowUUID,
title: Strings.blockMobileAnnoyances,
toggleValue: FilterListResourceDownloader.shared.isEnabled(
for: FilterList.mobileAnnoyancesComponentID
),
valueChange: { isEnabled in
FilterListResourceDownloader.shared.enableFilterList(
for: FilterList.mobileAnnoyancesComponentID, isEnabled: isEnabled
)
}, cellReuseId: "blockSwitchToAppNoticesReuseIdentifier"
)
}

Expand Down Expand Up @@ -368,7 +359,6 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
for: FilterList.cookieConsentNoticesComponentID
),
valueChange: { isEnabled in
self.currentCookieConsentNoticeBlockingState = isEnabled
FilterListResourceDownloader.shared.enableFilterList(
for: FilterList.cookieConsentNoticesComponentID, isEnabled: isEnabled
)
Expand Down Expand Up @@ -399,8 +389,15 @@ class BraveShieldsAndPrivacySettingsController: TableViewController {
alertController.addAction(.init(title: Strings.cancelButtonTitle, style: .cancel))
self.present(alertController, animated: true, completion: nil)
}

private func toggleSwitchIfNeeded(on: Bool, section: Section, rowUUID: String) {
guard let sectionRow = section.rows.first(where: { $0.uuid == rowUUID }) else { return }
guard let switchView = sectionRow.accessory.view as? UISwitch else { return }
guard switchView.isOn != on else { return }
switchView.setOn(on, animated: true)
}

func toggleSwitch(on: Bool, section: Section, rowUUID: String) {
private func toggleSwitch(on: Bool, section: Section, rowUUID: String) {
if let sectionRow: Row = section.rows.first(where: { $0.uuid == rowUUID }) {
if let switchView: UISwitch = sectionRow.accessory.view as? UISwitch {
switchView.setOn(on, animated: true)
Expand Down
15 changes: 15 additions & 0 deletions Sources/Brave/WebFilters/FilterList.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@ struct FilterList: Identifiable {
"llgjaaddopeckcifdceaaadmemagkepi" // Japanese filter lists
]

/// All the component ids that should be set to on by default.
public static var defaultOnComponentIds: Set<String> {
return [mobileAnnoyancesComponentID]
}

/// This is a list of component to UUID for some filter lists that have special toggles
/// (which are availble before filter lists are downloaded)
/// To save these values before filter lists are downloaded we need to also have the UUID
public static var componentToUUID: [String: String] {
return [
mobileAnnoyancesComponentID: "2F3DCE16-A19A-493C-A88F-2E110FBD37D6",
cookieConsentNoticesComponentID: "AC023D22-AE88-4060-A978-4FEEEC4221693"
]
}

let uuid: String
let title: String
let description: String
Expand Down
41 changes: 18 additions & 23 deletions Sources/Brave/WebFilters/FilterListResourceDownloader.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,27 +41,11 @@ public class FilterListResourceDownloader: ObservableObject {

/// - Warning: Do not call this before we load core data
@MainActor public func isEnabled(for componentID: String) -> Bool {
return allFilterListSettings.first(where: { $0.componentId == componentID })?.isEnabled ?? false
}

/// Set the enabled status of a filter list setting
/// Otherwise it will create a new setting with the specified properties
///
/// - Warning: Do not call this before we load core data
@MainActor public func upsertSetting(uuid: String, isEnabled: Bool) {
if let index = allFilterListSettings.firstIndex(where: { $0.uuid == uuid }) {
updateSetting(
uuid: uuid,
isEnabled: isEnabled,
componentId: allFilterListSettings[index].componentId
)
} else {
create(
uuid: uuid,
componentId: nil,
isEnabled: isEnabled
)
guard let setting = allFilterListSettings.first(where: { $0.componentId == componentID }) else {
return pendingDefaults[componentID] ?? FilterList.defaultOnComponentIds.contains(componentID)
}

return setting.isEnabled
}

/// Set the enabled status and componentId of a filter list setting if the setting exists.
Expand Down Expand Up @@ -240,11 +224,23 @@ public class FilterListResourceDownloader: ObservableObject {
@MainActor public func enableFilterList(for componentID: String, isEnabled: Bool) {
// Enable the setting
defer { self.recordP3ACookieListEnabled() }

if let index = filterLists.firstIndex(where: { $0.componentId == componentID }) {
// Only update the value if it has changed
guard filterLists[index].isEnabled != isEnabled else { return }
filterLists[index].isEnabled = isEnabled
} else if let uuid = FilterList.componentToUUID[componentID] {
let defaultToggle = FilterList.defaultOnComponentIds.contains(componentID)

settingsManager.upsertSetting(
uuid: uuid, isEnabled: isEnabled, componentId: componentID,
allowCreation: defaultToggle != isEnabled
)
} else {
assertionFailure(
"How can this be changed if we don't have a filter list or special shields toggle for this?"
)

// We haven't loaded the filter lists yet. Add it to the pending list.
settingsManager.pendingDefaults[componentID] = isEnabled
}
Expand Down Expand Up @@ -534,9 +530,8 @@ private extension FilterListLanguageProvider {
/// This method returns the default value for this filter list if the user does not manually toggle it.
/// - Warning: Make sure you use `componentID` to identify the filter list, as `uuid` will be deprecated in the future.
var defaultToggle: Bool {
let componentIDsToOverride = [FilterList.mobileAnnoyancesComponentID]

if componentIDsToOverride.contains(componentId) {
// First check if this is a statically set component
if FilterList.defaultOnComponentIds.contains(componentId) {
return true
}

Expand Down

0 comments on commit 2b97203

Please sign in to comment.