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

Update 1.27.108 causes breakage in dark mode #17139

Closed
youssef20031 opened this issue Jul 23, 2021 · 7 comments · Fixed by brave/brave-core#9735
Closed

Update 1.27.108 causes breakage in dark mode #17139

youssef20031 opened this issue Jul 23, 2021 · 7 comments · Fixed by brave/brave-core#9735
Assignees
Labels
feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields feature/themes OS/Desktop QA Pass-macOS QA/Yes release-notes/exclude

Comments

@youssef20031
Copy link

youssef20031 commented Jul 23, 2021

Description

After the update error screens show up as white despite me using dark mode in brave

Steps to Reproduce

  1. Update
  2. Simulate an error screen like no internet

Actual result:

Screenshot from 2021-07-23 20-00-17

Expected result:

the same picture but with dark mode

Reproduces how often:

Easy

Brave version (brave://version info)

Brave 1.27.108 Chromium: 92.0.4515.107 (Official Build) (64-bit)
Revision 87a818b10553a07434ea9e2b6dccf3cbe7895134-refs/branch-heads/4515@{#1634}
OS Ubuntu 20.04.2.0 LTS
JavaScript V8 9.2.230.20
User Agent Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.107 Safari/537.36

Version/Channel Information:

  • Can you reproduce this issue with the current release?
  • Yes
  • Can you reproduce this issue with the beta channel?
  • N/A
  • Can you reproduce this issue with the nightly channel?
  • N/A

Other Additional Information:

  • Does the issue resolve itself when disabling Brave Shields?
  • Yes, Fingerprinting protection is affecting local page
  • Does the issue resolve itself when disabling Brave Rewards?
  • No
  • Is the issue reproducible on the latest version of Chrome?
  • N/A

Miscellaneous Information:

@supernes
Copy link

supernes commented Jul 24, 2021

It's not just error screens, every website that uses media queries to detect dark mode is intentionally broken as of the last update with max fingerprinting protection enabled.

I know it's hard to find a balance between protection and usability, but in this specific case I think it's not justified to break what's basically an accessibility feature for a questionable amount of privacy gains.

Even with strict fingerprinting protection, most websites work fine (apart from WebGL heavy sites, where it makes sense to downgrade protections). This change on the other hand has a very visible effect on many sites people visit every day.

I'm not sure any site uses this particular signal for fingerprinting, or that they can gain much from it. Libraries like fingerprintjs use other features like prefers-reduced-motion and prefers-contrast, which would have much less of an impact if restricted.

Please reconsider this change, and/or add a way for users to override it.

@Tonev
Copy link
Contributor

Tonev commented Jul 24, 2021

I confirm the update broke websites' functionality to change their theme based on users' theme preferences.

Steps to reproduce:

  1. Go to brave://settings/appearance and set Brave colors to Dark.
  2. Open https://9gag.com/.
  3. Click on the Brave Shields icon in the address bar and set Fingerprinting blocked (strict, may break sites).

Website's theme will reset to light. Clicking on the Brave Shields icon in the address bar and setting Fingerprinting blocked (standard) will once again change the website's theme to dark, successfully enforcing a theme based on user's preferences.

CC: @ShivanKaul as an author of the changes, and @pes10k as someone who can provide the best approach in such cases 👍

@kjozwiak kjozwiak added feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields feature/themes QA/Yes labels Jul 24, 2021
@ShivanKaul
Copy link
Collaborator

The dark mode detection block checks for shields up and fingerprinting mode strict. I see that on error pages (no wifi, DNS lookup failed) we still have Shields up. Will check with @pes10k about the right thing to do here.

@ShivanKaul
Copy link
Collaborator

I added a feature flag in brave/brave-core#9735 that allows opting-out of this fingerprinting protection

@ShivanKaul ShivanKaul added this to the 1.30.x - Nightly milestone Aug 18, 2021
@stephendonner
Copy link

@ShivanKaul mind adding a succinct test plan for this? Thanks!

@stephendonner
Copy link

Verified PASSED using

Brave 1.30.45 Chromium: 93.0.4577.51 (Official Build) nightly (x86_64)
Revision 762d21050e2da59930c784c09b134d0b0b148188-refs/branch-heads/4577@{#915}
OS macOS Version 11.5.2 (Build 20G95)

Steps:

  1. clean profile
  2. launched Brave
  3. reproduced the issue in Update 1.27.108 causes breakage in dark mode #17139 (comment) by toggling between strict, may break site and standard modes for fingerprinting blocking with the Default (Enabled) value for Enable dark mode blocking fingerprinting protection in brave://flags
  4. next, set Enable dark mode blocking fingerprinting protection to Disabled in brave://flags and restarted
  5. repeated the steps in Update 1.27.108 causes breakage in dark mode #17139 (comment) but confirmed I was able to toggle between strict, may break site and standard modes for fingerprinting blocking without changing the site's background color

Default (Enabled)

example example example
Screen Shot 2021-08-24 at 3 48 39 PM Screen Shot 2021-08-24 at 3 48 25 PM Screen Shot 2021-08-24 at 3 48 32 PM

Disabled

example example example
Screen Shot 2021-08-24 at 3 42 37 PM Screen Shot 2021-08-24 at 3 49 48 PM Screen Shot 2021-08-24 at 3 49 53 PM

@ShivanKaul
Copy link
Collaborator

@stephendonner was on PTO, hence late response. Your test ^ looks good + we can re-use the tests from #15265 (comment) and just check if the feature flag works as intended (i.e. on disabling the feature flag, the protection shouldn't apply and dark mode should work again)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/shields/fingerprint The fingerprinting (aka: "device recognition") protection provided in Shields feature/themes OS/Desktop QA Pass-macOS QA/Yes release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants