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.

Sign up for GitHub

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

Jump to bottom

Inconsistent naming: Site Settings vs Content settings #6627

Closed
computersaysyes opened this issue Oct 27, 2019 · 4 comments
Closed

Inconsistent naming: Site Settings vs Content settings #6627

computersaysyes opened this issue Oct 27, 2019 · 4 comments
Assignees
@mkarolin
Labels
feature/settings needs-text-change This change requires some careful wording. priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Milestone
1.3.x - Release

Comments

@computersaysyes
Copy link

The Advanced and On exit tabs on the Clear browsing data window have respectively a Site Settings and Content settings checkbox for the same thing.
Brave 0.70.121 Windows
@computersaysyes computersaysyes mentioned this issue Oct 27, 2019
Closed
@bsclifton
Copy link
Member

@computersaysyes just want to make sure I understand your concern- it's for these two tabs, right?

Advanced has Site settings
Screen Shot 2019-10-27 at 9 37 58 PM

On exit has Content settings
Screen Shot 2019-10-27 at 9 37 48 PM

cc: @mkarolin

@bsclifton bsclifton added feature/settings needs-text-change This change requires some careful wording. labels Oct 28, 2019
@rebron
Copy link
Collaborator

rebron commented Nov 15, 2019

@mkarolin Can we double check, are we deleting everything from site settings that's in content settings is that why we have the difference in naming or is this just a labelling issue?

@rebron rebron added the priority/P4 Planned work. We expect to get to it "soon". label Nov 15, 2019
@mkarolin
Copy link
Contributor

Looks like this is just a labeling issue. There used to be a flag that flipped between "Content Settings" and "Site Settings". The flag was removed upsteam (https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f). Because of that our code ended up defaulting to the "Content Settings" label.

@mkarolin mkarolin self-assigned this Nov 19, 2019
mkarolin added a commit to brave/brave-core that referenced this issue Nov 20, 2019
@mkarolin
Fixes a label in Clear Browsing Data dialog.
7544cba 
Fixes brave/brave-browser#6627

The label used to be set based on a flag that's no longer available
causing our label in OnExit tab to differ from the Advanced tab.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f

commit a3f57021dd6ac0f0df8e473da36f874d4132819f
Author: Maggie Cai <mxcai@chromium.org>
Date:   Wed Jul 17 01:03:22 2019 +0000

    Settings: Remove site-settings flag.

    All sites is launched in M74, this CL removes site-settings flag.

    BUG=980426
mkarolin added a commit to brave/brave-core that referenced this issue Nov 20, 2019
@mkarolin
Fixes a label in Clear Browsing Data dialog.
2a6c8d5 
Fixes brave/brave-browser#6627

The label used to be set based on a flag that's no longer available
causing our label in OnExit tab to differ from the Advanced tab.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f

commit a3f57021dd6ac0f0df8e473da36f874d4132819f
Author: Maggie Cai <mxcai@chromium.org>
Date:   Wed Jul 17 01:03:22 2019 +0000

    Settings: Remove site-settings flag.

    All sites is launched in M74, this CL removes site-settings flag.

    BUG=980426
mkarolin added a commit to brave/brave-core that referenced this issue Nov 23, 2019
@mkarolin
Fixes a label in Clear Browsing Data dialog.
9ace89e 
Fixes brave/brave-browser#6627

The label used to be set based on a flag that's no longer available
causing our label in OnExit tab to differ from the Advanced tab.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f

commit a3f57021dd6ac0f0df8e473da36f874d4132819f
Author: Maggie Cai <mxcai@chromium.org>
Date:   Wed Jul 17 01:03:22 2019 +0000

    Settings: Remove site-settings flag.

    All sites is launched in M74, this CL removes site-settings flag.

    BUG=980426
mkarolin added a commit to brave/brave-core that referenced this issue Nov 26, 2019
@mkarolin
Fixes a label in Clear Browsing Data dialog.
52546a4 
Fixes brave/brave-browser#6627

The label used to be set based on a flag that's no longer available
causing our label in OnExit tab to differ from the Advanced tab.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f

commit a3f57021dd6ac0f0df8e473da36f874d4132819f
Author: Maggie Cai <mxcai@chromium.org>
Date:   Wed Jul 17 01:03:22 2019 +0000

    Settings: Remove site-settings flag.

    All sites is launched in M74, this CL removes site-settings flag.

    BUG=980426
mkarolin added a commit to brave/brave-core that referenced this issue Nov 27, 2019
@mkarolin
Fixes a label in Clear Browsing Data dialog.
37949a2 
Fixes brave/brave-browser#6627

The label used to be set based on a flag that's no longer available
causing our label in OnExit tab to differ from the Advanced tab.

Chromium change:

https://chromium.googlesource.com/chromium/src/+/a3f57021dd6ac0f0df8e473da36f874d4132819f

commit a3f57021dd6ac0f0df8e473da36f874d4132819f
Author: Maggie Cai <mxcai@chromium.org>
Date:   Wed Jul 17 01:03:22 2019 +0000

    Settings: Remove site-settings flag.

    All sites is launched in M74, this CL removes site-settings flag.

    BUG=980426
@rebron rebron added this to the 1.3.x - Dev milestone Dec 17, 2019
@rebron rebron added the QA/Yes label Dec 17, 2019
@btlechowski
Copy link

btlechowski commented Jan 9, 2020 •
edited by LaurenWags
Loading

Verification passed on

Brave 1.3.85 Chromium: 79.0.3945.88 (Official Build) beta (64-bit)
Revision c2a58a36b9411c80829b4b154bfcab97e581f1f3-refs/branch-heads/3945@{#954}
OS Ubuntu 18.04 LTS

Verified #6627 (comment)

image
image

Verification passed on

Brave 1.3.87 Chromium: 79.0.3945.117 (Official Build) beta (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Windows 10 OS Version 1803 (Build 17134.1006)

Verified passed with

Brave 1.3.87 Chromium: 79.0.3945.117 (Official Build) beta (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.14.6 (Build 18G103)

Screen Shot 2020-01-09 at 1 40 42 PM

Screen Shot 2020-01-09 at 1 41 00 PM

@LaurenWags LaurenWags mentioned this issue Feb 7, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mkarolin mkarolin

Labels
feature/settings needs-text-change This change requires some careful wording. priority/P4 Planned work. We expect to get to it "soon". QA Pass-Linux QA Pass-macOS QA Pass-Win64 QA/Yes release-notes/include
Projects
None yet
Milestone
1.3.x - Release
Development

No branches or pull requests
7 participants
@bsclifton @rebron @LaurenWags @btlechowski @GeetaSarvadnya @mkarolin @computersaysyes