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

Fix #1358: Cookie consent blocking #5839

Merged
merged 4 commits into from
Nov 2, 2022

Conversation

cuba
Copy link
Contributor

@cuba cuba commented Aug 11, 2022

Summary of Changes

This will add a setting to Shields to enable cookie consent notices
It will also show a popup on second launch to allow the user to know this feature is available

This pull request fixes #1358

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:

Make sure that

When the user clicks on "Yes block cookie consent notices" on the popup:

  • The setting is enabled in shields
  • The cookie notice blocking filter list is enabled
  • The animation plays and then the popup disappears

When the user clicks on "No thanks" on the popup

  • The setting is not enabled in shields
  • The cookie notice blocking filter list is not enabled
  • The popup is dismissed

When the setting is enabled:

  • The popup doesn't appear on second launch (Note: you will need to enabled it on first launch)
  • The cookie consent notices are blocked

When the setting is not enabled:

  • The popup appears on a second launch (note that depending on timing other full screen popups might appear in priority)
  • Cookie consent notices are not blocked

Persistence

  • Test for synchronization between the cookie blocking setting in shields and the filter list is enabled in filter lists screen
  • Test for persistence (setting is held) when the app is terminated

Screenshots:

Simulator Screen Shot - iPhone 13 Pro - 2022-08-11 at 12 35 06
Simulator Screen Shot - iPhone 13 Pro - 2022-08-11 at 12 34 57
Simulator Screen Shot - iPhone 13 Pro - 2022-08-11 at 12 34 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/1358-cookie-consent-blocking branch 2 times, most recently from e72549d to 733cac0 Compare August 13, 2022 21:46
@cuba cuba changed the title Js/1358 cookie consent blocking Fix #1358: Cookie consent blocking Aug 14, 2022
@cuba cuba changed the base branch from development to js/5729-filter-lists August 14, 2022 18:46
@cuba cuba force-pushed the js/5729-filter-lists branch from cf365a3 to 370e45e Compare August 14, 2022 20:44
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch 3 times, most recently from baf4e85 to 430ba23 Compare August 16, 2022 00:48
@cuba cuba force-pushed the js/5729-filter-lists branch from 9774b32 to 62efd3a Compare August 22, 2022 16:59
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from 430ba23 to e1f3b09 Compare August 22, 2022 18:14
@cuba cuba force-pushed the js/5729-filter-lists branch from 62efd3a to c858cc1 Compare August 23, 2022 17:54
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from e1f3b09 to 4ab9993 Compare August 23, 2022 17:54
@cuba cuba force-pushed the js/5729-filter-lists branch 4 times, most recently from 01a8bb2 to f5f547a Compare August 31, 2022 22:23
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from 4ab9993 to b126c68 Compare August 31, 2022 23:20
@cuba cuba force-pushed the js/5729-filter-lists branch 2 times, most recently from e1f6ef0 to 7cb4a0f Compare September 6, 2022 22:50
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch 3 times, most recently from 44b5300 to a236c6c Compare September 6, 2022 23:27
@cuba cuba force-pushed the js/5729-filter-lists branch 2 times, most recently from 24bcf0c to d6ddfbc Compare September 8, 2022 20:00
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from a236c6c to 7ce87b3 Compare September 10, 2022 02:47
Base automatically changed from js/5729-filter-lists to development September 15, 2022 16:35
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch 2 times, most recently from e687f94 to 7d5bd74 Compare September 26, 2022 10:10
@cuba cuba marked this pull request as ready for review September 26, 2022 10:10
@cuba cuba requested a review from a team September 26, 2022 10:10
Copy link
Contributor

@soner-yuksel soner-yuksel left a comment

Choose a reason for hiding this comment

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

All comments are fixed and working as expected 🎉

@iccub iccub added blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue and removed QA/Yes labels Sep 28, 2022
@soner-yuksel
Copy link
Contributor

I have another question. are we supposed to show this pop-up for new users ?

@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch 2 times, most recently from 49a147f to b25a4d1 Compare September 30, 2022 13:17
Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

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

SwiftUI changes, and the new swiftUI does not support accessibility, it has to be fixed

@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from 4802098 to 6023618 Compare October 3, 2022 15:46
@iccub iccub removed the blocked If a ticket is blocked for some reason, if not using a sub-block label, please provide info in issue label Oct 3, 2022
@iccub
Copy link
Contributor

iccub commented Oct 3, 2022

Most of my comments are still unaddressed

Copy link
Contributor

@iccub iccub left a comment

Choose a reason for hiding this comment

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

Code looks good to me, only thing left is the BraveButton style changes

@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from 295edc0 to 3fb68d9 Compare October 27, 2022 08:55
@iccub iccub requested a review from soner-yuksel November 2, 2022 11:45
@cuba cuba force-pushed the js/1358-cookie-consent-blocking branch from 3fb68d9 to 2b20c84 Compare November 2, 2022 14:12
@iccub iccub merged commit df087be into development Nov 2, 2022
@iccub iccub deleted the js/1358-cookie-consent-blocking branch November 2, 2022 16:00
@Uni-verse Uni-verse mentioned this pull request Nov 9, 2022
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.

Cookie consent options
4 participants