Skip to content

Commit

Permalink
Cookie Controls: Invert the cookie switch
Browse files Browse the repository at this point in the history 
Screenshot 1: https://screenshot.googleplex.com/7cqQNEdQtEJ9MWz.png
Screenshot 2: https://screenshot.googleplex.com/5mUjvi8vN96ZqZH.png

Bug: 1446230, b:285315027
Change-Id: I06e8eafe8ed61ac8abf0755155936f6b427e589e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4738358
Commit-Queue: Andrey Zaytsev <[email protected]>
Code-Coverage: Findit <[email protected]>
Reviewed-by: Christian Dullweber <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1179104}
  • Loading branch information
Andrey Zaytsev authored and Chromium LUCI CQ committed Aug 3, 2023
1 parent 89867a9 commit 3bf96cc
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 14 deletions.
4 changes: 2 additions & 2 deletions chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoViewTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -654,8 +654,8 @@ public void testClearCookiesOnSubpageUserBypass() throws Exception {
// Check that cookies usage is displayed.
onViewWaiting(allOf(withText(containsString("stored data")), isDisplayed()));
// Check that the cookie toggle is displayed and try clicking it.
onViewWaiting(allOf(withText(containsString("Block third-party cookies")), isDisplayed()));
onView(withText(containsString("Block third-party cookies"))).perform(click());
onViewWaiting(allOf(withText(containsString("Third-party cookies")), isDisplayed()));
onView(withText(containsString("Third-party cookies"))).perform(click());
// Clear cookies in page info.
onView(withText(containsString("stored data"))).perform(click());
onViewWaiting(allOf(withText("Delete"), isDisplayed()));
Expand Down
3 changes: 3 additions & 0 deletions components/browser_ui/strings/android/browser_ui_strings.grd
View file
Original file line number Diff line number Diff line change
Expand Up @@ -617,6 +617,9 @@
<message name="IDS_PAGE_INFO_COOKIES_SUBTITLE_BLOCKED" desc="" translateable="false">
Third-party cookies blocked
</message>
<message name="IDS_PAGE_INFO_COOKIES_THIRD_PARTY_COOKIES_LABEL" desc="" translateable="false">
Third-party cookies
</message>
<message name="IDS_PAGE_INFO_ANDROID_LOCATION_BLOCKED" desc="The label used in the Page Info dialog to indicate that location has been been disabled for the device in Android settings">
Turned off for this device
</message>
Expand Down
2 changes: 1 addition & 1 deletion components/page_info/android/java/res/xml/page_info_cookie_preference_user_bypass.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,6 @@ found in the LICENSE file.
<org.chromium.components.browser_ui.settings.ChromeSwitchPreference
android:key="cookie_switch"
android:persistent="false"
android:title="@string/page_info_cookies_block" />
android:title="@string/page_info_cookies_third_party_cookies_label" />

</PreferenceScreen>
8 changes: 4 additions & 4 deletions ...ge_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public View createViewForSubpage(ViewGroup parent) {
PageInfoCookiesPreference.PageInfoCookiesViewParams params =
new PageInfoCookiesPreference.PageInfoCookiesViewParams();
params.thirdPartyCookieBlockingEnabled = getDelegate().cookieControlsShown();
params.onCheckedChangedCallback = this::onCheckedChangedCallback;
params.onThirdPartyCookieToggleChanged = this::onThirdPartyCookieToggleChanged;
params.onClearCallback = this::onClearCookiesClicked;
params.onCookieSettingsLinkClicked = getDelegate()::showCookieSettings;
params.onFeedbackLinkClicked = getDelegate()::showCookieFeedback;
Expand Down Expand Up @@ -133,11 +133,11 @@ private void onStorageFetched(Collection<Website> result) {
}
}

private void onCheckedChangedCallback(boolean state) {
private void onThirdPartyCookieToggleChanged(boolean block) {
if (mBridge != null) {
mMainController.recordAction(state ? PageInfoAction.PAGE_INFO_COOKIES_BLOCKED_FOR_SITE
mMainController.recordAction(block ? PageInfoAction.PAGE_INFO_COOKIES_BLOCKED_FOR_SITE
: PageInfoAction.PAGE_INFO_COOKIES_ALLOWED_FOR_SITE);
mBridge.setThirdPartyCookieBlockingEnabledForSite(state);
mBridge.setThirdPartyCookieBlockingEnabledForSite(block);
}
}

Expand Down
29 changes: 22 additions & 7 deletions ...ge_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesPreference.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public class PageInfoCookiesPreference extends SiteSettingsPreferenceFragment {
public static class PageInfoCookiesViewParams {
// Called when the toggle controlling third-party cookie blocking changes.
public boolean thirdPartyCookieBlockingEnabled;
public Callback<Boolean> onCheckedChangedCallback;
public Callback<Boolean> onThirdPartyCookieToggleChanged;
public Runnable onClearCallback;
public Runnable onCookieSettingsLinkClicked;
public Callback<Activity> onFeedbackLinkClicked;
Expand Down Expand Up @@ -106,12 +106,17 @@ public void setParams(PageInfoCookiesViewParams params) {
// TODO(crbug.com/1077766): Set a ManagedPreferenceDelegate?
mCookieSwitch.setVisible(params.thirdPartyCookieBlockingEnabled);
mCookieSwitch.setOnPreferenceChangeListener((preference, newValue) -> {
params.onCheckedChangedCallback.onResult((Boolean) newValue);
boolean boolValue = (Boolean) newValue;
// Invert when the flag is on, since the switch is inverted.
if (PageInfoFeatures.USER_BYPASS_UI.isEnabled()) {
boolValue = !boolValue;
}
params.onThirdPartyCookieToggleChanged.onResult(boolValue);
return true;
});
boolean areAllCookiesBlocked = !WebsitePreferenceBridge.isCategoryEnabled(
getSiteSettingsDelegate().getBrowserContextHandle(), ContentSettingsType.COOKIES);
if (areAllCookiesBlocked) {
if (areAllCookiesBlocked && !PageInfoFeatures.USER_BYPASS_UI.isEnabled()) {
mCookieSwitch.setTitle(R.string.page_info_all_cookies_block);
}

Expand Down Expand Up @@ -152,6 +157,9 @@ private void showClearCookiesConfirmation() {

// Only used when UserBypassUI flag is off.
public void setCookieBlockingStatus(@CookieControlsStatus int status, boolean isEnforced) {
assert PageInfoFeatures.USER_BYPASS_UI.isEnabled()
: "This should only be invoked when UserBypassUI is enabled.";

boolean visible = status != CookieControlsStatus.DISABLED;
boolean enabled = status == CookieControlsStatus.ENABLED;
mCookieSwitch.setVisible(visible);
Expand All @@ -166,6 +174,9 @@ public void setCookieBlockingStatus(@CookieControlsStatus int status, boolean is
// Only used when UserBypassUI flag is on.
public void setCookieStatus(
@CookieControlsStatus int status, boolean isEnforced, long expiration) {
assert PageInfoFeatures.USER_BYPASS_UI.isEnabled()
: "This should only be invoked when UserBypassUI is enabled.";

boolean visible = status != CookieControlsStatus.DISABLED;
boolean blockingEnabled = status == CookieControlsStatus.ENABLED;

Expand All @@ -175,8 +186,10 @@ public void setCookieStatus(

if (!visible) return;

mCookieSwitch.setIcon(SettingsUtils.getTintedIcon(getContext(), R.drawable.ic_eye_crossed));
mCookieSwitch.setChecked(blockingEnabled);
mCookieSwitch.setIcon(SettingsUtils.getTintedIcon(getContext(),
blockingEnabled ? R.drawable.ic_visibility_off_black
: R.drawable.ic_visibility_black));
mCookieSwitch.setChecked(!blockingEnabled);
mCookieSwitch.setEnabled(!isEnforced);

boolean permanentException = (expiration == 0);
Expand Down Expand Up @@ -298,10 +311,12 @@ private void updateCookieDeleteButton() {
: R.color.default_icon_color_disabled);
}

// TODO(crbug.com/1446230): Invert the cookie switch.
// Only invoked when UserBypassUI is on.
private void updateCookieSwitch() {
assert PageInfoFeatures.USER_BYPASS_UI.isEnabled()
: "This should only be invoked when UserBypassUI is enabled.";
// TODO(crbug.com/1446230): Update the strings for when FPS are on.
if (mCookieSwitch.isChecked()) {
if (!mCookieSwitch.isChecked()) {
mCookieSwitch.setSummary(mBlockedSites > 0
? getContext().getResources().getQuantityString(
R.plurals.page_info_sites_blocked, mBlockedSites, mBlockedSites)
Expand Down

0 comments on commit 3bf96cc

Please sign in to comment.