From e61911581c4a5d879dada6a3f1e8ba15bc1c0b3d Mon Sep 17 00:00:00 2001 From: Anton Lazarev Date: Thu, 20 Feb 2020 23:08:29 -0500 Subject: [PATCH] force custom rules to be used regardless of 1p content --- browser/extensions/api/brave_shields_api.cc | 36 ++++++------- common/extensions/api/brave_shields.json | 12 +++-- .../background/api/cosmeticFilterAPI.ts | 16 +++++- .../ad_block_regional_service_manager.cc | 2 +- .../browser/ad_block_service_helper.cc | 20 ++++++- .../browser/ad_block_service_helper.h | 2 +- .../browser/cosmetic_merge_unittest.cc | 53 +++++++++++++++++-- components/definitions/chromel.d.ts | 3 +- 8 files changed, 111 insertions(+), 33 deletions(-) diff --git a/browser/extensions/api/brave_shields_api.cc b/browser/extensions/api/brave_shields_api.cc index 20b96f1da6d8..fbd4517d3f4d 100644 --- a/browser/extensions/api/brave_shields_api.cc +++ b/browser/extensions/api/brave_shields_api.cc @@ -69,7 +69,10 @@ BraveShieldsHostnameCosmeticResourcesFunction::Run() { HostnameCosmeticResources(params->hostname); if (regional_resources && regional_resources->is_dict()) { - ::brave_shields::MergeResourcesInto(&*resources, &*regional_resources); + ::brave_shields::MergeResourcesInto( + &*resources, + &*regional_resources, + false); } base::Optional custom_resources = g_brave_browser_process-> @@ -77,7 +80,10 @@ BraveShieldsHostnameCosmeticResourcesFunction::Run() { HostnameCosmeticResources(params->hostname); if (custom_resources && custom_resources->is_dict()) { - ::brave_shields::MergeResourcesInto(&*resources, &*custom_resources); + ::brave_shields::MergeResourcesInto( + &*resources, + &*custom_resources, + true); } auto result_list = std::make_unique(); @@ -93,7 +99,7 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { brave_shields::HiddenClassIdSelectors::Params::Create(*args_)); EXTENSION_FUNCTION_VALIDATE(params.get()); - base::Optional selectors = g_brave_browser_process-> + base::Optional hide_selectors = g_brave_browser_process-> ad_block_service()->HiddenClassIdSelectors(params->classes, params->ids, params->exceptions); @@ -104,16 +110,16 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { params->ids, params->exceptions); - if (selectors && selectors->is_list()) { + if (hide_selectors && hide_selectors->is_list()) { if (regional_selectors && regional_selectors->is_list()) { for (auto i = regional_selectors->GetList().begin(); i < regional_selectors->GetList().end(); i++) { - selectors->Append(std::move(*i)); + hide_selectors->Append(std::move(*i)); } } } else { - selectors = std::move(regional_selectors); + hide_selectors = std::move(regional_selectors); } base::Optional custom_selectors = g_brave_browser_process-> @@ -122,20 +128,12 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() { params->ids, params->exceptions); - if (selectors && custom_selectors->is_list()) { - if (custom_selectors && custom_selectors->is_list()) { - for (auto i = custom_selectors->GetList().begin(); - i < custom_selectors->GetList().end(); - i++) { - selectors->Append(std::move(*i)); - } - } - } else { - selectors = std::move(custom_selectors); - } + auto result_list = std::make_unique(); - return RespondNow(OneArgument(std::make_unique( - std::move(*selectors)))); + result_list->GetList().push_back(std::move(*hide_selectors)); + result_list->GetList().push_back(std::move(*custom_selectors)); + + return RespondNow(ArgumentList(std::move(result_list))); } diff --git a/common/extensions/api/brave_shields.json b/common/extensions/api/brave_shields.json index f3e859b65ffa..f28c0eb14275 100644 --- a/common/extensions/api/brave_shields.json +++ b/common/extensions/api/brave_shields.json @@ -90,10 +90,11 @@ "name": "hostnameSpecificResources", "type": "object", "properties": { - "hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page"}, + "hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page if they are determined to not include 1st party content"}, "style_selectors": {"type": "object", "additionalProperties": {"type": "array", "items": {"type": "string"}}, "description": "Hostname-specific CSS selectors that should be restyled, with their associated CSS style rules"}, "exceptions": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific overrides for generic cosmetic blocking selectors"}, - "injected_script": {"type": "string", "description": "A script to inject as the page is loading"} + "injected_script": {"type": "string", "description": "A script to inject as the page is loading"}, + "force_hide_selectors": {"type": "array", "items": {"type": "string"}, "description": "Hostname-specific CSS selectors that should be hidden from the page"} } } ] @@ -125,7 +126,12 @@ "name": "callback", "parameters": [ { - "name": "selectorsJson", + "name": "selectors", + "type": "array", + "items": {"type": "string"} + }, + { + "name": "forceHideSelectors", "type": "array", "items": {"type": "string"} } diff --git a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts index bec58c3d30fe..ec13a2894ee4 100644 --- a/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts +++ b/components/brave_extension/extension/brave_extension/background/api/cosmeticFilterAPI.ts @@ -19,8 +19,18 @@ const informTabOfCosmeticRulesToConsider = (tabId: number, selectors: string[]) // Fires when content-script calls hiddenClassIdSelectors export const injectClassIdStylesheet = (tabId: number, classes: string[], ids: string[], exceptions: string[]) => { - chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (selectors) => { + chrome.braveShields.hiddenClassIdSelectors(classes, ids, exceptions, (selectors, forceHideSelectors) => { informTabOfCosmeticRulesToConsider(tabId, selectors) + + if (forceHideSelectors.length > 0) { + const forceHideStylesheet = forceHideSelectors.join(',') + '{display:none!important;}\n' + + chrome.tabs.insertCSS(tabId, { + code: forceHideStylesheet, + cssOrigin: 'user', + runAt: 'document_start' + }) + } }) } @@ -33,7 +43,11 @@ export const applyAdblockCosmeticFilters = (tabId: number, hostname: string) => } informTabOfCosmeticRulesToConsider(tabId, resources.hide_selectors) + let styledStylesheet = '' + if (resources.force_hide_selectors.length > 0) { + styledStylesheet += resources.force_hide_selectors.join(',') + '{display:none!important;}\n' + } for (const selector in resources.style_selectors) { styledStylesheet += selector + '{' + resources.style_selectors[selector].join(';') + ';}\n' } diff --git a/components/brave_shields/browser/ad_block_regional_service_manager.cc b/components/brave_shields/browser/ad_block_regional_service_manager.cc index 0500cf2638cf..859abea27ca1 100644 --- a/components/brave_shields/browser/ad_block_regional_service_manager.cc +++ b/components/brave_shields/browser/ad_block_regional_service_manager.cc @@ -206,7 +206,7 @@ AdBlockRegionalServiceManager::HostnameCosmeticResources( it->second->HostnameCosmeticResources(hostname); if (first_value) { if (next_value) { - MergeResourcesInto(&*first_value, &*next_value); + MergeResourcesInto(&*first_value, &*next_value, false); } } else { first_value = std::move(next_value); diff --git a/components/brave_shields/browser/ad_block_service_helper.cc b/components/brave_shields/browser/ad_block_service_helper.cc index f51cece837a7..92f9355df32b 100644 --- a/components/brave_shields/browser/ad_block_service_helper.cc +++ b/components/brave_shields/browser/ad_block_service_helper.cc @@ -48,8 +48,24 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( // Merges the contents of the second HostnameCosmeticResources Value into the // first one provided. -void MergeResourcesInto(base::Value* into, base::Value* from) { - base::Value* resources_hide_selectors = into->FindKey("hide_selectors"); +// +// If `force_hide` is true, the contents of `from`'s `hide_selectors` field +// will be moved into a possibly new field of `into` called +// `force_hide_selectors`. +void MergeResourcesInto( + base::Value* into, + base::Value* from, + bool force_hide) { + base::Value* resources_hide_selectors = nullptr; + if (force_hide) { + resources_hide_selectors = into->FindKey("force_hide_selectors"); + if (!resources_hide_selectors || !resources_hide_selectors->is_list()) { + into->SetPath("force_hide_selectors", base::ListValue()); + resources_hide_selectors = into->FindKey("force_hide_selectors"); + } + } else { + resources_hide_selectors = into->FindKey("hide_selectors"); + } base::Value* from_resources_hide_selectors = from->FindKey("hide_selectors"); if (resources_hide_selectors && from_resources_hide_selectors) { diff --git a/components/brave_shields/browser/ad_block_service_helper.h b/components/brave_shields/browser/ad_block_service_helper.h index e7471d547528..7ce4e8b20122 100644 --- a/components/brave_shields/browser/ad_block_service_helper.h +++ b/components/brave_shields/browser/ad_block_service_helper.h @@ -21,7 +21,7 @@ std::vector::const_iterator FindAdBlockFilterListByLocale( const std::vector& region_lists, const std::string& locale); -void MergeResourcesInto(base::Value* into, base::Value* from); +void MergeResourcesInto(base::Value* into, base::Value* from, bool force_hide); } // namespace brave_shields diff --git a/components/brave_shields/browser/cosmetic_merge_unittest.cc b/components/brave_shields/browser/cosmetic_merge_unittest.cc index 82dcec44b294..893c5ccaf261 100644 --- a/components/brave_shields/browser/cosmetic_merge_unittest.cc +++ b/components/brave_shields/browser/cosmetic_merge_unittest.cc @@ -20,6 +20,7 @@ class CosmeticResourceMergeTest : public testing::Test { void CompareMergeFromStrings( const std::string& a, const std::string& b, + bool force_hide, const std::string& expected) { base::Optional a_val = base::JSONReader::Read(a); ASSERT_TRUE(a_val); @@ -31,7 +32,7 @@ class CosmeticResourceMergeTest : public testing::Test { base::JSONReader::Read(expected); ASSERT_TRUE(expected_val); - MergeResourcesInto(&*a_val, &*b_val); + MergeResourcesInto(&*a_val, &*b_val, force_hide); ASSERT_EQ(*a_val, *expected_val); } @@ -69,7 +70,7 @@ TEST_F(CosmeticResourceMergeTest, MergeTwoEmptyResources) { "\"injected_script\": \"\n\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) { @@ -85,7 +86,7 @@ TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) { "\"injected_script\": \"console.log('g')\n\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoEmpty) { @@ -101,7 +102,7 @@ TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoEmpty) { "\"injected_script\": \"\nconsole.log('g')\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); } TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoNonEmpty) { @@ -125,7 +126,49 @@ TEST_F(CosmeticResourceMergeTest, MergeNonEmptyIntoNonEmpty) { "\"injected_script\": \"console.log('g')\nconsole.log('n')\"" "}"; - CompareMergeFromStrings(a, b, expected); + CompareMergeFromStrings(a, b, false, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeEmptyForceHide) { + const std::string a = EMPTY_RESOURCES; + const std::string b = EMPTY_RESOURCES; + + // Same as EMPTY_RESOURCES, but with an additional newline in the + // injected_script and a new empty `force_hide_selectors` array + const std::string expected = "{" + "\"hide_selectors\": [], " + "\"style_selectors\": {}, " + "\"exceptions\": [], " + "\"injected_script\": \"\n\"," + "\"force_hide_selectors\": []" + "}"; + + CompareMergeFromStrings(a, b, true, expected); +} + +TEST_F(CosmeticResourceMergeTest, MergeNonEmptyForceHide) { + const std::string a = NONEMPTY_RESOURCES; + const std::string b = "{" + "\"hide_selectors\": [\"h\", \"i\"], " + "\"style_selectors\": {\"j\": \"color: #eee\", \"k\": \"color: #111\"}, " + "\"exceptions\": [\"l\", \"m\"], " + "\"injected_script\": \"console.log('n')\"" + "}"; + + const std::string expected = "{" + "\"hide_selectors\": [\"a\", \"b\"], " + "\"style_selectors\": {" + "\"c\": \"color: #fff\", " + "\"d\": \"color: #000\", " + "\"j\": \"color: #eee\", " + "\"k\": \"color: #111\"" + "}, " + "\"exceptions\": [\"e\", \"f\", \"l\", \"m\"], " + "\"injected_script\": \"console.log('g')\nconsole.log('n')\"," + "\"force_hide_selectors\": [\"h\", \"i\"]" + "}"; + + CompareMergeFromStrings(a, b, true, expected); } diff --git a/components/definitions/chromel.d.ts b/components/definitions/chromel.d.ts index 88122ad6b3eb..d1081c5e2647 100644 --- a/components/definitions/chromel.d.ts +++ b/components/definitions/chromel.d.ts @@ -218,9 +218,10 @@ declare namespace chrome.braveShields { style_selectors: any exceptions: string[] injected_script: string + force_hide_selectors: string[] } const hostnameCosmeticResources: (hostname: string, callback: (resources: HostnameSpecificResources) => void) => void - const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectors: string[]) => void) => void + const hiddenClassIdSelectors: (classes: string[], ids: string[], exceptions: string[], callback: (selectors: string[], forceHideSelectors: string[]) => void) => void type BraveShieldsViewPreferences = { showAdvancedView: boolean