From d1ce6defbd19f40f2ee55fb49581540837b6ce69 Mon Sep 17 00:00:00 2001 From: Pete Smith <5278441+aataraxiaa@users.noreply.github.com> Date: Wed, 13 Nov 2024 11:06:13 +0000 Subject: [PATCH 01/13] Prevent Freemium PIR Pixel Firing for All Users (#3543) Task/Issue URL: https://app.asana.com/0/1206488453854252/1208744771117526/f **Description**: We prevent this pixel firing and also prevent the creation of a `viewModel` unless the Freemium feature is enabled --- .github/workflows/build_notarized.yml | 7 ++++- .github/workflows/sync_end_to_end.yml | 1 + .github/workflows/ui_tests.yml | 1 + .../FreemiumDBPPromotionViewCoordinator.swift | 7 +++-- DuckDuckGo/HomePage/View/HomePageView.swift | 11 +++++-- ...miumDBPPromotionViewCoordinatorTests.swift | 29 ++++++++++++++----- 6 files changed, 41 insertions(+), 15 deletions(-) diff --git a/.github/workflows/build_notarized.yml b/.github/workflows/build_notarized.yml index c0dc2e22c7..76d8ff71f4 100644 --- a/.github/workflows/build_notarized.yml +++ b/.github/workflows/build_notarized.yml @@ -40,6 +40,11 @@ on: description: "Branch name" required: false type: string + skip-notify: + description: "Skip Mattermost notification" + required: false + default: false + type: boolean secrets: APPLE_API_KEY_BASE64: required: true @@ -360,7 +365,7 @@ jobs: name: Send Mattermost message needs: [export-notarized-app, create-dmg] - if: always() + if: ${{ always() && inputs.skip-notify == false }} runs-on: ubuntu-latest diff --git a/.github/workflows/sync_end_to_end.yml b/.github/workflows/sync_end_to_end.yml index 91308789b7..f1b46005df 100644 --- a/.github/workflows/sync_end_to_end.yml +++ b/.github/workflows/sync_end_to_end.yml @@ -13,6 +13,7 @@ jobs: release-type: review create-dmg: false branch: ${{ github.sha }} + skip-notify: true secrets: APPLE_API_KEY_BASE64: ${{ secrets.APPLE_API_KEY_BASE64 }} APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} diff --git a/.github/workflows/ui_tests.yml b/.github/workflows/ui_tests.yml index 611b105a5a..857158c321 100644 --- a/.github/workflows/ui_tests.yml +++ b/.github/workflows/ui_tests.yml @@ -17,6 +17,7 @@ jobs: release-type: review create-dmg: false branch: ${{ github.sha }} + skip-notify: true secrets: APPLE_API_KEY_BASE64: ${{ secrets.APPLE_API_KEY_BASE64 }} APPLE_API_KEY_ID: ${{ secrets.APPLE_API_KEY_ID }} diff --git a/DuckDuckGo/Freemium/DBP/FreemiumDBPPromotionViewCoordinator.swift b/DuckDuckGo/Freemium/DBP/FreemiumDBPPromotionViewCoordinator.swift index d764c8f154..56631b6b0c 100644 --- a/DuckDuckGo/Freemium/DBP/FreemiumDBPPromotionViewCoordinator.swift +++ b/DuckDuckGo/Freemium/DBP/FreemiumDBPPromotionViewCoordinator.swift @@ -31,9 +31,10 @@ final class FreemiumDBPPromotionViewCoordinator: ObservableObject { /// Published property that determines whether the promotion is visible on the home page. @Published var isHomePagePromotionVisible: Bool = false - /// The view model representing the promotion, which updates based on the user's state. - var viewModel: PromotionViewModel { - createViewModel() + /// The view model representing the promotion, which updates based on the user's state. Returns `nil` if the feature is not enabled + var viewModel: PromotionViewModel? { + guard freemiumDBPFeature.isAvailable else { return nil } + return createViewModel() } /// Stores whether the user has dismissed the home page promotion. diff --git a/DuckDuckGo/HomePage/View/HomePageView.swift b/DuckDuckGo/HomePage/View/HomePageView.swift index 428c7b85db..511213a45d 100644 --- a/DuckDuckGo/HomePage/View/HomePageView.swift +++ b/DuckDuckGo/HomePage/View/HomePageView.swift @@ -224,10 +224,15 @@ extension HomePage.Views { } } + @ViewBuilder func freemiumPromotionView() -> some View { - PromotionView(viewModel: freemiumDBPPromotionViewCoordinator.viewModel) - .padding(.bottom, 16) - .visibility(freemiumDBPPromotionViewCoordinator.isHomePagePromotionVisible ? .visible : .gone) + if let viewModel = freemiumDBPPromotionViewCoordinator.viewModel { + PromotionView(viewModel: viewModel) + .padding(.bottom, 16) + .visibility(freemiumDBPPromotionViewCoordinator.isHomePagePromotionVisible ? .visible : .gone) + } else { + EmptyView() + } } @ViewBuilder diff --git a/UnitTests/Freemium/DBP/FreemiumDBPPromotionViewCoordinatorTests.swift b/UnitTests/Freemium/DBP/FreemiumDBPPromotionViewCoordinatorTests.swift index bb5329ba78..fca8c31aec 100644 --- a/UnitTests/Freemium/DBP/FreemiumDBPPromotionViewCoordinatorTests.swift +++ b/UnitTests/Freemium/DBP/FreemiumDBPPromotionViewCoordinatorTests.swift @@ -37,6 +37,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { override func setUpWithError() throws { mockUserStateManager = MockFreemiumDBPUserStateManager() mockFeature = MockFreemiumDBPFeature() + mockFeature.featureAvailable = true mockPresenter = MockFreemiumDBPPresenter() mockPixelHandler = MockFreemiumDBPExperimentPixelHandler() @@ -97,7 +98,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { // When let viewModel = sut.viewModel - viewModel.proceedAction() + viewModel!.proceedAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -109,7 +110,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { func testCloseAction_dismissesPromotion_andFiresPixel() { // When let viewModel = sut.viewModel - viewModel.closeAction() + viewModel!.closeAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -124,7 +125,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { // When let viewModel = sut.viewModel - viewModel.proceedAction() + viewModel!.proceedAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -139,7 +140,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { // When let viewModel = sut.viewModel - viewModel.closeAction() + viewModel!.closeAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -154,7 +155,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { // When let viewModel = sut.viewModel - viewModel.proceedAction() + viewModel!.proceedAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -169,7 +170,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { // When let viewModel = sut.viewModel - viewModel.closeAction() + viewModel!.closeAction() // Then XCTAssertTrue(mockUserStateManager.didDismissHomePagePromotion) @@ -185,7 +186,7 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { let viewModel = sut.viewModel // Then - XCTAssertEqual(viewModel.description, UserText.homePagePromotionFreemiumDBPPostScanEngagementResultPluralDescription(resultCount: 5, brokerCount: 2)) + XCTAssertEqual(viewModel!.description, UserText.homePagePromotionFreemiumDBPPostScanEngagementResultPluralDescription(resultCount: 5, brokerCount: 2)) } @MainActor @@ -197,7 +198,19 @@ final class FreemiumDBPPromotionViewCoordinatorTests: XCTestCase { let viewModel = sut.viewModel // Then - XCTAssertEqual(viewModel.description, UserText.homePagePromotionFreemiumDBPDescriptionMarkdown) + XCTAssertEqual(viewModel!.description, UserText.homePagePromotionFreemiumDBPDescriptionMarkdown) + } + + @MainActor + func testViewModel_whenFeatureNotEnabled() { + // Given + mockFeature.featureAvailable = false + + // When + let viewModel = sut.viewModel + + // Then + XCTAssertNil(viewModel) } func testNotificationObservation_updatesPromotionVisibility() { From cf4389d8026f682c927696cdad48aba3c7ab9468 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 13 Nov 2024 13:01:50 +0100 Subject: [PATCH 02/13] Add cohort assignment pixel for NTP Search Box Experiment (#3539) Task/Issue URL: https://app.asana.com/0/1201621853593513/1208742047745355/f Description: This change adds a pixel that's fired when a user is enrolled into the NTP search box experiment. --- .../Model/HomePageAddressBarModel.swift | 16 +++---- .../Model/NewTabPageSearchBoxExperiment.swift | 47 +++++++++++++++---- .../View/HomePageViewController.swift | 1 + .../NewTabSearchBoxExperimentPixel.swift | 13 +++++ .../NewTabPageSearchBoxExperimentTests.swift | 35 ++++++++++++++ 5 files changed, 94 insertions(+), 18 deletions(-) diff --git a/DuckDuckGo/HomePage/Model/HomePageAddressBarModel.swift b/DuckDuckGo/HomePage/Model/HomePageAddressBarModel.swift index a945c8a700..6ec762b35d 100644 --- a/DuckDuckGo/HomePage/Model/HomePageAddressBarModel.swift +++ b/DuckDuckGo/HomePage/Model/HomePageAddressBarModel.swift @@ -98,6 +98,14 @@ extension HomePage.Models { return addressBarViewController.view } + func setUpExperimentIfNeeded() { + if isExperimentActive { + let ntpExperiment = NewTabPageSearchBoxExperiment() + ntpExperiment.assignUserToCohort() + shouldShowAddressBar = ntpExperiment.cohort?.isExperiment == true + } + } + let tabCollectionViewModel: TabCollectionViewModel private var isExperimentActive: Bool = false { @@ -118,14 +126,6 @@ extension HomePage.Models { } } - private func setUpExperimentIfNeeded() { - if isExperimentActive { - let ntpExperiment = NewTabPageSearchBoxExperiment() - ntpExperiment.assignUserToCohort() - shouldShowAddressBar = ntpExperiment.cohort?.isExperiment == true - } - } - private lazy var addressBarViewController: AddressBarViewController? = createAddressBarViewController() private func createAddressBarViewController() -> AddressBarViewController? { diff --git a/DuckDuckGo/HomePage/Model/NewTabPageSearchBoxExperiment.swift b/DuckDuckGo/HomePage/Model/NewTabPageSearchBoxExperiment.swift index eb49a31193..d63c0ab3e9 100644 --- a/DuckDuckGo/HomePage/Model/NewTabPageSearchBoxExperiment.swift +++ b/DuckDuckGo/HomePage/Model/NewTabPageSearchBoxExperiment.swift @@ -96,6 +96,11 @@ struct DefaultNewTabPageSearchBoxExperimentCohortDecider: NewTabPageSearchBoxExp } protocol NewTabPageSearchBoxExperimentPixelReporting { + func fireNTPSearchBoxExperimentCohortAssignmentPixel( + cohort: NewTabPageSearchBoxExperiment.Cohort, + onboardingCohort: PixelExperiment? + ) + func fireNTPSearchBoxExperimentPixel( day: Int, count: Int, @@ -106,6 +111,11 @@ protocol NewTabPageSearchBoxExperimentPixelReporting { } struct DefaultNewTabPageSearchBoxExperimentPixelReporter: NewTabPageSearchBoxExperimentPixelReporting { + + func fireNTPSearchBoxExperimentCohortAssignmentPixel(cohort: NewTabPageSearchBoxExperiment.Cohort, onboardingCohort: PixelExperiment?) { + PixelKit.fire(NewTabSearchBoxExperimentPixel.cohortAssigned(cohort: cohort, onboardingCohort: onboardingCohort)) + } + func fireNTPSearchBoxExperimentPixel( day: Int, count: Int, @@ -126,10 +136,15 @@ struct DefaultNewTabPageSearchBoxExperimentPixelReporter: NewTabPageSearchBoxExp } protocol OnboardingExperimentCohortProviding { + var isOnboardingFinished: Bool { get } var onboardingExperimentCohort: PixelExperiment? { get } } struct DefaultOnboardingExperimentCohortProvider: OnboardingExperimentCohortProviding { + var isOnboardingFinished: Bool { + UserDefaultsWrapper(key: .onboardingFinished, defaultValue: false).wrappedValue + } + var onboardingExperimentCohort: PixelExperiment? { PixelExperiment.logic.cohort } @@ -155,18 +170,24 @@ final class NewTabPageSearchBoxExperiment { } enum Cohort: String { - case control - case experiment = "ntp_search_box" - case controlExistingUser = "control_existing_user" - case experimentExistingUser = "ntp_search_box_existing_user" + case control = "control_v2" + case experiment = "ntp_search_box_v2" + case controlExistingUser = "control_existing_user_v2" + case experimentExistingUser = "ntp_search_box_existing_user_v2" + case legacyControl = "control" + case legacyExperiment = "ntp_search_box" + case legacyControlExistingUser = "control_existing_user" + case legacyExperimentExistingUser = "ntp_search_box_existing_user" + + static let allExperimentCohortValues: Set = [ + .legacyExperiment, + .legacyExperimentExistingUser, + .experiment, + .experimentExistingUser + ] var isExperiment: Bool { - switch self { - case .experiment, .experimentExistingUser: - return true - default: - return false - } + return Self.allExperimentCohortValues.contains(self) } } @@ -199,6 +220,11 @@ final class NewTabPageSearchBoxExperiment { return } + guard onboardingExperimentCohortProvider.isOnboardingFinished else { + Logger.newTabPageSearchBoxExperiment.debug("Skipping cohort assignment until onboarding is finished...") + return + } + guard let cohort = cohortDecider.cohort else { Logger.newTabPageSearchBoxExperiment.debug("User is not eligible for the experiment, skipping cohort assignment...") dataStore.experimentCohort = nil @@ -211,6 +237,7 @@ final class NewTabPageSearchBoxExperiment { dataStore.didRunEnrollment = true Logger.newTabPageSearchBoxExperiment.debug("User assigned to cohort \(cohort.rawValue)") + pixelReporter.fireNTPSearchBoxExperimentCohortAssignmentPixel(cohort: cohort, onboardingCohort: onboardingExperimentCohortProvider.onboardingExperimentCohort) } func recordSearch(from source: SearchSource) { diff --git a/DuckDuckGo/HomePage/View/HomePageViewController.swift b/DuckDuckGo/HomePage/View/HomePageViewController.swift index fef4a90e1b..04635afce2 100644 --- a/DuckDuckGo/HomePage/View/HomePageViewController.swift +++ b/DuckDuckGo/HomePage/View/HomePageViewController.swift @@ -125,6 +125,7 @@ final class HomePageViewController: NSViewController { PixelKit.fire(GeneralPixel.newTabInitial, frequency: .legacyInitial) } subscribeToHistory() + addressBarModel.setUpExperimentIfNeeded() } override func viewDidAppear() { diff --git a/DuckDuckGo/Statistics/NewTabSearchBoxExperimentPixel.swift b/DuckDuckGo/Statistics/NewTabSearchBoxExperimentPixel.swift index f075615339..9100f8aa1a 100644 --- a/DuckDuckGo/Statistics/NewTabSearchBoxExperimentPixel.swift +++ b/DuckDuckGo/Statistics/NewTabSearchBoxExperimentPixel.swift @@ -28,10 +28,13 @@ import PixelKit */ enum NewTabSearchBoxExperimentPixel: PixelKitEventV2 { + case cohortAssigned(cohort: NewTabPageSearchBoxExperiment.Cohort, onboardingCohort: PixelExperiment?) case initialSearch(day: Int, count: Int, from: NewTabPageSearchBoxExperiment.SearchSource, cohort: NewTabPageSearchBoxExperiment.Cohort, onboardingCohort: PixelExperiment?) var name: String { switch self { + case .cohortAssigned: + return "m_mac_initial-search-day-1" case .initialSearch(let day, _, _, _, _): return "m_mac_initial-search-day-\(day)" } @@ -39,6 +42,16 @@ enum NewTabSearchBoxExperimentPixel: PixelKitEventV2 { var parameters: [String: String]? { switch self { + case let .cohortAssigned(cohort, onboardingCohort): + var parameters = [ + Parameters.count: "0", + Parameters.cohort: cohort.rawValue + ] + if let onboardingCohort { + parameters[Parameters.onboardingCohort] = onboardingCohort.rawValue + } + return parameters + case let .initialSearch(_, count, from, cohort, onboardingCohort): var parameters = [ Parameters.count: String(count), diff --git a/UnitTests/HomePage/NewTabPageSearchBoxExperimentTests.swift b/UnitTests/HomePage/NewTabPageSearchBoxExperimentTests.swift index b28b340bc4..e363a28296 100644 --- a/UnitTests/HomePage/NewTabPageSearchBoxExperimentTests.swift +++ b/UnitTests/HomePage/NewTabPageSearchBoxExperimentTests.swift @@ -25,6 +25,7 @@ class MockNewTabPageSearchBoxExperimentCohortDecider: NewTabPageSearchBoxExperim } class MockOnboardingExperimentCohortProvider: OnboardingExperimentCohortProviding { + var isOnboardingFinished: Bool = true var onboardingExperimentCohort: PixelExperiment? } @@ -38,6 +39,12 @@ class CapturingNewTabPageSearchBoxExperimentPixelReporter: NewTabPageSearchBoxEx } var calls: [PixelArguments] = [] + var cohortAssignmentCalls: [NewTabPageSearchBoxExperiment.Cohort] = [] + + func fireNTPSearchBoxExperimentCohortAssignmentPixel(cohort: NewTabPageSearchBoxExperiment.Cohort, onboardingCohort: PixelExperiment?) { + cohortAssignmentCalls.append(cohort) + } + func fireNTPSearchBoxExperimentPixel( day: Int, count: Int, @@ -81,6 +88,31 @@ final class NewTabPageSearchBoxExperimentTests: XCTestCase { super.tearDown() } + func testThatLegacyAndCurrentExperimentCohortsAreCorrectlyIdentified() { + XCTAssertTrue(NewTabPageSearchBoxExperiment.Cohort.experiment.isExperiment) + XCTAssertTrue(NewTabPageSearchBoxExperiment.Cohort.experimentExistingUser.isExperiment) + XCTAssertTrue(NewTabPageSearchBoxExperiment.Cohort.legacyExperiment.isExperiment) + XCTAssertTrue(NewTabPageSearchBoxExperiment.Cohort.legacyExperimentExistingUser.isExperiment) + + XCTAssertFalse(NewTabPageSearchBoxExperiment.Cohort.control.isExperiment) + XCTAssertFalse(NewTabPageSearchBoxExperiment.Cohort.controlExistingUser.isExperiment) + XCTAssertFalse(NewTabPageSearchBoxExperiment.Cohort.legacyControl.isExperiment) + XCTAssertFalse(NewTabPageSearchBoxExperiment.Cohort.legacyControlExistingUser.isExperiment) + } + + func testWhenUserIsNotEnrolledAndOnboardingIsNotFinishedThenCohortIsNotSet() { + onboardingExperimentCohortProvider.isOnboardingFinished = false + cohortDecider.cohort = .experimentExistingUser + let date = Date() + experiment.assignUserToCohort() + + XCTAssertFalse(dataStore.didRunEnrollment) + XCTAssertFalse(experiment.isActive) + XCTAssertNil(dataStore.enrollmentDate) + XCTAssertNil(experiment.cohort) + XCTAssertTrue(pixelReporter.cohortAssignmentCalls.isEmpty) + } + func testWhenUserIsNotEnrolledAndIsEligibleForExperimentThenCohortIsSet() throws { cohortDecider.cohort = .experimentExistingUser let date = Date() @@ -90,6 +122,7 @@ final class NewTabPageSearchBoxExperimentTests: XCTestCase { XCTAssertTrue(experiment.isActive) XCTAssertGreaterThan(try XCTUnwrap(dataStore.enrollmentDate), date) XCTAssertEqual(experiment.cohort, cohortDecider.cohort) + XCTAssertEqual(pixelReporter.cohortAssignmentCalls, [cohortDecider.cohort]) } func testWhenUserIsNotEnrolledAndIsNotEligibleForExperimentThenCohortIsNil() { @@ -101,6 +134,7 @@ final class NewTabPageSearchBoxExperimentTests: XCTestCase { XCTAssertTrue(dataStore.didRunEnrollment) XCTAssertFalse(experiment.isActive) XCTAssertNil(experiment.cohort) + XCTAssertTrue(pixelReporter.cohortAssignmentCalls.isEmpty) } func testWhenUserIsEnrolledThenSubsequentCohortAssignmentsHaveNoEffect() { @@ -114,6 +148,7 @@ final class NewTabPageSearchBoxExperimentTests: XCTestCase { XCTAssertTrue(dataStore.didRunEnrollment) XCTAssertEqual(experiment.cohort, .control) XCTAssertEqual(dataStore.enrollmentDate, date) + XCTAssertTrue(pixelReporter.cohortAssignmentCalls.isEmpty) } func testWhenUserIsEnrolledThenIsActiveReturnsFalseWhenExperimentExpires() { From 7fdf739d346fc201594597030371d850eb950aec Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Wed, 13 Nov 2024 17:30:58 -0300 Subject: [PATCH 03/13] Implement Fire Window UI tests (#3544) Task/Issue URL: https://app.asana.com/0/1204006570077678/1205717021705381/f Tech Design URL: CC: **Description**: Implement Fire Window UI tests **Steps to test this PR**: Check that workflow passes on macOS 13 and 14: https://github.com/duckduckgo/macos-browser/actions/runs/11817794843 **Definition of Done**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --------- Co-authored-by: Dominik Kapusta --- .github/workflows/sync_end_to_end.yml | 1 + .github/workflows/ui_tests.yml | 4 + DuckDuckGo.xcodeproj/project.pbxproj | 4 + UITests/FireWindowTests.swift | 340 ++++++++++++++++++++++++++ fastlane/Fastfile | 12 + 5 files changed, 361 insertions(+) create mode 100644 UITests/FireWindowTests.swift diff --git a/.github/workflows/sync_end_to_end.yml b/.github/workflows/sync_end_to_end.yml index 91308789b7..d6820299e7 100644 --- a/.github/workflows/sync_end_to_end.yml +++ b/.github/workflows/sync_end_to_end.yml @@ -153,6 +153,7 @@ jobs: with: check_name: "Test Report ${{ matrix.runner }}" report_paths: ui-tests.xml + check_retries: true - name: Upload logs when workflow failed uses: actions/upload-artifact@v4 diff --git a/.github/workflows/ui_tests.yml b/.github/workflows/ui_tests.yml index 611b105a5a..c041fec09e 100644 --- a/.github/workflows/ui_tests.yml +++ b/.github/workflows/ui_tests.yml @@ -64,6 +64,9 @@ jobs: - name: Set up fastlane run: bundle install + + - name: Create Default Keychain + run: bundle exec fastlane create_keychain_ui_tests - name: Sync code signing assets env: @@ -141,6 +144,7 @@ jobs: with: check_name: "Test Report ${{ matrix.runner }}" report_paths: ui-tests.xml + check_retries: true - name: Upload logs when workflow failed uses: actions/upload-artifact@v4 diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 89b3f30f74..2989d7f8b3 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2809,6 +2809,7 @@ BB470EBC2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB470EBA2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift */; }; BB5789722B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */; }; BB5F46A32C8751F6005F72DF /* BookmarkSortTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB5F46A22C8751F6005F72DF /* BookmarkSortTests.swift */; }; + BB731F312CDBA6360023D2E4 /* FireWindowTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB731F302CDBA6320023D2E4 /* FireWindowTests.swift */; }; BB7B5F982C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; BB7B5F992C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift in Sources */ = {isa = PBXBuildFile; fileRef = BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */; }; BBB881882C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift in Sources */ = {isa = PBXBuildFile; fileRef = BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */; }; @@ -4760,6 +4761,7 @@ BB470EBA2C5A66D6002EE91D /* BookmarkManagementDetailViewModel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkManagementDetailViewModel.swift; sourceTree = ""; }; BB5789712B2CA70F0009DFE2 /* DataBrokerProtectionSubscriptionEventHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataBrokerProtectionSubscriptionEventHandler.swift; sourceTree = ""; }; BB5F46A22C8751F6005F72DF /* BookmarkSortTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkSortTests.swift; sourceTree = ""; }; + BB731F302CDBA6320023D2E4 /* FireWindowTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FireWindowTests.swift; sourceTree = ""; }; BB7B5F972C4ED73800BA4AF8 /* BookmarksSearchAndSortMetrics.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarksSearchAndSortMetrics.swift; sourceTree = ""; }; BBB881872C4029BA001247C6 /* BookmarkListTreeControllerSearchDataSource.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkListTreeControllerSearchDataSource.swift; sourceTree = ""; }; BBBB653F2C77BB9400E69AC6 /* BookmarkSearchTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BookmarkSearchTests.swift; sourceTree = ""; }; @@ -6972,6 +6974,7 @@ 7B4CE8DB26F02108009134B1 /* UITests */ = { isa = PBXGroup; children = ( + BB731F302CDBA6320023D2E4 /* FireWindowTests.swift */, 376E708D2BD686260082B7EB /* UI Tests.xctestplan */, EEBCE6802BA444FA00B9DF00 /* Common */, EEC7BE2D2BC6C09400F86835 /* AddressBarKeyboardShortcutsTests.swift */, @@ -12628,6 +12631,7 @@ EEC7BE2E2BC6C09500F86835 /* AddressBarKeyboardShortcutsTests.swift in Sources */, EE54F7B32BBFEA49006218DB /* BookmarksAndFavoritesTests.swift in Sources */, EE02D4222BB4611A00DBE6B3 /* TestsURLExtension.swift in Sources */, + BB731F312CDBA6360023D2E4 /* FireWindowTests.swift in Sources */, EE42CBCC2BC8004700AD411C /* PermissionsTests.swift in Sources */, 7B4CE8E726F02135009134B1 /* TabBarTests.swift in Sources */, EEBCE6832BA463DD00B9DF00 /* NSImageExtensions.swift in Sources */, diff --git a/UITests/FireWindowTests.swift b/UITests/FireWindowTests.swift new file mode 100644 index 0000000000..bedce50dff --- /dev/null +++ b/UITests/FireWindowTests.swift @@ -0,0 +1,340 @@ +// +// FireWindowTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest + +class FireWindowTests: XCTestCase { + private var app: XCUIApplication! + private var settingsGeneralButton: XCUIElement! + private var reopenAllWindowsFromLastSessionPreference: XCUIElement! + + override class func setUp() { + UITests.firstRun() + } + + override func setUpWithError() throws { + continueAfterFailure = false + app = XCUIApplication() + app.launchEnvironment["UITEST_MODE"] = "1" + + settingsGeneralButton = app.buttons["PreferencesSidebar.generalButton"] + reopenAllWindowsFromLastSessionPreference = app.radioButtons["PreferencesGeneralView.stateRestorePicker.reopenAllWindowsFromLastSession"] + + app.launch() + app.typeKey("w", modifierFlags: [.command, .option, .shift]) // Let's enforce a single window + } + + func testFireWindowDoesNotStoreHistory() { + openFireWindow() + openSite(pageTitle: "Some site") + openNormalWindow() + assertSiteIsNotShowingInNormalWindowHistory() + } + + func testFireWindowStateIsNotSavedAfterRestart() { + openNormalWindow() + app.typeKey(",", modifierFlags: [.command]) // Open settings + settingsGeneralButton.click(forDuration: 0.5, thenDragTo: settingsGeneralButton) + reopenAllWindowsFromLastSessionPreference.clickAfterExistenceTestSucceeds() + + openThreeSitesOnNormalWindow() + openFireWindow() + openThreeSitesOnFireWindow() + + app.terminate() + app.launch() + + assertSitesOpenedInNormalWindowAreRestored() + assertSitesOpenedOnFireWindowAreNotRestored() + } + + func testFireWindowDoNotShowPinnedTabs() { + openNormalWindow() + openSite(pageTitle: "Page #1") + app.menuItems["Pin Tab"].tap() + + app.openNewTab() + openSite(pageTitle: "Page #2") + app.menuItems["Pin Tab"].tap() + + openFireWindow() + assertFireWindowDoesNotHavePinnedTabs() + } + + func testFireWindowTabsCannotBeDragged() { + openFireWindow() + openSite(pageTitle: "Page #1") + + app.openNewTab() + openSite(pageTitle: "Page #2") + + dragFirstTabOutsideOfFireWindow() + + /// Assert that Page #1 is still on the fire window after the drag + app.typeKey("]", modifierFlags: [.command, .shift]) + XCTAssertTrue(app.staticTexts["Sample text for Page #2"].exists) + app.typeKey("[", modifierFlags: [.command, .shift]) + XCTAssertTrue(app.staticTexts["Sample text for Page #1"].exists) + } + + func testFireWindowsSignInDoesNotShowCredentialsPopup() { + openFireWindow() + hoverMouseOutsideTabSoPreviewIsNotShown() + openSignUpSite() + fillCredentials() + finishSignUp() + assertSavePasswordPopupIsNotShown() + } + + func testCrendentialsAreAutoFilledInFireWindows() { + openNormalWindow() + hoverMouseOutsideTabSoPreviewIsNotShown() + openLoginSite() + signIn() + saveCredentials() + + /// Here we start the same flow but in the fire window, but we use the autofill credentials saved in the step before. + openFireWindow() + hoverMouseOutsideTabSoPreviewIsNotShown() + openLoginSite() + signInUsingAutoFill() + } + + // MARK: - Utilities + + private func hoverMouseOutsideTabSoPreviewIsNotShown() { + let window = app.windows.firstMatch + let coordinate = window.coordinate(withNormalizedOffset: CGVector(dx: -100, dy: -100)) + coordinate.hover() + } + + private func signInUsingAutoFill() { + if areTestsRunningOnMacos13() { + let webViewFire = app.webViews.firstMatch + let webViewCoordinate = webViewFire.coordinate(withNormalizedOffset: CGVector(dx: 5, dy: 5)) + webViewCoordinate.tap() + app.typeKey("\t", modifierFlags: []) + sleep(1) + let autoFillPopup = webViewFire.buttons["test@duck.com privacy-test-pages.site"] + let coordinate = autoFillPopup.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) + coordinate.tap() + + /// On macOS 13 there are some issues when accessing web view elements so we do not check the value of the email text field. + /// If we can access the `test@duck.com privacy-test-pages.site` button means that auto fill is working correctly in the fire window. + /// Checking that the email is being filled correctly is more an autofill test that fire window, so we are okay to skip it. + /// + /// We do run this test on macOS 14 and above. + } else { + let webViewFire = app.webViews.firstMatch + webViewFire.tap() + let emailTextFieldFire = webViewFire.textFields["Email"].firstMatch + emailTextFieldFire.click() + let autoFillPopup = webViewFire.buttons["test@duck.com privacy-test-pages.site"] + let coordinate = autoFillPopup.coordinate(withNormalizedOffset: CGVector(dx: 0.5, dy: 0.5)) + coordinate.tap() + + XCTAssertEqual(emailTextFieldFire.value as? String, "test@duck.com") + } + } + + private func saveCredentials() { + let saveButton = app.buttons["Save"] + saveButton.tap() + } + + private func signIn() { + if areTestsRunningOnMacos13() { + let webView = app.webViews.firstMatch + let webViewCoordinate = webView.coordinate(withNormalizedOffset: CGVector(dx: 5, dy: 5)) + webViewCoordinate.tap() + app.typeKey("\t", modifierFlags: []) + app.typeText("test@duck.com") + app.typeKey("\t", modifierFlags: []) + app.typeText("pa$$word") + } else { + let webView = app.webViews.firstMatch + webView.tap() + let emailTextField = webView.textFields["Email"].firstMatch + emailTextField.click() + emailTextField.typeText("test@duck.com") + app.typeKey("\t", modifierFlags: []) + app.typeText("pa$$word") + } + + let signInButton = app.webViews.firstMatch.buttons["Sign in"].firstMatch + signInButton.click() + } + + private func openLoginSite() { + let addressBarTextField = app.windows.firstMatch.textFields["AddressBarViewController.addressBarTextField"].firstMatch + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(URL(string: "https://privacy-test-pages.site/autofill/autoprompt/1-standard-login-form.html")!) + XCTAssertTrue( + app.windows.firstMatch.webViews["Autofill autoprompt for signin forms"].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Visited site didn't load with the expected title in a reasonable timeframe." + ) + } + + private func assertSavePasswordPopupIsNotShown() { + let credentialsPopup = app.popovers["Save password in DuckDuckGo?"] + XCTAssertFalse(credentialsPopup.exists) + } + + private func finishSignUp() { + let signUpButton = app.webViews.firstMatch.buttons["Sign up"].firstMatch + signUpButton.click() + } + + private func fillCredentials() { + if areTestsRunningOnMacos13() { + /// On macOS 13 we tap in the webview coordinate and we use tabs to make it work given that it doesn't find web view elements + let webView = app.webViews.firstMatch + let webViewCoordinate = webView.coordinate(withNormalizedOffset: CGVector(dx: 5, dy: 5)) + webViewCoordinate.tap() + app.typeKey("\t", modifierFlags: []) + app.typeText("test@duck.com") + app.typeKey("\t", modifierFlags: []) + app.typeText("pa$$word") + app.typeKey("\t", modifierFlags: []) + app.typeText("pa$$word") + } else { + let webView = app.webViews.firstMatch + webView.tap() + let emailTextField = webView.textFields["Email"].firstMatch + emailTextField.click() + emailTextField.typeText("test@duck.com") + + let password = webView.secureTextFields["Password"].firstMatch + password.click() + password.typeText("pa$$word") + app.typeKey("\t", modifierFlags: []) + app.typeText("pa$$word") + } + } + + private func openSignUpSite() { + let addressBarTextField = app.windows.firstMatch.textFields["AddressBarViewController.addressBarTextField"].firstMatch + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(URL(string: "https://privacy-test-pages.site/autofill/signup.html")!) + XCTAssertTrue( + app.windows.firstMatch.webViews["Password generation during signup"].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Visited site didn't load with the expected title in a reasonable timeframe." + ) + } + + private func dragFirstTabOutsideOfFireWindow() { + let toolbar = app.toolbars.firstMatch + let toolbarCoordinate = toolbar.coordinate(withNormalizedOffset: CGVector(dx: 0, dy: 0)) + let startPoint = toolbarCoordinate.withOffset(CGVector(dx: 120, dy: 15)) + let endPoint = toolbarCoordinate.withOffset(CGVector(dx: -100, dy: -100)) + startPoint.press(forDuration: 0.5, thenDragTo: endPoint) + } + + private func assertFireWindowDoesNotHavePinnedTabs() { + let existsPredicate = NSPredicate(format: "exists == true") + let staticTextExistsExpectation = expectation(for: existsPredicate, evaluatedWith: app.windows.firstMatch.staticTexts.element(boundBy: 0), handler: nil) + + // Wait up to 10 seconds for the static texts to be available + let result = XCTWaiter().wait(for: [staticTextExistsExpectation], timeout: 10) + XCTAssertEqual(result, .completed, "No static texts were found in the app") + + // After confirming static texts are available, iterate through them + for staticText in app.staticTexts.allElementsBoundByIndex where staticText.exists { + XCTAssertFalse(staticText.label.contains("Page #1"), "Unwanted string found in static text: \(staticText.label)") + XCTAssertFalse(staticText.label.contains("Page #2"), "Unwanted string found in static text: \(staticText.label)") + } + } + + private func assertSitesOpenedInNormalWindowAreRestored() { + XCTAssertTrue(app.staticTexts["Sample text for Page #3"].waitForExistence(timeout: UITests.Timeouts.elementExistence), "Page #3 should exist.") + app.typeKey("[", modifierFlags: [.command, .shift]) + XCTAssertTrue(app.staticTexts["Sample text for Page #2"].waitForExistence(timeout: UITests.Timeouts.elementExistence), "Page #2 should exist.") + app.typeKey("[", modifierFlags: [.command, .shift]) + XCTAssertTrue(app.staticTexts["Sample text for Page #1"].waitForExistence(timeout: UITests.Timeouts.elementExistence), "Page #1 should exist.") + } + + private func assertSitesOpenedOnFireWindowAreNotRestored() { + let existsPredicate = NSPredicate(format: "exists == true") + let staticTextExistsExpectation = expectation(for: existsPredicate, evaluatedWith: app.staticTexts.element(boundBy: 0), handler: nil) + + // Wait up to 10 seconds for the static texts to be available + let result = XCTWaiter().wait(for: [staticTextExistsExpectation], timeout: 10) + XCTAssertEqual(result, .completed, "No static texts were found in the app") + + // After confirming static texts are available, iterate through them + for staticText in app.staticTexts.allElementsBoundByIndex where staticText.exists { + XCTAssertFalse(staticText.label.contains("Page #4"), "Unwanted string found in static text: \(staticText.label)") + XCTAssertFalse(staticText.label.contains("Page #5"), "Unwanted string found in static text: \(staticText.label)") + XCTAssertFalse(staticText.label.contains("Page #6"), "Unwanted string found in static text: \(staticText.label)") + } + } + + private func openThreeSitesOnNormalWindow() { + app.openNewTab() + openSite(pageTitle: "Page #1") + app.openNewTab() + openSite(pageTitle: "Page #2") + app.openNewTab() + openSite(pageTitle: "Page #3") + } + + private func openThreeSitesOnFireWindow() { + openSite(pageTitle: "Page #4") + app.openNewTab() + openSite(pageTitle: "Page #5") + app.openNewTab() + openSite(pageTitle: "Page #6") + } + + private func assertSiteIsNotShowingInNormalWindowHistory() { + let siteMenuItemInHistory = app.menuItems["Some site"] + XCTAssertFalse(siteMenuItemInHistory.exists, "Menu item should not exist because it was not stored in history.") + } + + private func openFireWindow() { + app.typeKey("n", modifierFlags: [.command, .shift]) + } + + private func openNormalWindow() { + app.typeKey("n", modifierFlags: .command) + } + + private func openSite(pageTitle: String) { + let url = UITests.simpleServedPage(titled: pageTitle) + let addressBarTextField = app.windows.firstMatch.textFields["AddressBarViewController.addressBarTextField"].firstMatch + XCTAssertTrue( + addressBarTextField.waitForExistence(timeout: UITests.Timeouts.elementExistence), + "The address bar text field didn't become available in a reasonable timeframe." + ) + addressBarTextField.typeURL(url) + XCTAssertTrue( + app.windows.firstMatch.webViews[pageTitle].waitForExistence(timeout: UITests.Timeouts.elementExistence), + "Visited site didn't load with the expected title in a reasonable timeframe." + ) + } + + private func areTestsRunningOnMacos13() -> Bool { + return ProcessInfo.processInfo.operatingSystemVersion.majorVersion == 13 + } +} diff --git a/fastlane/Fastfile b/fastlane/Fastfile index 61158943f5..0b92d927dd 100644 --- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -341,6 +341,18 @@ platform :mac do ) end + desc 'Creates a new Kechain to use on UI tests' + lane :create_keychain_ui_tests do |options| + create_keychain( + name: "DefaultKeychain", + password: "default", + default_keychain: true, + unlock: true, + timeout: 54000, + lock_when_sleeps: false + ) + end + ################################################# # Helper functions ################################################# From 6fe2acde9c5e8d2a629ed9a3bbebbb37b739ee31 Mon Sep 17 00:00:00 2001 From: Anh Do <18567+quanganhdo@users.noreply.github.com> Date: Wed, 13 Nov 2024 21:16:08 -0500 Subject: [PATCH 04/13] Enforce stricter hasPendingUpdate check (#3541) Task/Issue URL: https://app.asana.com/0/1201048563534612/1208746324713624/f Tech Design URL: CC: **Description**: Fixes Install Now menu item does nothing by making sure we can resume the update process. As a bonus, this fixes an edge case where the user toggles back and forth between automatic & manual update checkboxes and that causes the upgrade process to go weird. **Steps to test this PR**: 1. Use this build: https://staticcdn.duckduckgo.com/macos-desktop-browser/a81f9679-e41d-404d-81a6-fd4a461d82f7/testbuilds/3447618/duckduckgo-1.114.0.305.dmg 1. Test different scenarios (automatic/manual update, trigger the update from different entry points, etc) **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo/Updates/UpdateController.swift | 2 +- DuckDuckGo/Updates/UpdateUserDriver.swift | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/Updates/UpdateController.swift b/DuckDuckGo/Updates/UpdateController.swift index f044522f63..e45a117790 100644 --- a/DuckDuckGo/Updates/UpdateController.swift +++ b/DuckDuckGo/Updates/UpdateController.swift @@ -71,7 +71,7 @@ final class UpdateController: NSObject, UpdateControllerProtocol { didSet { if let cachedUpdateResult { latestUpdate = Update(appcastItem: cachedUpdateResult.item, isInstalled: cachedUpdateResult.isInstalled) - hasPendingUpdate = latestUpdate?.isInstalled == false && updateProgress.isIdle + hasPendingUpdate = latestUpdate?.isInstalled == false && updateProgress.isDone && userDriver?.isResumable == true needsNotificationDot = hasPendingUpdate } showUpdateNotificationIfNeeded() diff --git a/DuckDuckGo/Updates/UpdateUserDriver.swift b/DuckDuckGo/Updates/UpdateUserDriver.swift index 960825f46e..15f4f9ed94 100644 --- a/DuckDuckGo/Updates/UpdateUserDriver.swift +++ b/DuckDuckGo/Updates/UpdateUserDriver.swift @@ -86,15 +86,18 @@ final class UpdateUserDriver: NSObject, SPUUserDriver { private var internalUserDecider: InternalUserDecider private var checkpoint: Checkpoint - private var onResuming: () -> Void = {} - + private var onResuming: (() -> Void)? private var onSkipping: () -> Void = {} + var isResumable: Bool { + onResuming != nil + } + private var bytesToDownload: UInt64 = 0 private var bytesDownloaded: UInt64 = 0 @Published var updateProgress = UpdateCycleProgress.default - public var updateProgressPublisher: Published.Publisher { $updateProgress } + var updateProgressPublisher: Published.Publisher { $updateProgress } init(internalUserDecider: InternalUserDecider, areAutomaticUpdatesEnabled: Bool) { @@ -103,7 +106,7 @@ final class UpdateUserDriver: NSObject, SPUUserDriver { } func resume() { - onResuming() + onResuming?() } func cancelAndDismissCurrentUpdate() { From f946f075dbbcb9016085f0ae7b23a5cf5aff1a13 Mon Sep 17 00:00:00 2001 From: Dax the Duck Date: Thu, 14 Nov 2024 06:23:34 +0000 Subject: [PATCH 05/13] Bump version to 1.114.0 (306) --- Configuration/BuildNumber.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index 60e2adb5be..b8206ee1bd 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 305 +CURRENT_PROJECT_VERSION = 306 From ff9eacd0784d2a933b6d262170dbb2190f430f79 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Thu, 14 Nov 2024 10:01:04 +0100 Subject: [PATCH 06/13] DuckPlayer Overlay Pixels (#3538) Task/Issue URL: https://app.asana.com/0/1204099484721401/1208739766555303/f **Description**: - Adds temporary Overlay usage pixels - This uses the same approach we use on iOS: https://github.com/duckduckgo/iOS/pull/3565 --- DuckDuckGo.xcodeproj/project.pbxproj | 12 ++ DuckDuckGo/InfoPlist.xcstrings | 110 +++++++-------- DuckDuckGo/Statistics/GeneralPixel.swift | 24 ++++ .../DuckPlayerTabExtension.swift | 25 +++- .../DuckPlayerOverlayPixels.swift | 99 +++++++++++++ .../DuckPlayerOverlayPixelsTests.swift | 133 ++++++++++++++++++ 6 files changed, 346 insertions(+), 57 deletions(-) create mode 100644 DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift create mode 100644 UnitTests/YoutubePlayer/DuckPlayerOverlayPixelsTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 2989d7f8b3..fe5bf4d2da 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -3001,6 +3001,10 @@ D64A5FF92AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */; }; D6BC8AC62C5A95AA0025375B /* DuckPlayer in Frameworks */ = {isa = PBXBuildFile; productRef = D6BC8AC52C5A95AA0025375B /* DuckPlayer */; }; D6BC8AC82C5A95B10025375B /* DuckPlayer in Frameworks */ = {isa = PBXBuildFile; productRef = D6BC8AC72C5A95B10025375B /* DuckPlayer */; }; + D6E0ACB12CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */; }; + D6E0ACB22CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */; }; + D6E0ACB42CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */; }; + D6E0ACB52CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */; }; EA0BA3A9272217E6002A0B6C /* ClickToLoadUserScript.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA0BA3A8272217E6002A0B6C /* ClickToLoadUserScript.swift */; }; EA18D1CA272F0DC8006DC101 /* social_images in Resources */ = {isa = PBXBuildFile; fileRef = EA18D1C9272F0DC8006DC101 /* social_images */; }; EA8AE76A279FBDB20078943E /* ClickToLoadTDSTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = EA8AE769279FBDB20078943E /* ClickToLoadTDSTests.swift */; }; @@ -4848,6 +4852,8 @@ CDE248A62C821FFE00F9399D /* filterSet.json */ = {isa = PBXFileReference; lastKnownFileType = text.json; path = filterSet.json; sourceTree = ""; }; CDE248A72C821FFE00F9399D /* PhishingDetectionStateManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PhishingDetectionStateManager.swift; sourceTree = ""; }; D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = HomeButtonMenuFactory.swift; sourceTree = ""; }; + D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixels.swift; sourceTree = ""; }; + D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerOverlayPixelsTests.swift; sourceTree = ""; }; EA0BA3A8272217E6002A0B6C /* ClickToLoadUserScript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ClickToLoadUserScript.swift; sourceTree = ""; }; EA18D1C9272F0DC8006DC101 /* social_images */ = {isa = PBXFileReference; lastKnownFileType = folder; path = social_images; sourceTree = ""; }; EA8AE769279FBDB20078943E /* ClickToLoadTDSTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ClickToLoadTDSTests.swift; sourceTree = ""; }; @@ -5619,6 +5625,7 @@ 315AA06F28CA5CC800200030 /* YoutubePlayerNavigationHandler.swift */, 31F28C4C28C8EEC500119F70 /* YoutubePlayerUserScript.swift */, 31F28C4E28C8EEC500119F70 /* YoutubeOverlayUserScript.swift */, + D6E0ACB02CE36DC4005D3486 /* DuckPlayerOverlayPixels.swift */, ); path = YoutubePlayer; sourceTree = ""; @@ -5710,6 +5717,7 @@ 376718FE28E58504003A2A15 /* YoutubePlayer */ = { isa = PBXGroup; children = ( + D6E0ACB32CE36FA8005D3486 /* DuckPlayerOverlayPixelsTests.swift */, 3199AF812C80736B003AEBDC /* DuckPlayerOnboardingLocationValidatorTests.swift */, 3714B1E828EDBAAB0056C57A /* DuckPlayerTests.swift */, 567DA94429E95C3F008AC5EE /* YoutubeOverlayUserScriptTests.swift */, @@ -11336,6 +11344,7 @@ 3706FB2F293F65D500E42796 /* HomePageRecentlyVisitedModel.swift in Sources */, C1935A152C88F958001AD72D /* SyncPromoView.swift in Sources */, C126B35B2C820924005DC2A3 /* FreemiumDebugMenu.swift in Sources */, + D6E0ACB22CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */, 3707C718294B5D0F00682A9F /* AdClickAttributionTabExtension.swift in Sources */, 31EF1E812B63FFB800E6DB17 /* DataBrokerProtectionManager.swift in Sources */, 3706FEBA293F6EFF00E42796 /* BWStatus.swift in Sources */, @@ -12158,6 +12167,7 @@ 3706FE27293F661700E42796 /* AppPrivacyConfigurationTests.swift in Sources */, B626A7652992506A00053070 /* SerpHeadersNavigationResponderTests.swift in Sources */, 9F6434712BECBA2800D2D8A0 /* SubscriptionRedirectManagerTests.swift in Sources */, + D6E0ACB52CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */, 9F26060C2B85C20B00819292 /* AddEditBookmarkDialogViewModelTests.swift in Sources */, 567A23DF2C89980A0010F66C /* OnboardingNavigationDelegateTests.swift in Sources */, 562532A12BC069190034D316 /* ZoomPopoverViewModelTests.swift in Sources */, @@ -13372,6 +13382,7 @@ EE66666F2B56EDE4001D898D /* VPNLocationsHostingViewController.swift in Sources */, 37CC53EC27E8A4D10028713D /* PreferencesDataClearingView.swift in Sources */, 4B0135CE2729F1AA00D54834 /* NSPasteboardExtension.swift in Sources */, + D6E0ACB12CE36DCA005D3486 /* DuckPlayerOverlayPixels.swift in Sources */, 85707F31276A7DCA00DC0649 /* OnboardingViewModel.swift in Sources */, 85AC3B0525D6B1D800C7D2AA /* ScriptSourceProviding.swift in Sources */, 848648A12C76F4B20082282D /* BookmarksBarMenuViewController.swift in Sources */, @@ -13701,6 +13712,7 @@ B630E7FE29C887ED00363609 /* NSErrorAdditionalInfo.swift in Sources */, 370270C02C78EB13002E44E4 /* HomePageSettingsModelTests.swift in Sources */, 4B9292BB2667103100AD2C21 /* BookmarkNodeTests.swift in Sources */, + D6E0ACB42CE36FB0005D3486 /* DuckPlayerOverlayPixelsTests.swift in Sources */, 4B0219A825E0646500ED7DEA /* WebsiteDataStoreTests.swift in Sources */, AAC9C01E24CB6BEB00AD1325 /* TabCollectionViewModelTests.swift in Sources */, 56CE77612C7DFCF800AC1ED2 /* OnboardingSuggestedSearchesProviderTests.swift in Sources */, diff --git a/DuckDuckGo/InfoPlist.xcstrings b/DuckDuckGo/InfoPlist.xcstrings index ef75ed805d..42ce7f4c7f 100644 --- a/DuckDuckGo/InfoPlist.xcstrings +++ b/DuckDuckGo/InfoPlist.xcstrings @@ -67,55 +67,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Lade Fotos und Videos hoch" } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to upload photographs and videos" + "value" : "Lets websites ask permission to use your camera for video calls, recording and uploading videos, or taking and uploading photos." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite subir fotos y vídeos" } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Vous permet de télécharger des photos et des vidéos" } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente il caricamento di foto e video" } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je foto's en video's uploaden" } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia przesyłanie zdjęć i filmów" } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite-lhe carregar fotografias e vídeos" } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет загружать фото и видео" } } @@ -127,55 +127,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Speichere heruntergeladene Dateien in diesem Ordner." } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to save downloaded files to this folder." + "value" : "Lets you save files to the Desktop folder on your computer." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite guardar los archivos descargados en esta carpeta." } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permet d'enregistrer les fichiers téléchargés dans ce dossier." } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente di salvare in questa cartella i file scaricati." } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je gedownloade bestanden opslaan in deze map." } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia zapisywanie pobranych plików w tym folderze." } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite guardar ficheiros transferidos nesta pasta." } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет сохранять загруженные файлы в эту папку." } } @@ -187,55 +187,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Speichere heruntergeladene Dateien in diesem Ordner." } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to save downloaded files to this folder." + "value" : "Lets you save files to the Documents folder on your computer." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite guardar los archivos descargados en esta carpeta." } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permet d'enregistrer les fichiers téléchargés dans ce dossier." } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente di salvare in questa cartella i file scaricati." } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je gedownloade bestanden opslaan in deze map." } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia zapisywanie pobranych plików w tym folderze." } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite guardar ficheiros transferidos nesta pasta." } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет сохранять загруженные файлы в эту папку." } } @@ -247,55 +247,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Speichere heruntergeladene Dateien in diesem Ordner." } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to save downloaded files to this folder." + "value" : "Lets you save files to the Downloads folder on your computer." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite guardar los archivos descargados en esta carpeta." } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permet d'enregistrer les fichiers téléchargés dans ce dossier." } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente di salvare in questa cartella i file scaricati." } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je gedownloade bestanden opslaan in deze map." } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia zapisywanie pobranych plików w tym folderze." } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite guardar ficheiros transferidos nesta pasta." } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет сохранять загруженные файлы в эту папку." } } @@ -367,55 +367,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Teile deinen Standort" } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to share your geolocation" + "value" : "Lets websites ask permission to use your location to improve search results, provide weather and directions, or show nearby map locations." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite compartir tu ubicación" } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Vous permet de partager votre géolocalisation" } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente la condivisione della tua geolocalizzazione" } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je je geolocatie delen" } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia udostępnianie geolokalizacji" } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite-te partilhar a tua localização geográfica" } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет делиться геопозицией" } } @@ -423,7 +423,7 @@ }, "NSLocationWhenInUseUsageDescription" : { "comment" : "Privacy - Location When In Use Usage Description", - "extractionState" : "extracted_with_value", + "extractionState" : "stale", "localizations" : { "de" : { "stringUnit" : { @@ -487,55 +487,55 @@ "localizations" : { "de" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Teile deine Aufzeichnungen" } }, "en" : { "stringUnit" : { "state" : "new", - "value" : "Allows you to share recordings" + "value" : "Lets websites ask permission to use your microphone for audio and video calls or for recording and uploading audio." } }, "es" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Te permite compartir grabaciones" } }, "fr" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Vous permet de partager des enregistrements" } }, "it" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Consente la condivisione delle registrazioni" } }, "nl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Hiermee kun je opnames delen" } }, "pl" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Umożliwia udostępnianie nagrań" } }, "pt" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Permite-lhe partilhar gravações" } }, "ru" : { "stringUnit" : { - "state" : "translated", + "state" : "needs_review", "value" : "Позволяет делиться аудиозаписями" } } diff --git a/DuckDuckGo/Statistics/GeneralPixel.swift b/DuckDuckGo/Statistics/GeneralPixel.swift index 095bc239c0..2297918c6e 100644 --- a/DuckDuckGo/Statistics/GeneralPixel.swift +++ b/DuckDuckGo/Statistics/GeneralPixel.swift @@ -136,6 +136,14 @@ enum GeneralPixel: PixelKitEventV2 { case duckPlayerContingencySettingsDisplayed case duckPlayerContingencyLearnMoreClicked + // Temporary Overlay Pixels + case duckPlayerYouTubeOverlayNavigationBack + case duckPlayerYouTubeOverlayNavigationRefresh + case duckPlayerYouTubeNavigationWithinYouTube + case duckPlayerYouTubeOverlayNavigationOutsideYoutube + case duckPlayerYouTubeOverlayNavigationClosed + case duckPlayerYouTubeNavigationIdle30 + // Dashboard case dashboardProtectionAllowlistAdd(triggerOrigin: String?) case dashboardProtectionAllowlistRemove(triggerOrigin: String?) @@ -659,6 +667,21 @@ enum GeneralPixel: PixelKitEventV2 { return "duckplayer_mac_contingency_settings-displayed" case .duckPlayerContingencyLearnMoreClicked: return "duckplayer_mac_contingency_learn-more-clicked" + + // Duck Player Temporary Overlay Pixels + case .duckPlayerYouTubeOverlayNavigationBack: + return "duckplayer_youtube_overlay_navigation_back" + case .duckPlayerYouTubeOverlayNavigationRefresh: + return "duckplayer_youtube_overlay_navigation_refresh" + case .duckPlayerYouTubeNavigationWithinYouTube: + return "duckplayer_youtube_overlay_navigation_within-youtube" + case .duckPlayerYouTubeOverlayNavigationOutsideYoutube: + return "duckplayer_youtube_overlay_navigation_outside-youtube" + case .duckPlayerYouTubeOverlayNavigationClosed: + return "duckplayer_youtube_overlay_navigation_closed" + case .duckPlayerYouTubeNavigationIdle30: + return "duckplayer_youtube_overlay_idle-30" + case .dashboardProtectionAllowlistAdd: return "mp_wla" case .dashboardProtectionAllowlistRemove: @@ -1101,6 +1124,7 @@ enum GeneralPixel: PixelKitEventV2 { case .pageRefreshThreeTimesWithin20Seconds: return "m_mac_reload-three-times-within-20-seconds" case .siteNotWorkingShown: return "m_mac_site-not-working_shown" case .siteNotWorkingWebsiteIsBroken: return "m_mac_site-not-working_website-is-broken" + } } diff --git a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift index 316f5dde06..85b73b3bc0 100644 --- a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift @@ -55,17 +55,20 @@ final class DuckPlayerTabExtension { private weak var youtubePlayerScript: YoutubePlayerUserScript? private let onboardingDecider: DuckPlayerOnboardingDecider private var shouldSelectNextNewTab: Bool? + private var duckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring init(duckPlayer: DuckPlayer, isBurner: Bool, scriptsPublisher: some Publisher, webViewPublisher: some Publisher, preferences: DuckPlayerPreferences = .shared, - onboardingDecider: DuckPlayerOnboardingDecider) { + onboardingDecider: DuckPlayerOnboardingDecider, + duckPlayerOverlayPixels: DuckPlayerOverlayPixelFiring = DuckPlayerOverlayUsagePixels()) { self.duckPlayer = duckPlayer self.isBurner = isBurner self.preferences = preferences self.onboardingDecider = onboardingDecider + self.duckPlayerOverlayUsagePixels = duckPlayerOverlayPixels webViewPublisher.sink { [weak self] webView in self?.webView = webView }.store(in: &cancellables) @@ -179,7 +182,7 @@ extension DuckPlayerTabExtension: NewWindowPolicyDecisionMaker { } extension DuckPlayerTabExtension: NavigationResponder { - + // swiftlint:disable cyclomatic_complexity @MainActor func decidePolicy(for navigationAction: NavigationAction, preferences: inout NavigationPreferences) async -> NavigationActionPolicy? { // only proceed when Private Player is enabled @@ -206,6 +209,18 @@ extension DuckPlayerTabExtension: NavigationResponder { } } + // Fire DuckPlayer Temporary Pixels on Reload + if case .reload = navigationAction.navigationType { + if let url = navigationAction.request.url { + duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) + } + } + + // Fire DuckPlayer temporary pixels on navigating outside Youtube + if let url = navigationAction.request.url, !url.isYoutube { + duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) + } + // when in Private Player, don't directly reload current URL when it‘s a Private Player target URL if case .reload = navigationAction.navigationType, navigationAction.url.isDuckPlayer { @@ -243,6 +258,7 @@ extension DuckPlayerTabExtension: NavigationResponder { // Navigating to a Youtube URL return handleYoutubeNavigation(for: navigationAction) } + // swiftlint:enable cyclomatic_complexity @MainActor private func handleYoutubeNavigation(for navigationAction: NavigationAction) -> NavigationActionPolicy? { @@ -271,6 +287,11 @@ extension DuckPlayerTabExtension: NavigationResponder { webView.goBack() webView.load(URLRequest(url: .duckPlayer(videoID, timestamp: timestamp))) } + + // Fire DuckPlayer Overlay Temporary Pixels + if let url = navigation.request.url { + duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) + } } @MainActor diff --git a/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift new file mode 100644 index 0000000000..8988465794 --- /dev/null +++ b/DuckDuckGo/YoutubePlayer/DuckPlayerOverlayPixels.swift @@ -0,0 +1,99 @@ +// +// DuckPlayerOverlayPixels.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +import PixelKit + +protocol DuckPlayerOverlayPixelFiring { + + var pixelFiring: PixelFiring? { get set } + var navigationHistory: [URL] { get set } + + func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode) +} + +final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring { + + var pixelFiring: PixelFiring? + var navigationHistory: [URL] = [] + + private var idleTimer: Timer? + private var idleTimeInterval: TimeInterval + + init(pixelFiring: PixelFiring? = PixelKit.shared, + navigationHistory: [URL] = [], + timeoutInterval: TimeInterval = 30.0) { + self.pixelFiring = pixelFiring + self.idleTimeInterval = timeoutInterval + } + + func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode) { + guard let url = url else { return } + let comparisonURL = url.forComparison() + + // Only append the URL if it's different from the last entry in normalized form + navigationHistory.append(comparisonURL) + + // DuckPlayer is in Ask Mode, there's navigation history, and last URL is a YouTube Watch Video + guard duckPlayerMode == .alwaysAsk, + navigationHistory.count > 1, + let currentURL = navigationHistory.last, + let previousURL = navigationHistory.dropLast().last, + previousURL.isYoutubeWatch else { return } + + var isReload = false + // Check for a reload condition: when current videoID is the same as Previous + if let currentVideoID = currentURL.youtubeVideoParams?.videoID, + let previousVideoID = previousURL.youtubeVideoParams?.videoID, + !previousURL.isDuckPlayer, !currentURL.isDuckPlayer { + isReload = currentVideoID == previousVideoID + } + + // Fire the reload pixel if this is a reload navigation + if isReload { + pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeOverlayNavigationRefresh) + } else { + // Determine if it’s a back navigation by looking further back in history + let isBackNavigation = navigationHistory.count > 2 && + navigationHistory[navigationHistory.count - 3].forComparison() == currentURL.forComparison() + + // Fire the appropriate pixel based on navigation type + if isBackNavigation { + pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeOverlayNavigationBack) + } else if previousURL.isYoutubeWatch && currentURL.isYoutube { + // Forward navigation within YouTube (including non-video URLs) + pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeNavigationWithinYouTube) + } else if previousURL.isYoutubeWatch && !currentURL.isYoutube && !currentURL.isDuckPlayer { + // Navigation outside YouTube + pixelFiring?.fire(GeneralPixel.duckPlayerYouTubeOverlayNavigationOutsideYoutube) + navigationHistory.removeAll() + } + } + + // Truncation logic: Remove all URLs up to the last occurrence of the current URL in normalized form + if navigationHistory.count > 0 { + if let lastOccurrenceIndex = (0.. Date: Thu, 14 Nov 2024 12:12:11 +0100 Subject: [PATCH 07/13] Send error with fetch device failure pixel (#3546) Task/Issue URL: https://app.asana.com/0/0/1208738052951188/f **Description**: The pixel `sync_refresh_devices_error` didn't include the error parameter due to confusion about how the slightly newer PixelKit types do this. This PR just includes the error. --- DuckDuckGo/Preferences/Model/SyncPreferences.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/Preferences/Model/SyncPreferences.swift b/DuckDuckGo/Preferences/Model/SyncPreferences.swift index f6be1b3b6c..307fdf4237 100644 --- a/DuckDuckGo/Preferences/Model/SyncPreferences.swift +++ b/DuckDuckGo/Preferences/Model/SyncPreferences.swift @@ -376,7 +376,7 @@ final class SyncPreferences: ObservableObject, SyncUI.ManagementViewModel { let registeredDevices = try await syncService.fetchDevices() mapDevices(registeredDevices) } catch { - PixelKit.fire(DebugEvent(GeneralPixel.syncRefreshDevicesError(error: error))) + PixelKit.fire(DebugEvent(GeneralPixel.syncRefreshDevicesError(error: error), error: error)) Logger.sync.debug("Failed to refresh devices: \(error)") } } From 8b531a5110b2a5a3ee64366dc5527ebec964cb3e Mon Sep 17 00:00:00 2001 From: Graeme Arthur Date: Thu, 14 Nov 2024 12:36:40 +0100 Subject: [PATCH 08/13] Speculative password import prompt crash fix (#3542) Task/Issue URL: https://app.asana.com/0/1203822806345703/1208648341254407/f CC: @dbajpeyi **Description**: There's a crash in moderate numbers which is occurring during the password import prompt flow. This is speculative fix for that. The reason seemed to be that the replyHandler was being called too many times. However: - The docs for `userContentController:didReceiveScriptMessage:replyHandler:` say "The replyHandler can be called at most once. If the replyHandler is deallocated before it is called, the Promise will be rejected with a JavaScript Error object with an appropriate message indicating the handler was never called." - In the case of startCredentialsImportFlow where the crash is occurring, the JS is not awaiting. So there is no need to call the replyHandler. **Steps to test this PR:** Just smoke tests for the feature: https://app.asana.com/0/1142021229838617/1208543152554655/f **Definition of Done**: * [ ] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? --- ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- LocalPackages/DataBrokerProtection/Package.swift | 2 +- LocalPackages/FeatureFlags/Package.swift | 2 +- LocalPackages/NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index fe5bf4d2da..770bbd4d59 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -15066,7 +15066,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 209.1.0; + version = 209.1.2; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index dd2c08b0cc..b81ffcdc95 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "948420e704ea4d9412a4fc3e2c2ab0d5ea5fe5d7", - "version" : "209.1.0" + "revision" : "16a1f44f24fd492a87fc28571cac87022c1b740b", + "version" : "209.1.2" } }, { diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 48b04b904e..f0e8b50ef3 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), .package(path: "../SwiftUIExtensions"), .package(path: "../AppKitExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/FeatureFlags/Package.swift b/LocalPackages/FeatureFlags/Package.swift index 067f18eb00..4664955e62 100644 --- a/LocalPackages/FeatureFlags/Package.swift +++ b/LocalPackages/FeatureFlags/Package.swift @@ -32,7 +32,7 @@ let package = Package( targets: ["FeatureFlags"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), ], targets: [ // Targets are the basic building blocks of a package, defining a module or a test suite. diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index 32b780ee4d..e35805cdc2 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -32,7 +32,7 @@ let package = Package( .library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), .package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"), .package(path: "../AppLauncher"), .package(path: "../UDSHelper"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index f3d08b97a4..ff5e096f9b 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), .package(path: "../SwiftUIExtensions") ], targets: [ From 7bf460d9da22cf9e013d896efcbffc036b1cf7cf Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Thu, 14 Nov 2024 13:12:20 +0100 Subject: [PATCH 09/13] Add support for local overrides for feature flags (#3545) Task/Issue URL: https://app.asana.com/0/72649045549333/1208716221426945/f Tech Design URL: https://app.asana.com/0/481882893211075/1208716218352496/f Description: This change adds support for overriding feature flags locally. FeatureFlagLocalOverrides is instantiated and passed to DefaultFeatureFlagger to allow local overrides. A dedicated submenu within Debug menu was added to handle feature flags state. The menu is dynamically populated and only available to internal users. Local overrides have no effect when default user flag is disabled. Currently only 1 flag uses the local overrides: the new HTML New Tab Page flag. It's a WIP feature and the feature flag currently only presents an empty webview in place of the NTP. --- DuckDuckGo.xcodeproj/project.pbxproj | 8 +- .../xcshareddata/swiftpm/Package.resolved | 6 +- DuckDuckGo/Application/AppDelegate.swift | 8 +- .../WKWebViewConfigurationExtensions.swift | 5 +- .../Utilities/UserDefaultsWrapper.swift | 1 - .../FeatureFlagOverridesMenu.swift | 112 ++++++++++++++++++ DuckDuckGo/Menus/MainMenu.swift | 4 + .../Tab/View/BrowserTabViewController.swift | 33 +++++- .../YoutubePlayer/DuckURLSchemeHandler.swift | 23 ++++ .../PhishingDetectionIntegrationTests.swift | 5 +- .../DataBrokerProtection/Package.swift | 2 +- LocalPackages/FeatureFlags/Package.swift | 2 +- .../Sources/FeatureFlags/FeatureFlag.swift | 25 +++- .../NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- .../DuckSchemeHandlerTests.swift | 17 ++- UnitTests/Menus/MainMenuTests.swift | 5 +- .../ErrorPageTabExtensionTest.swift | 5 +- 18 files changed, 237 insertions(+), 28 deletions(-) create mode 100644 DuckDuckGo/InternalUserDecider/FeatureFlagOverridesMenu.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 770bbd4d59..4c2c971788 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -1269,6 +1269,8 @@ 37F19A6728E1B43200740DC6 /* DuckPlayerPreferences.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F19A6628E1B43200740DC6 /* DuckPlayerPreferences.swift */; }; 37F19A6A28E2F2D000740DC6 /* DuckPlayer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F19A6928E2F2D000740DC6 /* DuckPlayer.swift */; }; 37F44A5F298C17830025E7FE /* Navigation in Frameworks */ = {isa = PBXBuildFile; productRef = 37F44A5E298C17830025E7FE /* Navigation */; }; + 37F8ABD32CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */; }; + 37F8ABD42CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */; }; 37FD78112A29EBD100B36DB1 /* SyncErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */; }; 37FD78122A29EBD100B36DB1 /* SyncErrorHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */; }; 4B0135CE2729F1AA00D54834 /* NSPasteboardExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4B0135CD2729F1AA00D54834 /* NSPasteboardExtension.swift */; }; @@ -3712,6 +3714,7 @@ 37F19A6428E1B3FB00740DC6 /* PreferencesDuckPlayerView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PreferencesDuckPlayerView.swift; sourceTree = ""; }; 37F19A6628E1B43200740DC6 /* DuckPlayerPreferences.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayerPreferences.swift; sourceTree = ""; }; 37F19A6928E2F2D000740DC6 /* DuckPlayer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DuckPlayer.swift; sourceTree = ""; }; + 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeatureFlagOverridesMenu.swift; sourceTree = ""; }; 37FD78102A29EBD100B36DB1 /* SyncErrorHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncErrorHandler.swift; sourceTree = ""; }; 4B0135CD2729F1AA00D54834 /* NSPasteboardExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NSPasteboardExtension.swift; sourceTree = ""; }; 4B02197F25E05FAC00ED7DEA /* FireproofingURLExtensions.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = FireproofingURLExtensions.swift; sourceTree = ""; }; @@ -5302,6 +5305,7 @@ 1D36E651298A84F600AA485D /* InternalUserDecider */ = { isa = PBXGroup; children = ( + 37F8ABD22CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift */, 1D36E657298AA3BA00AA485D /* InternalUserDeciderStore.swift */, ); path = InternalUserDecider; @@ -11304,6 +11308,7 @@ 3706FB13293F65D500E42796 /* Bookmark.swift in Sources */, 3706FB14293F65D500E42796 /* ConnectBitwardenViewModel.swift in Sources */, 3706FB15293F65D500E42796 /* NSNotificationName+DataImport.swift in Sources */, + 37F8ABD42CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */, EE6666702B56EDE4001D898D /* VPNLocationsHostingViewController.swift in Sources */, 3706FB16293F65D500E42796 /* StoredPermission.swift in Sources */, 3706FB17293F65D500E42796 /* FirePopoverCollectionViewHeader.swift in Sources */, @@ -13055,6 +13060,7 @@ 37F19A6A28E2F2D000740DC6 /* DuckPlayer.swift in Sources */, AA5FA69A275F91C700DCE9C9 /* Favicon.swift in Sources */, AABEE69A24A902A90043105B /* SuggestionContainerViewModel.swift in Sources */, + 37F8ABD32CE3EE5B00CB0294 /* FeatureFlagOverridesMenu.swift in Sources */, AA840A9827319D1600E63CDD /* FirePopoverWrapperViewController.swift in Sources */, 4B85A48028821CC500FC4C39 /* NSPasteboardItemExtension.swift in Sources */, 37CD54CA27F2FDD100F1F7B9 /* AutofillPreferencesModel.swift in Sources */, @@ -15066,7 +15072,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 209.1.2; + version = 210.0.0; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index b81ffcdc95..347bb2d949 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "16a1f44f24fd492a87fc28571cac87022c1b740b", - "version" : "209.1.2" + "revision" : "cfb178099738bc6cd0c3a3d73df717420ef4a59f", + "version" : "210.0.0" } }, { @@ -75,7 +75,7 @@ { "identity" : "lottie-spm", "kind" : "remoteSourceControl", - "location" : "https://github.com/airbnb/lottie-spm", + "location" : "https://github.com/airbnb/lottie-spm.git", "state" : { "revision" : "1d29eccc24cc8b75bff9f6804155112c0ffc9605", "version" : "4.4.3" diff --git a/DuckDuckGo/Application/AppDelegate.swift b/DuckDuckGo/Application/AppDelegate.swift index 1556feef05..38ab9f6654 100644 --- a/DuckDuckGo/Application/AppDelegate.swift +++ b/DuckDuckGo/Application/AppDelegate.swift @@ -25,6 +25,7 @@ import Configuration import CoreData import Crashes import DDGSync +import FeatureFlags import History import MetricKit import Networking @@ -264,7 +265,12 @@ final class AppDelegate: NSObject, NSApplicationDelegate { featureFlagger = DefaultFeatureFlagger( internalUserDecider: internalUserDecider, - privacyConfigManager: AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager + privacyConfigManager: AppPrivacyFeatures.shared.contentBlocking.privacyConfigurationManager, + localOverrides: FeatureFlagLocalOverrides( + keyValueStore: UserDefaults.appConfiguration, + actionHandler: FeatureFlagOverridesPublishingHandler() + ), + for: FeatureFlag.self ) onboardingStateMachine = ContextualOnboardingStateMachine() diff --git a/DuckDuckGo/Common/Extensions/WKWebViewConfigurationExtensions.swift b/DuckDuckGo/Common/Extensions/WKWebViewConfigurationExtensions.swift index 9a89e51e4f..5ecc8aac06 100644 --- a/DuckDuckGo/Common/Extensions/WKWebViewConfigurationExtensions.swift +++ b/DuckDuckGo/Common/Extensions/WKWebViewConfigurationExtensions.swift @@ -60,7 +60,10 @@ extension WKWebViewConfiguration { if SupportedOSChecker.isCurrentOSReceivingUpdates { if urlSchemeHandler(forURLScheme: URL.NavigationalScheme.duck.rawValue) == nil { - setURLSchemeHandler(DuckURLSchemeHandler(), forURLScheme: URL.NavigationalScheme.duck.rawValue) + setURLSchemeHandler( + DuckURLSchemeHandler(featureFlagger: NSApp.delegateTyped.featureFlagger), + forURLScheme: URL.NavigationalScheme.duck.rawValue + ) } } diff --git a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift index 5d464c726a..9939f5b3df 100644 --- a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift +++ b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift @@ -232,7 +232,6 @@ public struct UserDefaultsWrapper { case lastBrokenSiteToastShownDate = "brokenSitePrompt.last-broken-site-toast-shown-date" case toastDismissStreakCounter = "brokenSitePrompt.toast-dismiss-streak-counter" - } enum RemovedKeys: String, CaseIterable { diff --git a/DuckDuckGo/InternalUserDecider/FeatureFlagOverridesMenu.swift b/DuckDuckGo/InternalUserDecider/FeatureFlagOverridesMenu.swift new file mode 100644 index 0000000000..3506d97984 --- /dev/null +++ b/DuckDuckGo/InternalUserDecider/FeatureFlagOverridesMenu.swift @@ -0,0 +1,112 @@ +// +// FeatureFlagOverridesMenu.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import AppKit +import BrowserServicesKit +import FeatureFlags + +final class FeatureFlagOverridesMenu: NSMenu { + + let featureFlagger: FeatureFlagger + + let setInternalUserStateItem: NSMenuItem = { + let item = NSMenuItem(title: "Set Internal User State First") + item.isEnabled = false + return item + }() + + init(featureFlagOverrides: FeatureFlagger) { + self.featureFlagger = featureFlagOverrides + super.init(title: "") + + buildItems { + setInternalUserStateItem + NSMenuItem.separator() + + FeatureFlag.allCases.filter(\.supportsLocalOverriding).map { flag in + NSMenuItem( + title: "\(flag.rawValue) (default: \(featureFlagger.isFeatureOn(for: flag, allowOverride: false) ? "on" : "off"))", + action: #selector(toggleFeatureFlag(_:)), + target: self, + representedObject: flag + ) + } + + NSMenuItem.separator() + NSMenuItem(title: "Remove All Overrides", action: #selector(resetAllOverrides(_:))).targetting(self) + } + } + + required init(coder: NSCoder) { + fatalError("init(coder:) has not been implemented") + } + + override func update() { + super.update() + + items.forEach { item in + guard let flag = item.representedObject as? FeatureFlag else { + return + } + item.isHidden = !featureFlagger.internalUserDecider.isInternalUser + item.title = "\(flag.rawValue) (default: \(defaultValue(for: flag)), override: \(overrideValue(for: flag)))" + let override = featureFlagger.localOverrides?.override(for: flag) + item.state = override == true ? .on : .off + + if override != nil { + item.submenu = NSMenu(items: [ + NSMenuItem( + title: "Remove Override", + action: #selector(resetOverride(_:)), + target: self, + representedObject: flag + ) + ]) + } else { + item.submenu = nil + } + } + + setInternalUserStateItem.isHidden = featureFlagger.internalUserDecider.isInternalUser + } + + private func defaultValue(for flag: FeatureFlag) -> String { + featureFlagger.isFeatureOn(for: flag, allowOverride: false) ? "on" : "off" + } + + private func overrideValue(for flag: FeatureFlag) -> String { + guard let override = featureFlagger.localOverrides?.override(for: flag) else { + return "none" + } + return override ? "on" : "off" + } + + @objc func toggleFeatureFlag(_ sender: NSMenuItem) { + guard let featureFlag = sender.representedObject as? FeatureFlag else { return } + featureFlagger.localOverrides?.toggleOverride(for: featureFlag) + } + + @objc func resetOverride(_ sender: NSMenuItem) { + guard let featureFlag = sender.representedObject as? FeatureFlag else { return } + featureFlagger.localOverrides?.clearOverride(for: featureFlag) + } + + @objc func resetAllOverrides(_ sender: NSMenuItem) { + featureFlagger.localOverrides?.clearAllOverrides(for: FeatureFlag.self) + } +} diff --git a/DuckDuckGo/Menus/MainMenu.swift b/DuckDuckGo/Menus/MainMenu.swift index 018308dd0a..eafcb07d1b 100644 --- a/DuckDuckGo/Menus/MainMenu.swift +++ b/DuckDuckGo/Menus/MainMenu.swift @@ -20,6 +20,7 @@ import BrowserServicesKit import Cocoa import Common import Combine +import FeatureFlags import OSLog import SwiftUI import WebKit @@ -618,6 +619,9 @@ final class MainMenu: NSMenu { @MainActor private func setupDebugMenu() -> NSMenu { let debugMenu = NSMenu(title: "Debug") { + NSMenuItem(title: "Feature Flag Overrides") + .submenu(FeatureFlagOverridesMenu(featureFlagOverrides: NSApp.delegateTyped.featureFlagger)) + NSMenuItem.separator() NSMenuItem(title: "Open Vanilla Browser", action: #selector(MainViewController.openVanillaBrowser)).withAccessibilityIdentifier("MainMenu.openVanillaBrowser") NSMenuItem.separator() NSMenuItem(title: "Tab") { diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index 46667bc167..74f97fb45a 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -20,6 +20,7 @@ import BrowserServicesKit import Cocoa import Combine import Common +import FeatureFlags import SwiftUI import WebKit import Subscription @@ -132,6 +133,7 @@ final class BrowserTabViewController: NSViewController { hoverLabelContainer.alphaValue = 0 subscribeToTabs() subscribeToSelectedTabViewModel() + subscribeToHTMLNewTabPageFeatureFlagChanges() if let webViewContainer { removeChild(in: self.containerStackView, webViewContainer: webViewContainer) @@ -273,6 +275,25 @@ final class BrowserTabViewController: NSViewController { } } + private func subscribeToHTMLNewTabPageFeatureFlagChanges() { + guard let overridesHandler = featureFlagger.localOverrides?.actionHandler as? FeatureFlagOverridesPublishingHandler else { + return + } + + overridesHandler.flagDidChangePublisher + .filter { $0.0 == .htmlNewTabPage } + .asVoid() + .sink { [weak self] in + guard let self, let tabViewModel else { + return + } + if tabViewModel.tab.content == .newtab { + showTabContent(of: tabViewModel) + } + } + .store(in: &cancellables) + } + private func subscribeToSelectedTabViewModel() { tabCollectionViewModel.$selectedTabViewModel .sink { [weak self] selectedTabViewModel in @@ -777,8 +798,12 @@ final class BrowserTabViewController: NSViewController { updateTabIfNeeded(tabViewModel: tabViewModel) case .newtab: - removeAllTabContent() - addAndLayoutChild(homePageViewControllerCreatingIfNeeded()) + if NSApp.delegateTyped.featureFlagger.isFeatureOn(.htmlNewTabPage) { + updateTabIfNeeded(tabViewModel: tabViewModel) + } else { + removeAllTabContent() + addAndLayoutChild(homePageViewControllerCreatingIfNeeded()) + } case .dataBrokerProtection: removeAllTabContent() @@ -835,7 +860,9 @@ final class BrowserTabViewController: NSViewController { switch tabViewModel.tab.content { case .onboarding: return - case .newtab, .settings: + case .newtab: + containsHostingView = !NSApp.delegateTyped.featureFlagger.isFeatureOn(.htmlNewTabPage) + case .settings: containsHostingView = true default: containsHostingView = false diff --git a/DuckDuckGo/YoutubePlayer/DuckURLSchemeHandler.swift b/DuckDuckGo/YoutubePlayer/DuckURLSchemeHandler.swift index e169c255e5..acd03cc599 100644 --- a/DuckDuckGo/YoutubePlayer/DuckURLSchemeHandler.swift +++ b/DuckDuckGo/YoutubePlayer/DuckURLSchemeHandler.swift @@ -16,6 +16,8 @@ // limitations under the License. // +import BrowserServicesKit +import FeatureFlags import Foundation import WebKit import ContentScopeScripts @@ -23,6 +25,12 @@ import PhishingDetection final class DuckURLSchemeHandler: NSObject, WKURLSchemeHandler { + let featureFlagger: FeatureFlagger + + init(featureFlagger: FeatureFlagger) { + self.featureFlagger = featureFlagger + } + func webView(_ webView: WKWebView, start urlSchemeTask: WKURLSchemeTask) { guard let requestURL = webView.url ?? urlSchemeTask.request.url else { assertionFailure("No URL for Duck scheme handler") @@ -36,6 +44,12 @@ final class DuckURLSchemeHandler: NSObject, WKURLSchemeHandler { handleDuckPlayer(requestURL: requestURL, urlSchemeTask: urlSchemeTask, webView: webView) case .phishingErrorPage: handleErrorPage(urlSchemeTask: urlSchemeTask) + case .newTab: + if featureFlagger.isFeatureOn(.htmlNewTabPage) { + handleSpecialPages(urlSchemeTask: urlSchemeTask) + } else { + handleNativeUIPages(requestURL: requestURL, urlSchemeTask: urlSchemeTask) + } default: handleNativeUIPages(requestURL: requestURL, urlSchemeTask: urlSchemeTask) } @@ -127,6 +141,8 @@ private extension DuckURLSchemeHandler { directoryURL = URL(fileURLWithPath: "/pages/onboarding") } else if url.isReleaseNotesScheme { directoryURL = URL(fileURLWithPath: "/pages/release-notes") + } else if url.isNewTab { + directoryURL = URL(fileURLWithPath: "/pages/new-tab") } else { assertionFailure("Unknown scheme") return nil @@ -205,6 +221,7 @@ private extension DuckURLSchemeHandler { extension URL { enum URLType { + case newTab case onboarding case duckPlayer case releaseNotes @@ -220,6 +237,8 @@ extension URL { return .phishingErrorPage } else if self.isReleaseNotesScheme { return .releaseNotes + } else if self.isNewTab { + return .newTab } else { return nil } @@ -229,6 +248,10 @@ extension URL { return isDuckURLScheme && host == "onboarding" } + var isNewTab: Bool { + return isDuckURLScheme && host == "newtab" + } + var isDuckURLScheme: Bool { navigationalScheme == .duck } diff --git a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift index d10d7e0c6b..0080b3fd6d 100644 --- a/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift +++ b/IntegrationTests/PhishingDetection/PhishingDetectionIntegrationTests.swift @@ -148,7 +148,10 @@ class PhishingDetectionIntegrationTests: XCTestCase { } class MockFeatureFlagger: FeatureFlagger { - func isFeatureOn(forProvider: F) -> Bool where F: BrowserServicesKit.FeatureFlagSourceProviding { + var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: MockInternalUserStoring()) + var localOverrides: FeatureFlagLocalOverriding? + + func isFeatureOn(for featureFlag: Flag, allowOverride: Bool) -> Bool { return true } } diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index f0e8b50ef3..0f3b5215f7 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), .package(path: "../SwiftUIExtensions"), .package(path: "../AppKitExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/FeatureFlags/Package.swift b/LocalPackages/FeatureFlags/Package.swift index 4664955e62..f0fad2df10 100644 --- a/LocalPackages/FeatureFlags/Package.swift +++ b/LocalPackages/FeatureFlags/Package.swift @@ -32,7 +32,7 @@ let package = Package( targets: ["FeatureFlags"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), ], targets: [ // Targets are the basic building blocks of a package, defining a module or a test suite. diff --git a/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift b/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift index 3811ad546a..933dcb23e3 100644 --- a/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift +++ b/LocalPackages/FeatureFlags/Sources/FeatureFlags/FeatureFlag.swift @@ -19,7 +19,7 @@ import Foundation import BrowserServicesKit -public enum FeatureFlag: String { +public enum FeatureFlag: String, CaseIterable { case debugMenu case sslCertificatesBypass case phishingDetectionErrorPage @@ -44,9 +44,21 @@ public enum FeatureFlag: String { /// https://app.asana.com/0/72649045549333/1208617860225199/f case networkProtectionEnforceRoutes + + /// https://app.asana.com/0/72649045549333/1208241266421040/f + case htmlNewTabPage } -extension FeatureFlag: FeatureFlagSourceProviding { +extension FeatureFlag: FeatureFlagDescribing { + public var supportsLocalOverriding: Bool { + switch self { + case .htmlNewTabPage: + return true + default: + return false + } + } + public var source: FeatureFlagSource { switch self { case .debugMenu: @@ -71,12 +83,15 @@ extension FeatureFlag: FeatureFlagSourceProviding { return .remoteDevelopment(.subfeature(NetworkProtectionSubfeature.userTips)) case .networkProtectionEnforceRoutes: return .remoteDevelopment(.subfeature(NetworkProtectionSubfeature.enforceRoutes)) + case .htmlNewTabPage: + return .disabled } } } -extension FeatureFlagger { - public func isFeatureOn(_ featureFlag: FeatureFlag) -> Bool { - isFeatureOn(forProvider: featureFlag) +public extension FeatureFlagger { + + func isFeatureOn(_ featureFlag: FeatureFlag) -> Bool { + isFeatureOn(for: featureFlag) } } diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index e35805cdc2..8f8fff2eb8 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -32,7 +32,7 @@ let package = Package( .library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), .package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"), .package(path: "../AppLauncher"), .package(path: "../UDSHelper"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index ff5e096f9b..02f91f329a 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "209.1.2"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), .package(path: "../SwiftUIExtensions") ], targets: [ diff --git a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift index 7e28a5335c..32b6109bff 100644 --- a/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift +++ b/UnitTests/DuckSchemeHandler/DuckSchemeHandlerTests.swift @@ -25,10 +25,20 @@ import PhishingDetection final class DuckSchemeHandlerTests: XCTestCase { + var featureFlagger: MockFeatureFlagger! + var handler: DuckURLSchemeHandler! + + override func setUp() { + super.setUp() + featureFlagger = MockFeatureFlagger() + featureFlagger.isFeatureOn = false + + handler = DuckURLSchemeHandler(featureFlagger: featureFlagger) + } + func testWebViewFromOnboardingHandlerReturnsResponseAndData() throws { // Given let onboardingURL = URL(string: "duck://onboarding")! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: onboardingURL)) @@ -47,7 +57,6 @@ final class DuckSchemeHandlerTests: XCTestCase { func testWebViewFromReleaseNoteHandlerReturnsResponseAndData() throws { // Given let releaseNotesURL = URL(string: "duck://release-notes")! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: releaseNotesURL)) @@ -67,7 +76,6 @@ final class DuckSchemeHandlerTests: XCTestCase { func testWebViewFromDuckPlayerHandlerReturnsResponseAndData() throws { // Given let duckPlayerURL = URL(string: "duck://player")! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: duckPlayerURL)) @@ -84,7 +92,6 @@ final class DuckSchemeHandlerTests: XCTestCase { func testWebViewFromNativeUIHandlerReturnsResponseAndData() throws { // Given let nativeURL = URL(string: "duck://newtab")! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: nativeURL)) @@ -123,7 +130,6 @@ final class DuckSchemeHandlerTests: XCTestCase { let token = URLTokenValidator.shared.generateToken(for: phishingUrl) let errorURLString = "duck://error?reason=phishing&url=\(encodedURL)&token=\(token)" let errorURL = URL(string: errorURLString)! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: errorURL)) @@ -148,7 +154,6 @@ final class DuckSchemeHandlerTests: XCTestCase { let token = "ababababababababababab" let errorURLString = "duck://error?reason=phishing&url=\(encodedURL)&token=\(token)" let errorURL = URL(string: errorURLString)! - let handler = DuckURLSchemeHandler() let webView = WKWebView() let schemeTask = MockSchemeTask(request: URLRequest(url: errorURL)) diff --git a/UnitTests/Menus/MainMenuTests.swift b/UnitTests/Menus/MainMenuTests.swift index 56042fbf87..73f48a53e5 100644 --- a/UnitTests/Menus/MainMenuTests.swift +++ b/UnitTests/Menus/MainMenuTests.swift @@ -194,7 +194,10 @@ class MainMenuTests: XCTestCase { } private class DummyFeatureFlagger: FeatureFlagger { - func isFeatureOn(forProvider: F) -> Bool { + var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: MockInternalUserStoring()) + var localOverrides: FeatureFlagLocalOverriding? + + func isFeatureOn(for: Flag, allowOverride: Bool) -> Bool { false } } diff --git a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift index ffa3b7d9c6..dbfad1313c 100644 --- a/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift +++ b/UnitTests/TabExtensionsTests/ErrorPageTabExtensionTest.swift @@ -480,8 +480,11 @@ class ChallangeSender: URLAuthenticationChallengeSender { } class MockFeatureFlagger: FeatureFlagger { + var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: MockInternalUserStoring()) + var localOverrides: FeatureFlagLocalOverriding? + var isFeatureOn = true - func isFeatureOn(forProvider: F) -> Bool where F: BrowserServicesKit.FeatureFlagSourceProviding { + func isFeatureOn(for featureFlag: Flag, allowOverride: Bool) -> Bool { return isFeatureOn } } From 8d82528305715516d2fbf37914c924596ddf1e42 Mon Sep 17 00:00:00 2001 From: Christopher Brind Date: Thu, 14 Nov 2024 12:41:33 +0000 Subject: [PATCH 10/13] bump bsk for text zoom feature definition (#3496) Task/Issue URL: https://app.asana.com/0/72649045549333/1206268893832417/f Tech Design URL: CC: **Description**: Bumps BSK to add text zoom feature flag. **Steps to test this PR**: 1. Build and run the app --- DuckDuckGo.xcodeproj/project.pbxproj | 2 +- .../project.xcworkspace/xcshareddata/swiftpm/Package.resolved | 4 ++-- LocalPackages/DataBrokerProtection/Package.swift | 2 +- LocalPackages/FeatureFlags/Package.swift | 2 +- LocalPackages/NetworkProtectionMac/Package.swift | 2 +- LocalPackages/SubscriptionUI/Package.swift | 2 +- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 4c2c971788..37c92cdb75 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -15072,7 +15072,7 @@ repositoryURL = "https://github.com/duckduckgo/BrowserServicesKit"; requirement = { kind = exactVersion; - version = 210.0.0; + version = 210.0.1; }; }; 9FF521422BAA8FF300B9819B /* XCRemoteSwiftPackageReference "lottie-spm" */ = { diff --git a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index 347bb2d949..06a74c8d0d 100644 --- a/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -32,8 +32,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/duckduckgo/BrowserServicesKit", "state" : { - "revision" : "cfb178099738bc6cd0c3a3d73df717420ef4a59f", - "version" : "210.0.0" + "revision" : "183b9c111176fd7821cd17d01c01ddb38486c9ac", + "version" : "210.0.1" } }, { diff --git a/LocalPackages/DataBrokerProtection/Package.swift b/LocalPackages/DataBrokerProtection/Package.swift index 0f3b5215f7..ba3685c716 100644 --- a/LocalPackages/DataBrokerProtection/Package.swift +++ b/LocalPackages/DataBrokerProtection/Package.swift @@ -29,7 +29,7 @@ let package = Package( targets: ["DataBrokerProtection"]) ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.1"), .package(path: "../SwiftUIExtensions"), .package(path: "../AppKitExtensions"), .package(path: "../XPCHelper"), diff --git a/LocalPackages/FeatureFlags/Package.swift b/LocalPackages/FeatureFlags/Package.swift index f0fad2df10..673391b842 100644 --- a/LocalPackages/FeatureFlags/Package.swift +++ b/LocalPackages/FeatureFlags/Package.swift @@ -32,7 +32,7 @@ let package = Package( targets: ["FeatureFlags"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.1"), ], targets: [ // Targets are the basic building blocks of a package, defining a module or a test suite. diff --git a/LocalPackages/NetworkProtectionMac/Package.swift b/LocalPackages/NetworkProtectionMac/Package.swift index 8f8fff2eb8..e0e7b63e3a 100644 --- a/LocalPackages/NetworkProtectionMac/Package.swift +++ b/LocalPackages/NetworkProtectionMac/Package.swift @@ -32,7 +32,7 @@ let package = Package( .library(name: "VPNAppLauncher", targets: ["VPNAppLauncher"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.1"), .package(url: "https://github.com/airbnb/lottie-spm", exact: "4.4.3"), .package(path: "../AppLauncher"), .package(path: "../UDSHelper"), diff --git a/LocalPackages/SubscriptionUI/Package.swift b/LocalPackages/SubscriptionUI/Package.swift index 02f91f329a..fadd3544e4 100644 --- a/LocalPackages/SubscriptionUI/Package.swift +++ b/LocalPackages/SubscriptionUI/Package.swift @@ -12,7 +12,7 @@ let package = Package( targets: ["SubscriptionUI"]), ], dependencies: [ - .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.0"), + .package(url: "https://github.com/duckduckgo/BrowserServicesKit", exact: "210.0.1"), .package(path: "../SwiftUIExtensions") ], targets: [ From 17196a33e91e383be12aca6ef9ad084a34bf9a82 Mon Sep 17 00:00:00 2001 From: Dax the Duck Date: Thu, 14 Nov 2024 13:11:47 +0000 Subject: [PATCH 11/13] Bump version to 1.114.0 (307) --- Configuration/BuildNumber.xcconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Configuration/BuildNumber.xcconfig b/Configuration/BuildNumber.xcconfig index b8206ee1bd..919ac1eab9 100644 --- a/Configuration/BuildNumber.xcconfig +++ b/Configuration/BuildNumber.xcconfig @@ -1 +1 @@ -CURRENT_PROJECT_VERSION = 306 +CURRENT_PROJECT_VERSION = 307 From ce3975e839cb45eea897c4a1433a69dc73ec7687 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Thu, 14 Nov 2024 15:18:33 +0100 Subject: [PATCH 12/13] Move FE Overlay Pixel to the Native layer (#3550) Task/Issue URL: https://app.asana.com/0/1204099484721401/1208754606104631/f Tech Design URL: CC: **Description**: - Move the `m_mac_duck-player_overlay_youtube_impressions` pixel to the native layer --- .../DuckPlayerTabExtension.swift | 32 +++++++++++++++++++ .../YoutubeOverlayUserScript.swift | 7 ++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift index 85b73b3bc0..a05d0e03ba 100644 --- a/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift +++ b/DuckDuckGo/Tab/TabExtensions/DuckPlayerTabExtension.swift @@ -128,6 +128,30 @@ final class DuckPlayerTabExtension { } } + private func fireOverlayShownPixelIfNeeded(url: URL) { + + guard duckPlayer.isAvailable, + duckPlayer.mode == .alwaysAsk, + url.isYoutubeWatch else { + return + } + + // Static variable for debounce logic + let debounceInterval: TimeInterval = 1.0 + let now = Date() + + struct Debounce { + static var lastFireTime: Date? + } + + // Check debounce condition and update timestamp if firing + guard Debounce.lastFireTime == nil || now.timeIntervalSince(Debounce.lastFireTime!) >= debounceInterval else { + return + } + + Debounce.lastFireTime = now + PixelKit.fire(GeneralPixel.duckPlayerOverlayYoutubeImpressions) + } } extension DuckPlayerTabExtension: YoutubeOverlayUserScriptDelegate { @@ -191,6 +215,11 @@ extension DuckPlayerTabExtension: NavigationResponder { return decidePolicyWithDisabledDuckPlayer(for: navigationAction) } + // Fires the Overlay Shown Pixel if not coming from DuckPlayer's Watch in Youtube + if !navigationAction.sourceFrame.url.isDuckPlayer { + fireOverlayShownPixelIfNeeded(url: navigationAction.url) + } + // session restoration will try to load real www.youtube-nocookie.com url // we need to redirect it to custom duck:// scheme handler which will load // www.youtube-nocookie.com as a simulated request @@ -288,6 +317,9 @@ extension DuckPlayerTabExtension: NavigationResponder { webView.load(URLRequest(url: .duckPlayer(videoID, timestamp: timestamp))) } + // Fire Overlay Shown Pixels + fireOverlayShownPixelIfNeeded(url: navigation.url) + // Fire DuckPlayer Overlay Temporary Pixels if let url = navigation.request.url { duckPlayerOverlayUsagePixels.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayer.mode) diff --git a/DuckDuckGo/YoutubePlayer/YoutubeOverlayUserScript.swift b/DuckDuckGo/YoutubePlayer/YoutubeOverlayUserScript.swift index 4d20a6ccd4..5342bea1ec 100644 --- a/DuckDuckGo/YoutubePlayer/YoutubeOverlayUserScript.swift +++ b/DuckDuckGo/YoutubePlayer/YoutubeOverlayUserScript.swift @@ -161,8 +161,11 @@ extension YoutubeOverlayUserScript { case "play.do_not_use": duckPlayerPreferences.youtubeOverlayAnyButtonPressed = true PixelKit.fire(GeneralPixel.duckPlayerOverlayYoutubeWatchHere) - case "overlay": - PixelKit.fire(GeneralPixel.duckPlayerOverlayYoutubeImpressions) + + // Moved to DuckPlayerTabExtension + // See :https://app.asana.com/0/1203249713006009/1208754606104659/f + // case "overlay": + // PixelKit.fire(GeneralPixel.duckPlayerOverlayYoutubeImpressions) default: break } From 9e4e47253b88715af91299340f55f078563c9f66 Mon Sep 17 00:00:00 2001 From: Juan Manuel Pereira Date: Thu, 14 Nov 2024 13:05:40 -0300 Subject: [PATCH 13/13] Disable reordering when sort by name is enabled (#3552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task/Issue URL: https://app.asana.com/0/1204006570077678/1208661547711012/f Tech Design URL: CC: **Description**: This fixes a bug where we enabled reordering bookmarks in both the panel and the manager when sort by name was enabled. **Steps to test this PR**: 1. Go to the bookmarks panel or manager 2. Sort by name descending or ascending 3. Try to re-order bookmarks 4. It shouldn’t let you drop bookmarks to re-order them **Definition of Done**: * [x] Does this PR satisfy our [Definition of Done](https://app.asana.com/0/1202500774821704/1207634633537039/f)? — ###### Internal references: [Pull Request Review Checklist](https://app.asana.com/0/1202500774821704/1203764234894239/f) [Software Engineering Expectations](https://app.asana.com/0/59792373528535/199064865822552) [Technical Design Template](https://app.asana.com/0/59792373528535/184709971311943) [Pull Request Documentation](https://app.asana.com/0/1202500774821704/1204012835277482/f) --- .../Bookmarks/Model/BookmarkOutlineViewDataSource.swift | 6 ++++++ .../View/BookmarkManagementDetailViewController.swift | 1 + DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift | 4 ++++ UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift | 6 ++++++ 4 files changed, 17 insertions(+) diff --git a/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift b/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift index a4a3f63e8e..d5ca01e50e 100644 --- a/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift +++ b/DuckDuckGo/Bookmarks/Model/BookmarkOutlineViewDataSource.swift @@ -48,6 +48,7 @@ final class BookmarkOutlineViewDataSource: NSObject, BookmarksOutlineViewDataSou private var outlineView: BookmarksOutlineView? private let contentMode: ContentMode + private var sortMode: BookmarksSortMode private(set) var expandedNodesIDs = Set() @Published private(set) var isSearching = false @@ -95,6 +96,7 @@ final class BookmarkOutlineViewDataSource: NSObject, BookmarksOutlineViewDataSou self.dragDropManager = dragDropManager self.treeController = treeController self.presentFaviconsFetcherOnboarding = presentFaviconsFetcherOnboarding + self.sortMode = sortMode super.init() } @@ -102,11 +104,13 @@ final class BookmarkOutlineViewDataSource: NSObject, BookmarksOutlineViewDataSou func reloadData(with sortMode: BookmarksSortMode, withRootFolder rootFolder: BookmarkFolder? = nil) { isSearching = false dragDestinationFolder = nil + self.sortMode = sortMode treeController.rebuild(for: sortMode, withRootFolder: rootFolder) } func reloadData(forSearchQuery searchQuery: String, sortMode: BookmarksSortMode) { isSearching = true + self.sortMode = sortMode treeController.rebuild(forSearchQuery: searchQuery, sortMode: sortMode) } @@ -260,6 +264,8 @@ final class BookmarkOutlineViewDataSource: NSObject, BookmarksOutlineViewDataSou } func outlineView(_ outlineView: NSOutlineView, validateDrop info: NSDraggingInfo, proposedItem item: Any?, proposedChildIndex index: Int) -> NSDragOperation { + if !sortMode.isReorderingEnabled { return .none } + let destinationNode = nodeForItem(item) if contentMode == .foldersOnly, destinationNode.isRoot { diff --git a/DuckDuckGo/Bookmarks/View/BookmarkManagementDetailViewController.swift b/DuckDuckGo/Bookmarks/View/BookmarkManagementDetailViewController.swift index 1179c44b3e..ca181a64fa 100644 --- a/DuckDuckGo/Bookmarks/View/BookmarkManagementDetailViewController.swift +++ b/DuckDuckGo/Bookmarks/View/BookmarkManagementDetailViewController.swift @@ -555,6 +555,7 @@ extension BookmarkManagementDetailViewController: NSTableViewDelegate, NSTableVi validateDrop info: NSDraggingInfo, proposedRow row: Int, proposedDropOperation dropOperation: NSTableView.DropOperation) -> NSDragOperation { + if !sortBookmarksViewModel.selectedSortMode.isReorderingEnabled { return .none } let destination = destination(for: dropOperation, at: row) guard !isSearching || destination is BookmarkFolder else { return .none } diff --git a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift index 729408802a..3f10dcd297 100644 --- a/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift +++ b/DuckDuckGo/Bookmarks/ViewModel/SortBookmarksViewModel.swift @@ -55,6 +55,10 @@ enum BookmarksSortMode: Codable { return self == .nameAscending || self == .nameDescending } + var isReorderingEnabled: Bool{ + return self == .manual + } + func menu(target: AnyObject) -> NSMenu { switch self { case .manual: diff --git a/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift b/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift index d3cbaf14fb..87bf16d394 100644 --- a/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift +++ b/UnitTests/Bookmarks/ViewModels/BookmarksSortModeTests.swift @@ -100,4 +100,10 @@ class BookmarksSortModeTests: XCTestCase { XCTAssertEqual(descendingMenu.items[4].title, UserText.bookmarksSortByNameDescending) XCTAssertEqual(descendingMenu.items[4].state, .on) } + + func testReorderingValueIsCorrect() { + XCTAssertTrue(BookmarksSortMode.manual.isReorderingEnabled) + XCTAssertFalse(BookmarksSortMode.nameAscending.isReorderingEnabled) + XCTAssertFalse(BookmarksSortMode.nameDescending.isReorderingEnabled) + } }