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

Warning text if the user has currently selected Block all cookies #27090

Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

import android.content.SharedPreferences;
import android.os.Bundle;
import android.text.SpannableString;
import android.text.style.ForegroundColorSpan;

import androidx.preference.Preference;
import androidx.preference.PreferenceCategory;
Expand All @@ -33,18 +35,25 @@
import org.chromium.chrome.browser.settings.BravePreferenceDialogFragment;
import org.chromium.chrome.browser.settings.BraveWebrtcPolicyPreference;
import org.chromium.chrome.browser.shields.FilterListServiceFactory;
import org.chromium.chrome.browser.util.TabUtils;
import org.chromium.chrome.browser.webcompat_reporter.WebcompatReporterServiceFactory;
import org.chromium.components.browser_ui.settings.ChromeBasePreference;
import org.chromium.components.browser_ui.settings.ChromeSwitchPreference;
import org.chromium.components.browser_ui.settings.SettingsUtils;
import org.chromium.components.browser_ui.settings.TextMessagePreference;
import org.chromium.components.user_prefs.UserPrefs;
import org.chromium.gms.ChromiumPlayServicesAvailability;
import org.chromium.mojo.bindings.ConnectionErrorHandler;
import org.chromium.mojo.system.MojoException;
import org.chromium.ui.text.NoUnderlineClickableSpan;
import org.chromium.ui.text.SpanApplier;
import org.chromium.webcompat_reporter.mojom.WebcompatReporterHandler;

/** Fragment to keep track of the all the brave privacy related preferences. */
public class BravePrivacySettings extends PrivacySettings implements ConnectionErrorHandler {
private static final String BLOCK_ALL_COOKIES_LEARN_MORE_LINK =
"https://github.com/brave/brave-browser/wiki/Block-all-cookies-global-Shields-setting";

// Chromium Prefs
private static final String PREF_CAN_MAKE_PAYMENT = "can_make_payment";
private static final String PREF_NETWORK_PREDICTIONS = "preload_pages";
Expand Down Expand Up @@ -102,6 +111,8 @@ public class BravePrivacySettings extends PrivacySettings implements ConnectionE

public static final String PREF_BLOCK_TRACKERS_ADS = "block_trackers_ads";
private static final String PREF_BLOCK_CROSS_SITE_COOKIES = "block_cross_site_cookies";
private static final String BLOCK_CROSS_SITE_COOKIES_LEARN_MORE =
"block_all_cross_site_cookies_learn_more";
private static final String PREF_SHIELDS_SUMMARY = "shields_summary";
private static final String PREF_CLEAR_ON_EXIT = "clear_on_exit";
private static final String PREF_HTTPS_UPGRADE = "https_upgrade";
Expand All @@ -117,6 +128,7 @@ public class BravePrivacySettings extends PrivacySettings implements ConnectionE
PREF_HTTPS_FIRST_MODE,
PREF_BLOCK_SCRIPTS,
PREF_BLOCK_CROSS_SITE_COOKIES,
BLOCK_CROSS_SITE_COOKIES_LEARN_MORE,
PREF_FINGERPRINTING_PROTECTION,
PREF_FINGERPRINTING_PROTECTION2,
PREF_FINGERPRINT_LANGUAGE,
Expand Down Expand Up @@ -163,6 +175,7 @@ public class BravePrivacySettings extends PrivacySettings implements ConnectionE
private ChromeSwitchPreference mCanMakePayment;
private BraveDialogPreference mAdsTrakersBlockPref;
private BraveDialogPreference mBlockCrosssiteCookies;
private TextMessagePreference mBlockCrosssiteCookiesLearnMore;
private ChromeSwitchPreference mDeAmpPref;
private ChromeSwitchPreference mDebouncePref;
private ChromeSwitchPreference mHttpsFirstModePref;
Expand Down Expand Up @@ -280,6 +293,32 @@ public void onCreatePreferences(Bundle savedInstanceState, String rootKey) {
(BraveDialogPreference) findPreference(PREF_BLOCK_CROSS_SITE_COOKIES);
mBlockCrosssiteCookies.setOnPreferenceChangeListener(this);

mBlockCrosssiteCookiesLearnMore =
(TextMessagePreference) findPreference(BLOCK_CROSS_SITE_COOKIES_LEARN_MORE);
if (mBlockCrosssiteCookiesLearnMore != null) {
SpannableString learnMoreDesc =
SpanApplier.applySpans(
getString(R.string.block_cookies_deprecated_label),
new SpanApplier.SpanInfo(
"<LINK_0>",
"</LINK_0>",
new ForegroundColorSpan(
getContext()
.getColor(R.color.wallet_error_text_color))),
new SpanApplier.SpanInfo(
"<LINK_1>",
"</LINK_1>",
new NoUnderlineClickableSpan(
requireContext(),
R.color.brave_link,
result -> {
TabUtils.openUrlInCustomTab(
requireContext(),
BLOCK_ALL_COOKIES_LEARN_MORE_LINK);
})));
mBlockCrosssiteCookiesLearnMore.setSummary(learnMoreDesc);
}

mBlockScriptsPref = (ChromeSwitchPreference) findPreference(PREF_BLOCK_SCRIPTS);
mBlockScriptsPref.setOnPreferenceChangeListener(this);

Expand Down Expand Up @@ -511,6 +550,8 @@ public boolean onPreferenceChange(Preference preference, Object newValue) {
0,
ChromeFeatureList.isEnabled(BraveFeatureList.BRAVE_BLOCK_ALL_COOKIES_TOGGLE)
|| (int) newValue == STRICT);
mBlockCrosssiteCookiesLearnMore.setVisible(STRICT == (int) newValue);
mBlockCrosssiteCookiesLearnMore.setDividerAllowedAbove(false);
switch ((int) newValue) {
case STRICT:
BraveShieldsContentSettings.setCookiesPref(
Expand Down Expand Up @@ -642,6 +683,8 @@ private void updateBravePreferences() {
0,
ChromeFeatureList.isEnabled(BraveFeatureList.BRAVE_BLOCK_ALL_COOKIES_TOGGLE)
|| cookiesBlockPref == STRICT);
mBlockCrosssiteCookiesLearnMore.setVisible(STRICT == (int) cookiesBlockPref);
mBlockCrosssiteCookiesLearnMore.setDividerAllowedAbove(false);
if (cookiesBlockPref == STRICT) {
mBlockCrosssiteCookies.setCheckedIndex(0);
mBlockCrosssiteCookies.setSummary(
Expand Down
5 changes: 5 additions & 0 deletions android/java/res/xml/brave_privacy_preferences.xml
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@
app:dialog_subtitle="@string/block_cookies_text"
app:dialog_entries="@array/blockCookiesTexts"
app:dialog_default_index="1" />
<!-- A description with hyperlink to explain more about block cross site cookies settings. -->
<org.chromium.components.browser_ui.settings.TextMessagePreference
app:icon="@drawable/ic_warning"
android:key="block_all_cross_site_cookies_learn_more"
app:iconTint="@color/wallet_error_text_color"/>
<org.chromium.chrome.browser.settings.BraveDialogPreference
android:key="fingerprinting_protection"
android:title="@string/block_fingerprinting"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ public class BravePrivacySettingsTest {
private static final String PREF_INCOGNITO_LOCK = "incognito_lock";
private static final String PREF_PHONE_AS_A_SECURITY_KEY = "phone_as_a_security_key";

private static final int BRAVE_PRIVACY_SETTINGS_NUMBER_OF_ITEMS = 29;
private static final int BRAVE_PRIVACY_SETTINGS_NUMBER_OF_ITEMS = 30;

private int mItemsLeft;

Expand Down
3 changes: 3 additions & 0 deletions app/brave_settings_strings.grdp
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,9 @@
<message name="IDS_SETTINGS_BRAVE_SHIELDS_COOKIE_CONTROL_LABEL" desc="Default Brave cookies control setting label">
Block cookies
</message>
<message name="IDS_SETTINGS_BRAVE_SHIELDS_COOKIE_CONTROL_BLOCK_ALL_DEPRECATED_LABEL" desc="Block all cookies deprecation warning">
The "Block all Cookies" option has been deprecated. To learn how to force-enable this option, visit <ph name="BEGIN_LINK">&lt;a target="_blank" href="$1"&gt;</ph>our help page<ph name="END_LINK">&lt;/a&gt;</ph>
</message>
<message name="IDS_SETTINGS_BLOCK_3RD_PARTY_COOKIES" desc="Select value">
Block third-party cookies
</message>
Expand Down
6 changes: 4 additions & 2 deletions browser/about_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -691,8 +691,10 @@
}, \
{ \
"block-all-cookies-toggle", \
"If the feature flag is on, we show the Block all Cookies toggle", \
"If the feature flag is on, we show the Block all Cookies toggle", \
"'Block all cookies' option in Shields global defaults", \
"Shows the 'Block all cookies' toggle in Shields global defaults. " \
"This global setting prevents all websites from storing cookies " \
"on your device, but is also very likely to lead to site breakage.", \
kOsAll, \
FEATURE_VALUE_TYPE(brave_shields::features::kBlockAllCookiesToggle), \
}, \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@
cursor: pointer;
margin: 0 20px;
}
.block-all-cookies-warn {
cursor: pointer;
color: red;
}
</style>
<div hidden="[[isAdBlockRoute_]]">
<div class="settings-box first">
Expand Down Expand Up @@ -94,7 +98,12 @@
on-settings-boolean-control-change="onFingerprintingToggleControlChange_">
</settings-toggle-button>
<div class="settings-box">
<div class="start">$i18n{cookieControlLabel}</div>
<div class="start">
<div class="label">$i18n{cookieControlLabel}</div>
<div class="cr-secondary-text label block-all-cookies-warn" hidden="[[!controlEqual_('block', cookieControlType_)]]">
<span>$i18nRaw{blockAllCookiesDeprecatedLabel}</span>
</div>
</div>
<select id="cookieControlType" class="md-select"
on-change="onCookieControlChange_">
<template is="dom-repeat" items="[[cookieControlTypes_]]">
Expand Down
3 changes: 3 additions & 0 deletions browser/ui/android/strings/android_brave_strings.grd
Original file line number Diff line number Diff line change
Expand Up @@ -1549,6 +1549,9 @@ Are you sure you want to do this?
<message name="IDS_BLOCK_TRACKERS_ADS_OPTION_3" desc="Text for block trackers and ads option.">
Allow all trackers &amp; ads
</message>
<message name="IDS_BLOCK_COOKIES_DEPRECATED_LABEL" desc="Block all cookies deprecation warning">
<ph name="BEGIN_LINK_0">&lt;LINK_0&gt;</ph>The "Block all Cookies" option has been deprecated. To learn how to force-enable this option, visit<ph name="END_LINK_0">&lt;/LINK_0&gt;</ph> <ph name="BEGIN_LINK_1">&lt;LINK_1&gt;</ph>our help page<ph name="END_LINK_1">&lt;/LINK_1&gt;</ph>
</message>
<message name="IDS_BLOCK_COOKIES" desc="Title for shields option.">
Block Cookies
</message>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ constexpr char16_t kDebounceLearnMoreUrl[] =
constexpr char16_t kEnableNftDiscoveryLearnMoreUrl[] =
u"https://github.com/brave/brave-browser/wiki/"
u"NFT-Discovery";
constexpr char16_t kBlockAllCookiesLearnMoreUrl[] =
u"https://github.com/brave/brave-browser/wiki/"
u"Block-all-cookies-global-Shields-setting";

void BraveAddCommonStrings(content::WebUIDataSource* html_source,
Profile* profile) {
Expand Down Expand Up @@ -968,6 +971,12 @@ void BraveAddCommonStrings(content::WebUIDataSource* html_source,
"ensOffchainLookupDesc",
l10n_util::GetStringFUTF16(IDS_SETTINGS_ENABLE_ENS_OFFCHAIN_LOOKUP_DESC,
kEnsOffchainLookupLearnMoreURL));

html_source->AddString(
"blockAllCookiesDeprecatedLabel",
l10n_util::GetStringFUTF16(
IDS_SETTINGS_BRAVE_SHIELDS_COOKIE_CONTROL_BLOCK_ALL_DEPRECATED_LABEL,
kBlockAllCookiesLearnMoreUrl));
} // NOLINT(readability/fn_size)

void BraveAddResources(content::WebUIDataSource* html_source,
Expand Down
35 changes: 33 additions & 2 deletions browser/webcompat_reporter/webcompat_reporter_service_delegate.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,23 @@

#include "brave/common/brave_channel_info.h"
#include "brave/components/brave_shields/content/browser/ad_block_service.h"
#include "brave/components/brave_shields/content/browser/brave_shields_util.h"
#include "brave/components/brave_shields/core/browser/ad_block_component_service_manager.h"
#include "brave/components/brave_shields/core/browser/filter_list_catalog_entry.h"
#include "brave/components/webcompat_reporter/browser/webcompat_reporter_utils.h"
#include "components/component_updater/component_updater_service.h"

namespace webcompat_reporter {

WebcompatReporterServiceDelegateImpl::WebcompatReporterServiceDelegateImpl(
component_updater::ComponentUpdateService* component_update_service,
brave_shields::AdBlockService* adblock_service)
brave_shields::AdBlockService* adblock_service,
HostContentSettingsMap* host_content_settings_map,
scoped_refptr<content_settings::CookieSettings> content_settings)
: WebcompatReporterServiceDelegateBase(component_update_service),
adblock_service_(adblock_service) {}
adblock_service_(adblock_service),
host_content_settings_map_(host_content_settings_map),
cookie_settings_(content_settings) {}

WebcompatReporterServiceDelegateImpl::~WebcompatReporterServiceDelegateImpl() =
default;
Expand Down Expand Up @@ -53,4 +59,29 @@ WebcompatReporterServiceDelegateImpl::GetChannelName() const {
return brave::GetChannelName();
}

std::optional<std::string>
WebcompatReporterServiceDelegateImpl::GetCookiePolicy() const {
DCHECK(host_content_settings_map_);
DCHECK(cookie_settings_);
if (!host_content_settings_map_ || !cookie_settings_) {
return std::nullopt;
}

return brave_shields::ControlTypeToString(brave_shields::GetCookieControlType(
host_content_settings_map_, cookie_settings_.get(), GURL()));
}

std::optional<std::string>
WebcompatReporterServiceDelegateImpl::GetScriptBlockingFlag() const {
DJAndries marked this conversation as resolved.
Show resolved Hide resolved
DCHECK(host_content_settings_map_);
DCHECK(cookie_settings_);
if (!host_content_settings_map_ || !cookie_settings_) {
return std::nullopt;
}

return BoolToString(brave_shields::GetNoScriptControlType(
host_content_settings_map_, GURL()) ==
brave_shields::ControlType::BLOCK);
}

} // namespace webcompat_reporter
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include <vector>

#include "brave/components/webcompat_reporter/browser/webcompat_reporter_service_delegate_base.h"
#include "components/content_settings/core/browser/cookie_settings.h"
#include "components/content_settings/core/browser/host_content_settings_map.h"

namespace brave_shields {
class AdBlockService;
Expand All @@ -22,7 +24,9 @@ class WebcompatReporterServiceDelegateImpl
public:
explicit WebcompatReporterServiceDelegateImpl(
component_updater::ComponentUpdateService* component_update_service,
brave_shields::AdBlockService* adblock_service);
brave_shields::AdBlockService* adblock_service,
HostContentSettingsMap* host_content_settings_map,
scoped_refptr<content_settings::CookieSettings> content_settings);
WebcompatReporterServiceDelegateImpl(
const WebcompatReporterServiceDelegateImpl&) = delete;
WebcompatReporterServiceDelegateImpl& operator=(
Expand All @@ -32,9 +36,13 @@ class WebcompatReporterServiceDelegateImpl
std::optional<std::vector<std::string>> GetAdblockFilterListNames()
const override;
std::optional<std::string> GetChannelName() const override;
std::optional<std::string> GetCookiePolicy() const override;
std::optional<std::string> GetScriptBlockingFlag() const override;

private:
const raw_ptr<brave_shields::AdBlockService> adblock_service_;
const raw_ptr<HostContentSettingsMap> host_content_settings_map_;
scoped_refptr<content_settings::CookieSettings> cookie_settings_;
};

} // namespace webcompat_reporter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
#include "brave/browser/webcompat_reporter/webcompat_reporter_service_delegate.h"
#include "brave/components/webcompat_reporter/browser/webcompat_reporter_service.h"
#include "chrome/browser/browser_process.h"
#include "chrome/browser/content_settings/cookie_settings_factory.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/profiles/incognito_helpers.h"
#include "chrome/browser/profiles/profile.h"
#include "components/keyed_service/content/browser_context_dependency_manager.h"
Expand Down Expand Up @@ -53,6 +55,7 @@ WebcompatReporterServiceFactory::~WebcompatReporterServiceFactory() = default;
std::unique_ptr<KeyedService>
WebcompatReporterServiceFactory::BuildServiceInstanceForBrowserContext(
content::BrowserContext* context) const {
DCHECK(context);
auto* default_storage_partition = context->GetDefaultStoragePartition();
if (!default_storage_partition) {
return nullptr;
Expand All @@ -62,11 +65,15 @@ WebcompatReporterServiceFactory::BuildServiceInstanceForBrowserContext(
!profile || profile->IsOffTheRecord() ? nullptr : profile->GetPrefs();
auto report_uploader = std::make_unique<WebcompatReportUploader>(
default_storage_partition->GetURLLoaderFactoryForBrowserProcess());

return std::make_unique<WebcompatReporterService>(
prefs,
std::make_unique<WebcompatReporterServiceDelegateImpl>(
g_browser_process->component_updater(),
g_brave_browser_process->ad_block_service()),
g_brave_browser_process->ad_block_service(),
HostContentSettingsMapFactory::GetForProfile(context),
CookieSettingsFactory::GetForProfile(
Profile::FromBrowserContext(context))),
std::move(report_uploader));
}

Expand Down
2 changes: 2 additions & 0 deletions components/webcompat_reporter/browser/fields.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ inline constexpr char kLanguageFarblingField[] = "languageFarblingEnabled";
inline constexpr char kBraveVPNEnabledField[] = "braveVPNEnabled";
inline constexpr char kChannelField[] = "channel";
inline constexpr char kVersionField[] = "version";
inline constexpr char kCookiePolicyField[] = "cookie_policy";
inline constexpr char kBlockScriptsField[] = "block_scripts";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add these fields on the server-side. Please remember to update https://github.com/brave/brave-browser/wiki/Web-compatibility-reports


inline constexpr char kSiteURLField[] = "url";
inline constexpr char kDomainField[] = "domain";
Expand Down
10 changes: 10 additions & 0 deletions components/webcompat_reporter/browser/webcompat_report_uploader.cc
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@ void WebcompatReportUploader::SubmitReport(mojom::ReportInfoPtr report_info) {
report_info->brave_vpn_connected.value() == kStringTrue);
}

if (report_info->cookie_policy) {
report_details_dict.Set(kCookiePolicyField,
report_info->cookie_policy.value());
}

if (report_info->block_scripts) {
report_details_dict.Set(kBlockScriptsField,
report_info->block_scripts.value() == kStringTrue);
}

report_details_dict.Set(kApiKeyField, base::Value(api_key));

std::string report_details_json;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,8 @@ TEST_F(WebcompatReportUploaderUnitTest, GenerateReport) {
auto report = webcompat_reporter::mojom::ReportInfo::New(
"dev", "1.231.45", "https://abc.url/p1/p2", "true", "ad_block_setting",
"fp_block_setting", "ad_block_list_names", "languages", "true", "true",
"details", "contact", std::move(components), std::nullopt);
"details", "contact", "block", "true", std::move(components),
std::nullopt);

auto report_copy = report->Clone();

Expand All @@ -120,9 +121,11 @@ TEST_F(WebcompatReportUploaderUnitTest, GenerateReport) {
"adBlockSetting": "%s",
"additionalDetails": "%s",
"api_key": "%s",
"block_scripts": "%s",
"braveVPNEnabled": "%s",
"channel": "%s",
"contactInfo": "%s",
"cookie_policy": "%s",
"domain": "%s",
"fpBlockSetting": "%s",
"languageFarblingEnabled": "%s",
Expand All @@ -134,9 +137,10 @@ TEST_F(WebcompatReportUploaderUnitTest, GenerateReport) {
R"([{"id": "id", "name": "name", "version": "version"}])",
report_copy->ad_block_list_names->c_str(),
report_copy->ad_block_setting->c_str(), report_copy->details->c_str(),
brave_stats::GetAPIKey().c_str(),
brave_stats::GetAPIKey().c_str(), report_copy->block_scripts->c_str(),
report_copy->brave_vpn_connected ? "true" : "false",
report_copy->channel->c_str(), report_copy->contact->c_str(),
report_copy->cookie_policy->c_str(),
url::Origin::Create(GURL(report_copy->report_url.value()))
.Serialize()
.c_str(),
Expand Down
Loading
Loading