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

Add ad-block level to advanced shield settings #7437

Merged
merged 1 commit into from
May 16, 2023

Conversation

cuba
Copy link
Contributor

@cuba cuba commented May 11, 2023

Summary of Changes

This pull request fixes #7352

Submitter Checklist:

  • Unit Tests are updated to cover new or changed functionality
  • User-facing strings use NSLocalizableString()
  • New or updated UI has been tested across:
    • Light & dark mode
    • Different size classes (iPhone, landscape, iPad)
    • Different dynamic type sizes

Test Plan:

  1. Go to brave search and search for "trezor". (note it's possible the blocking rules or ads might change or may be regional)
  • The ad on the top of the page should be hidden on aggressive
  • The ad on the top of the page should be shown on standard
  1. Go to brave search and search for "3d printer" (you need to VPN to the United States).
  • Should see the shopping carrousel on standard mode
  • Should not see the shopping carrousel on aggressive mode
  1. Enable "Block cookie consent notices" and go to reddit. (note rules and ads sometimes change this is a somewhat uncontrolled test)
  • Cookie notices should be blocked on standard and aggressive
  1. Enable "Block open in app notices" and go to reddit. (note rules and ads sometimes change this is a somewhat uncontrolled test)
  • Open in app notices should be blocked on standard and aggressive
  1. Go to the following page: https://dev-pages.brave.software/filtering/cosmetic-filtering.html
  • On aggressive mode, all images should be hidden after clicking "Run tests"
  • On standard mode, only 3rd party images should be hidden after clicking "Run tests" (there is a slight unhiding delay)
  1. Test any other known sites you're familiar with.
  2. These changes doesn't touch at all YouTube ad blocking but just in case its worthwhile to test this.
  3. Test migration. Install an older version of the app and set the default blocking to false (disabled)
  • Update the app and ensure that the default is set to "Disabled"
  1. Test migration. Install an older version of the app and switch between off and on for tracking and ad blocking
  • Update the app and ensure that the default is set to "Standard"
  1. Test default.
  • Install the app from scratch and ensure that the default is set to "Standard".
  1. Test site settings.
  • Install from scratch and make sure the site tracking and ad blocking is on
  • Change the default to "Disabled". Ensure the site tracking blocking is off
  • Change the site settings to be different than the default. Change the default. Site settings are not affected.

Screenshots:

iPhone 14

Dark Light
Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 41 05 Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 40 36
Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 41 00 Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 40 39

Tests

Aggressive Standard
Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 50 43 Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 12 50 10

iPad

Model 1 Model 2
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 54 29 Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 55 23
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 54 10 Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 55 01
Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 54 39 Simulator Screenshot - iPad Pro (12 9-inch) (6th generation) - 2023-05-12 at 12 54 53

Ads

Standard Aggressive
Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 19 57 16 Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 19 59 58
Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 20 01 52 Simulator Screenshot - iPhone 14 Pro - 2023-05-12 at 20 03 24

Reviewer Checklist:

  • Issues include necessary QA labels:
    • QA/(Yes|No)
    • bug / enhancement
  • Necessary security reviews have taken place.
  • Adequate unit test coverage exists to prevent regressions.
  • Adequate test plan exists for QA to validate (if applicable).
  • Issue and pull request is assigned to a milestone (should happen at merge time).

@cuba cuba force-pushed the js/advanced-shield-settings-view-ad-block-level branch 2 times, most recently from e7ac811 to 3333f96 Compare May 11, 2023 12:09
@cuba cuba marked this pull request as ready for review May 12, 2023 10:29
@cuba cuba requested a review from a team as a code owner May 12, 2023 10:29
@cuba cuba added the QA/Yes label May 12, 2023
@cuba cuba force-pushed the js/advanced-shield-settings-view-ad-block-level branch 3 times, most recently from fd0e6a1 to e72871f Compare May 16, 2023 12:32
Base automatically changed from js/advanced-shields-settings-view to development May 16, 2023 13:52
@cuba cuba force-pushed the js/advanced-shield-settings-view-ad-block-level branch from e72871f to 1da1bb7 Compare May 16, 2023 14:00
@cuba cuba force-pushed the js/advanced-shield-settings-view-ad-block-level branch from 1da1bb7 to d7cf098 Compare May 16, 2023 14:05
@iccub iccub added this to the 1.51 milestone May 16, 2023
@iccub iccub removed the QA/Yes label May 16, 2023
@iccub iccub merged commit 9ac98b8 into development May 16, 2023
@iccub iccub deleted the js/advanced-shield-settings-view-ad-block-level branch May 16, 2023 14:59
@iccub iccub modified the milestones: 1.51, 1.52 May 16, 2023
@iccub iccub modified the milestones: 1.52, 1.51.1 May 17, 2023
arthuredelstein pushed a commit to brave/brave-core that referenced this pull request Feb 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add standard (non-aggressive) Adblock mode
2 participants