From 3bf96ccdde4c706c913f37fe5fd7a467a518536d Mon Sep 17 00:00:00 2001 From: Andrey Zaytsev Date: Thu, 3 Aug 2023 17:10:58 +0000 Subject: [PATCH] Cookie Controls: Invert the cookie switch 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 Code-Coverage: Findit Reviewed-by: Christian Dullweber Cr-Commit-Position: refs/heads/main@{#1179104} --- .../browser/page_info/PageInfoViewTest.java | 4 +-- .../strings/android/browser_ui_strings.grd | 3 ++ ...age_info_cookie_preference_user_bypass.xml | 2 +- .../page_info/PageInfoCookiesController.java | 8 ++--- .../page_info/PageInfoCookiesPreference.java | 29 ++++++++++++++----- 5 files changed, 32 insertions(+), 14 deletions(-) diff --git a/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoViewTest.java b/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoViewTest.java index d8c8cd95628fec..ed1aa00ebf16e2 100644 --- a/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoViewTest.java +++ b/chrome/android/javatests/src/org/chromium/chrome/browser/page_info/PageInfoViewTest.java @@ -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())); diff --git a/components/browser_ui/strings/android/browser_ui_strings.grd b/components/browser_ui/strings/android/browser_ui_strings.grd index 033e2d2aed7d4e..d56fa8ffae4be1 100644 --- a/components/browser_ui/strings/android/browser_ui_strings.grd +++ b/components/browser_ui/strings/android/browser_ui_strings.grd @@ -617,6 +617,9 @@ Third-party cookies blocked + + Third-party cookies + Turned off for this device diff --git a/components/page_info/android/java/res/xml/page_info_cookie_preference_user_bypass.xml b/components/page_info/android/java/res/xml/page_info_cookie_preference_user_bypass.xml index 8de049c57edf57..5dfbebb3cbcd65 100644 --- a/components/page_info/android/java/res/xml/page_info_cookie_preference_user_bypass.xml +++ b/components/page_info/android/java/res/xml/page_info_cookie_preference_user_bypass.xml @@ -31,6 +31,6 @@ found in the LICENSE file. + android:title="@string/page_info_cookies_third_party_cookies_label" /> diff --git a/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java b/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java index ec1e57312b3c36..dc3d43a6b268cd 100644 --- a/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java +++ b/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesController.java @@ -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; @@ -133,11 +133,11 @@ private void onStorageFetched(Collection 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); } } diff --git a/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesPreference.java b/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesPreference.java index 82a5eb43b9e11c..1623ba3e43bbee 100644 --- a/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesPreference.java +++ b/components/page_info/android/java/src/org/chromium/components/page_info/PageInfoCookiesPreference.java @@ -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 onCheckedChangedCallback; + public Callback onThirdPartyCookieToggleChanged; public Runnable onClearCallback; public Runnable onCookieSettingsLinkClicked; public Callback onFeedbackLinkClicked; @@ -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); } @@ -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); @@ -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; @@ -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); @@ -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)