Skip to content

Commit

Permalink
force custom rules to be used regardless of 1p content
Browse files Browse the repository at this point in the history
  • Loading branch information
antonok-edm committed Feb 21, 2020
1 parent 505d68f commit ef7f9b9
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 33 deletions.
36 changes: 17 additions & 19 deletions browser/extensions/api/brave_shields_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -69,15 +69,21 @@ 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<base::Value> custom_resources = g_brave_browser_process->
ad_block_custom_filters_service()->
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<base::ListValue>();
Expand All @@ -93,7 +99,7 @@ BraveShieldsHiddenClassIdSelectorsFunction::Run() {
brave_shields::HiddenClassIdSelectors::Params::Create(*args_));
EXTENSION_FUNCTION_VALIDATE(params.get());

base::Optional<base::Value> selectors = g_brave_browser_process->
base::Optional<base::Value> hide_selectors = g_brave_browser_process->
ad_block_service()->HiddenClassIdSelectors(params->classes,
params->ids,
params->exceptions);
Expand All @@ -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<base::Value> custom_selectors = g_brave_browser_process->
Expand All @@ -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<base::ListValue>();

return RespondNow(OneArgument(std::make_unique<base::Value>(
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)));
}


Expand Down
12 changes: 9 additions & 3 deletions common/extensions/api/brave_shields.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
}
}
]
Expand Down Expand Up @@ -125,7 +126,12 @@
"name": "callback",
"parameters": [
{
"name": "selectorsJson",
"name": "selectors",
"type": "array",
"items": {"type": "string"}
},
{
"name": "forceHideSelectors",
"type": "array",
"items": {"type": "string"}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
})
}
})
}

Expand All @@ -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'
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
20 changes: 18 additions & 2 deletions components/brave_shields/browser/ad_block_service_helper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,24 @@ std::vector<FilterList>::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) {
Expand Down
2 changes: 1 addition & 1 deletion components/brave_shields/browser/ad_block_service_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ std::vector<adblock::FilterList>::const_iterator FindAdBlockFilterListByLocale(
const std::vector<adblock::FilterList>& 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

Expand Down
53 changes: 48 additions & 5 deletions components/brave_shields/browser/cosmetic_merge_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<base::Value> a_val = base::JSONReader::Read(a);
ASSERT_TRUE(a_val);
Expand All @@ -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);
}
Expand Down Expand Up @@ -69,7 +70,7 @@ TEST_F(CosmeticResourceMergeTest, MergeTwoEmptyResources) {
"\"injected_script\": \"\n\""
"}";

CompareMergeFromStrings(a, b, expected);
CompareMergeFromStrings(a, b, false, expected);
}

TEST_F(CosmeticResourceMergeTest, MergeEmptyIntoNonEmpty) {
Expand All @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
}


Expand Down
3 changes: 2 additions & 1 deletion components/definitions/chromel.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit ef7f9b9

Please sign in to comment.