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

Shields should show Simple view by default for new users + browser profiles #8533

Closed
karenkliu opened this issue Mar 4, 2020 · 8 comments · Fixed by brave/brave-core#4860
Assignees
Labels
feature/shields The overall Shields feature in Brave. priority/P3 The next thing for us to work on. It'll ride the trains. QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Test-Plan-Specified QA/Yes release-notes/include

Comments

@karenkliu
Copy link

karenkliu commented Mar 4, 2020

Description

Currently, new users and browser profile see the Advanced view of Shields by default, but it should show Simple. Seems to be a bug that broke #1196 because it was working correctly before.

Test plan / Steps to Reproduce

  1. Open a new browser profile or destroy your profile directory for Brave
  2. Launch into Brave
  3. Visit brave://settings/shields and confirm Simple view is selected
  4. Quit Brave and relaunch
  5. Visit brave://settings/shields and Advanced view is selected?? 🤔

Actual result:

User ends up seeing Advanced view for their first time ☹️
Screen Shot 2020-03-04 at 11 59 34 AM

The Shields global settings in browser preferences also shows Advanced view as the default:
Screen Shot 2020-03-04 at 12 00 46 PM

Expected result:

Shields should show simple view by default:
Screen Shot 2020-03-04 at 12 02 22 PM

Reproduces how often:

Always

Brave version (brave://version info)

All versions

@rebron rebron added regression feature/shields The overall Shields feature in Brave. labels Mar 4, 2020
@rebron rebron added the priority/P3 The next thing for us to work on. It'll ride the trains. label Mar 4, 2020
@bsclifton
Copy link
Member

I made changes related to this, but it was signed off and working in 0.69.x:
brave/brave-core#3154

Basically, when profile is checked, it will look to see if the user is on their first run (ex: no profile directory exists). If no profile directory exists, it defaults to simple mode.

@bsclifton
Copy link
Member

@karenkliu @rebron unable to reproduce with a fresh profile. I also tried creating a new profile from inside the browser (Hamburger => Create New Profile). Both showed simple mode

I don't think there's a bug- can you verify please?

@bsclifton
Copy link
Member

OK bug identified - let me update the steps in the original post 😄

@bsclifton bsclifton assigned bsclifton and unassigned rebron Mar 4, 2020
@bsclifton bsclifton added this to the 1.7.x - Nightly milestone Mar 4, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Mar 4, 2020
Logic introduced with #3154 did properly set the value for first run, but the value didn't seem to be persisted to profile. When quitting and relaunching, it would evaluate and make default advanced (not simple).

Since all users have a value for this (and most of them have it advanced = true), it's safe to roll this out. New users will have simple view by default.

Fixes brave/brave-browser#8533

-----

Revert "Merge pull request #3154 from brave/bsc-shields-advanced-default-change"

This reverts commit 590da54, reversing
changes made to b295203.
@bsclifton bsclifton modified the milestone: 1.7.x - Nightly Mar 5, 2020
bsclifton added a commit to brave/brave-core that referenced this issue Mar 5, 2020
Defaults to:
- advanced for existing users
- simple for new users

Changing the default was not "locking in" the setting. This change
locks in that default.

Fixes brave/brave-browser#8533
bsclifton added a commit to brave/brave-core that referenced this issue Mar 10, 2020
Defaults to:
- advanced for existing users
- simple for new users

Changing the default was not "locking in" the setting. This change
locks in that default.

Fixes brave/brave-browser#8533
@bsclifton
Copy link
Member

Turns out this wasn't a regression - it was a bug. Was broken even with initial implementation

@karenkliu
Copy link
Author

@bsclifton 😱 Ah! So Shields has been showing Advanced by default to new users for the last 5 months??

@bsclifton
Copy link
Member

@karenkliu unfortunately, yes ☹️ As of today, this will be in Beta- so it'll be coming soon

@karenkliu
Copy link
Author

@bsclifton ok, thanks for the update and hustling to fix it! 😬

@kjozwiak
Copy link
Member

kjozwiak commented Mar 24, 2020

Verification PASSED on macOS 10.15.13 x64 Catalina using the following build:

Brave | 1.7.71 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
-- | --
Revision | 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS | macOS Version 10.15.3 (Build 19D76)
  • Verified the STR outlined via Shields should show Simple view by default for new users + browser profiles #8533 (comment)
  • ensured that Simple View was being used after several restarts
  • ensured that switching to Advanced View via brave://settings/shields worked
    • ensured that the shields display the Changing Shield settings in this view may affect web compatibility on this site. when initially changing to Advanced View
    • ensured that the Advanced View is kept selected after restarting Brave several times
  • ensured that switching between Simple View & Advanced View via the shields works as expected
    • ensured that brave://settings/shields also reflects the changes
  • ensured that switching between Simple View & Advanced View via brave://settings/shields

Verification passed on

Brave 1.7.71 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Ubuntu 18.04 LTS

Verified test plan from the description

  • Verified that Simple View was being used after several restarts
  • Verified that switching to Advanced View via brave://settings/shields worked
    • Verified that the shields display the Changing Shield settings in this view may affect web compatibility on this site. when initially changing to Advanced View
    • Verified that the Advanced View is kept selected after restarting Brave several times
  • Verified that switching between Simple View & Advanced View via the shields works as expected
    • Verified that brave://settings/shields also reflects the changes
  • Verified that switching between Simple View & Advanced View via brave://settings/shields

Verification passed on

Brave 1.7.73 Chromium: 80.0.3987.149 (Official Build) dev (64-bit)
Revision 5f4eb224680e5d7dca88504586e9fd951840cac6-refs/branch-heads/3987_137@{#16}
OS Windows 10 OS Version 1803 (Build 17134.1006)
  • Verified the STR outlined via Shields should show Simple view by default for new users + browser profiles #8533 (comment)
  • ensured that Simple View was being used after several restarts
  • ensured that switching to Advanced View via brave://settings/shields worked
    • ensured that the shields display the Changing Shield settings in this view may affect web compatibility on this site. when initially changing to Advanced View
    • ensured that the Advanced View is kept selected after restarting Brave several times
  • ensured that switching between Simple View & Advanced View via the shields works as expected
    • ensured that brave://settings/shields also reflects the changes
  • ensured that switching between Simple View & Advanced View via brave://settings/shields

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment