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

[Desktop] Site settings isn't retained #10285

Closed
srirambv opened this issue Jun 16, 2020 · 2 comments · Fixed by brave/brave-core#5879
Closed

[Desktop] Site settings isn't retained #10285

srirambv opened this issue Jun 16, 2020 · 2 comments · Fixed by brave/brave-core#5879
Assignees
Labels
feature/global-settings Settings at browser level independent of shields settings feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes regression release-notes/exclude

Comments

@srirambv
Copy link
Contributor

Description

Site settings isn't retained

Steps to Reproduce

  1. Set FP to strict in global settings
  2. Visit YT and change FP setting to standard
  3. Page reloads but settings is retained to strict
  4. Change FP to allow, page reloads and retains settings
  5. Change FP to standard from allow, sets it to strict

Actual result:

site-shields-settings

Expected result:

Site settings should override global settings and retain

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 1.12.25 Chromium: 83.0.4103.97 (Official Build) nightly (64-bit)
Revision 326d148b9655369b86498d9ecca39f63dd2bdd2d-refs/branch-heads/4103@{#657}
OS Linux

Version/Channel Information:

  • Can you reproduce this issue with the current release? NA
  • Can you reproduce this issue with the beta channel? Yes
  • Can you reproduce this issue with the dev channel? NA
  • Can you reproduce this issue with the nightly channel? Yes

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields? Yes
  • Does the issue resolve itself when disabling Brave Rewards? NA
  • Is the issue reproducible on the latest version of Chrome? NA

Miscellaneous Information:

@brave/uplift-approvers when fixed should be uplifted to beta

@srirambv srirambv added feature/global-settings Settings at browser level independent of shields settings feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields QA/Yes release-notes/exclude regression OS/Desktop labels Jun 16, 2020
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Jun 16, 2020
@rebron rebron added priority/P2 A bad problem. We might uplift this to the next planned release. and removed priority/P3 The next thing for us to work on. It'll ride the trains. labels Jun 16, 2020
@simonhong
Copy link
Member

simonhong commented Jun 17, 2020

When global FP is set to block, site FP could be allow or block. default becomes block.
When global FP is set to default, site FP could be allow, block or default.
When global FP is set to allow, site FP could be allow or block. default becomes allow.
That means, site FP's default always follow global FP.
When site FP has default, Preferences file doesn't include its setting. Because of this, global settings is applied. This is expected behavior because setting default means deleting current setting.
So, global setting is effective value. => Fixed by using secondary pattern(balanced) instead of using DEFAULT.

@LaurenWags
Copy link
Member

LaurenWags commented Jul 1, 2020

Verified passed with

Brave | 1.11.84 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
-- | --
Revision | 8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS | macOS Version 10.14.6 (Build 18G3020)
  • Verified STR from description on a clean profile. Confirmed working as expected. Confirmed able to change from Strict > Standard or Allow > Standard and setting was retained on page reload.
  • Reproduced the issue using 1.11.82 (Dev). Updated to 1.11.84 and confirmed working as expected.

Verified passed with

Brave	1.11.87 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
Revision	8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS	Linux
  • Verified STR from description on a clean profile. Confirmed working as expected. Confirmed able to change from Strict > Standard or Allow > Standard and setting was retained on page reload.

Verification passed on


Brave | 1.11.90 Chromium: 83.0.4103.116 (Official Build) dev (64-bit)
-- | --
Revision | 8f0c18b4dca9b6699eb629be0f51810c24fb6428-refs/branch-heads/4103@{#716}
OS | Windows 10 OS Version 1903 (Build 18362.900)
JavaScript | V8 8.3.110.13


  • Reproduced issue in 1.11.82 upgraded to 1.11.90 and ensured working as expected
  • Ensured working as expected in a clean profile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/global-settings Settings at browser level independent of shields settings feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes regression release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants