Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Browser setting doesn't reflect on shield extension #226

Closed
srirambv opened this issue May 25, 2018 · 9 comments
Closed

Browser setting doesn't reflect on shield extension #226

srirambv opened this issue May 25, 2018 · 9 comments
Labels
closed/not-actionable feature/global-settings Settings at browser level independent of shields settings feature/shields The overall Shields feature in Brave.

Comments

@srirambv
Copy link
Contributor

Description

Browser setting doesn't reflect on shield extension

Steps to Reproduce

  1. Settings->Content Setting->Cookies-> Disable Allow sites to save and read cookie data switch setting it to block cookies
  2. Open a new tab and visit jsfiddle.net, page opens correctly
  3. Open shields, Cookie control is set to Block 3rd party cookies (default setting)
  4. Change cookie setting to Block all cookies in shield
  5. Page doesn't load because cookies not set

Actual result:

Browser setting doesn't reflect on shield extension

Expected result:

Cookie settings in shields should match if it is changed at browser level

@bbondy
Copy link
Member

bbondy commented Jun 6, 2018

cc @bradleyrichter

@bbondy bbondy self-assigned this Jun 6, 2018
@bbondy bbondy added this to the Milestone 3: June-July milestone Jun 6, 2018
@srirambv srirambv added feature/shields The overall Shields feature in Brave. feature/global-settings Settings at browser level independent of shields settings labels Jun 15, 2018
@cezaraugusto cezaraugusto self-assigned this Aug 20, 2018
@bbondy
Copy link
Member

bbondy commented Sep 30, 2018

I think since we already have global shield settings we can just hide the option that is conflicting. Do you agree @srirambv ?

@rebron
Copy link
Collaborator

rebron commented Sep 30, 2018

This setting should be hidden Settings->Content Setting->Cookies-> Allow sites to save and read cookie data
since it's redundant with Shield Settings-> Cookies Blocked
See: #606 (comment)
I'll get an updated design screenshot specific for Cookies

@srirambv
Copy link
Contributor Author

@bbondy @rebron agree Allow sites to save and read cookie option should be disabled. It should only have the following sections

  1. Keep local data only until you quit your browser
  2. Block
  3. Clear on exit (not shown on Global Shields Settings restyle #606 (comment) but should be part of it since user can set the cookies to be deleted upon exit or for a session only)
  4. Allow

@bbondy bbondy added the QA/Yes label Oct 10, 2018
@bbondy
Copy link
Member

bbondy commented Oct 12, 2018

This one is probably not the end of the world even know it is inconsistent, so I'll move to 1.x and put as p2.

@bbondy bbondy added the priority/P2 A bad problem. We might uplift this to the next planned release. label Oct 12, 2018
@bbondy bbondy modified the milestones: 0.55.x, 1.x Backlog Oct 12, 2018
@LaurenWags
Copy link
Member

LaurenWags commented Oct 15, 2018

+1 from me - encountered this and it was very confusing when trying to trouble shoot why a site wouldn't work properly. Having Cookies set to 'Blocked' here overrode my shield settings of 'Block 3rd party cookies' but that wasn't evident when trying to resolve an issue. Would be nice to get this in sooner rather than later.
screen shot 2018-10-15 at 10 04 04 am

@cezaraugusto
Copy link
Contributor

I don't know exactly what we expect here. should we remove the setting from the settings page?

@tildelowengrimm tildelowengrimm added priority/P3 The next thing for us to work on. It'll ride the trains. and removed priority/P2 A bad problem. We might uplift this to the next planned release. labels Oct 31, 2018
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@rebron rebron modified the milestone: 1.x Backlog Feb 7, 2019
@rebron
Copy link
Collaborator

rebron commented Jan 23, 2020

Still need a decision here whether we need to hide general browser cookie pref in favor of shields pref due to conflicts. Label has changed over time and now reads 'Allow sites to save and read cookie data (recommended).

Screen Shot 2020-01-23 at 12 55 00 PM

@bbondy bbondy removed their assignment Feb 21, 2020
@bridiver
Copy link
Contributor

bridiver commented Feb 3, 2022

this is not relevant anymore because the settings have changed, but there is a new ticket to cover a UI update issue #20856

@bridiver bridiver closed this as completed Feb 3, 2022
@rebron rebron removed priority/P3 The next thing for us to work on. It'll ride the trains. QA/Yes labels May 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed/not-actionable feature/global-settings Settings at browser level independent of shields settings feature/shields The overall Shields feature in Brave.
Projects
None yet
Development

No branches or pull requests

7 participants