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

Adding 'protectionsState' to breakage form submission #2120

Merged
merged 1 commit into from
Nov 7, 2023
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
16 changes: 13 additions & 3 deletions DuckDuckGo/BrokenSiteInfo.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public struct BrokenSiteInfo {

static let allowedQueryReservedCharacters = CharacterSet(charactersIn: ",")

enum ProtectionsState: String {
case enabled = "1"
case disabled = "0"
}
Comment on lines +27 to +30
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Not sure what is the expected way of extending the values range here, but maybe a simple bool would suffice?
  2. Either with the above change or not I suggest replacing "1"/"0" to "true"/"false" to be consistent with other parameters.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Duplicating the response for visibility :)

@miasma13 as we discussed in MM - this mapping to "1" | "0" was done intentionally to match what 'other' platforms are doing with this parameter - since I'm adding to all 5 :)


private struct Keys {
static let url = "siteUrl"
static let category = "category"
Expand All @@ -40,6 +45,7 @@ public struct BrokenSiteInfo {
static let gpc = "gpc"
static let ampUrl = "ampUrl"
static let urlParametersRemoved = "urlParametersRemoved"
static let protectionsState = "protectionsState"
}

private let url: URL?
Expand All @@ -54,12 +60,14 @@ public struct BrokenSiteInfo {
private let manufacturer: String
private let systemVersion: String
private let gpc: Bool

private let protectionsState: ProtectionsState

public init(url: URL?, httpsUpgrade: Bool,
blockedTrackerDomains: [String], installedSurrogates: [String],
isDesktop: Bool, tdsETag: String?,
ampUrl: String?,
urlParametersRemoved: Bool,
protected: Bool,
model: String = UIDevice.current.model,
manufacturer: String = "Apple",
systemVersion: String = UIDevice.current.systemVersion,
Expand All @@ -76,7 +84,8 @@ public struct BrokenSiteInfo {
self.model = model
self.manufacturer = manufacturer
self.systemVersion = systemVersion

self.protectionsState = protected ? .enabled : .disabled

if let gpcParam = gpc {
self.gpc = gpcParam
} else {
Expand All @@ -101,7 +110,8 @@ public struct BrokenSiteInfo {
Keys.model: model,
Keys.gpc: gpc ? "true" : "false",
Keys.ampUrl: ampUrl ?? "",
Keys.urlParametersRemoved: urlParametersRemoved ? "true" : "false"
Keys.urlParametersRemoved: urlParametersRemoved ? "true" : "false",
Keys.protectionsState: protectionsState.rawValue
]

Pixel.fire(pixel: .brokenSiteReport,
Expand Down
8 changes: 6 additions & 2 deletions DuckDuckGo/TabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -866,15 +866,19 @@ class TabViewController: UIViewController {

public func getCurrentWebsiteInfo() -> BrokenSiteInfo {
let blockedTrackerDomains = privacyInfo?.trackerInfo.trackersBlocked.compactMap { $0.domain } ?? []


let configuration = ContentBlocking.shared.privacyConfigurationManager.privacyConfig
let protected = configuration.isFeature(.contentBlocking, enabledForDomain: url?.host)

return BrokenSiteInfo(url: url,
httpsUpgrade: httpsForced,
blockedTrackerDomains: blockedTrackerDomains,
installedSurrogates: privacyInfo?.trackerInfo.installedSurrogates.map { $0 } ?? [],
isDesktop: tabModel.isDesktop,
tdsETag: ContentBlocking.shared.contentBlockingManager.currentMainRules?.etag ?? "",
ampUrl: linkProtection.lastAMPURLString,
urlParametersRemoved: linkProtection.urlParametersRemoved)
urlParametersRemoved: linkProtection.urlParametersRemoved,
protected: protected)
}

public func print() {
Expand Down
1 change: 1 addition & 0 deletions DuckDuckGoTests/BrokenSiteReportingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ final class BrokenSiteReportingTests: XCTestCase {
tdsETag: test.blocklistVersion,
ampUrl: nil,
urlParametersRemoved: false,
protected: true,
model: test.model ?? "",
manufacturer: test.manufacturer ?? "",
systemVersion: test.os ?? "",
Expand Down