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

don't override website's document.adoptedStyleSheets #8081

Merged
merged 1 commit into from
Feb 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions chromium_src/chrome/common/chrome_isolated_world_ids.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at http://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_
#define BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_
#ifndef BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_H_
#define BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_H_

#define ISOLATED_WORLD_ID_CHROME_INTERNAL \
ISOLATED_WORLD_ID_CHROME_INTERNAL, ISOLATED_WORLD_ID_BRAVE_INTERNAL
Expand All @@ -13,4 +13,4 @@

#undef ISOLATED_WORLD_ID_CHROME_INTERNAL

#endif // BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_
#endif // BRAVE_CHROMIUM_SRC_CHROME_COMMON_CHROME_ISOLATED_WORLD_IDS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@ const char kHideSelectorsInjectScript[] =
if (!document.adoptedStyleSheets.includes(
window.content_cosmetic.cosmeticStyleSheet)) {
document.adoptedStyleSheets =
[window.content_cosmetic.cosmeticStyleSheet];
[window.content_cosmetic.cosmeticStyleSheet,
...document.adoptedStyleSheets];
Copy link
Collaborator

Choose a reason for hiding this comment

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

if this can overwrite a website's stylesheet, is it possible for websites to "clear" cosmetic filtering that we set this way too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is

Copy link
Member Author

Choose a reason for hiding this comment

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

perhaps a way to prevent that is to check and reapply when we do passes for 1st party to unhide

};
})();)";

Expand All @@ -112,7 +113,8 @@ const char kForceHideSelectorsInjectScript[] =
if (!document.adoptedStyleSheets.includes(
window.content_cosmetic.cosmeticStyleSheet)) {
document.adoptedStyleSheets =
[window.content_cosmetic.cosmeticStyleSheet];
[window.content_cosmetic.cosmeticStyleSheet,
...document.adoptedStyleSheets];
};
})();)";

Expand Down Expand Up @@ -144,7 +146,8 @@ const char kStyleSelectorsInjectScript[] =
if (!document.adoptedStyleSheets.includes(
window.content_cosmetic.cosmeticStyleSheet)){
document.adoptedStyleSheets =
[window.content_cosmetic.cosmeticStyleSheet];
[window.content_cosmetic.cosmeticStyleSheet,
...document.adoptedStyleSheets];
};
})();)";

Expand Down Expand Up @@ -270,10 +273,11 @@ bool CosmeticFiltersJSHandler::EnsureConnected() {
}

void CosmeticFiltersJSHandler::ProcessURL(const GURL& url) {
if (!EnsureConnected())
url_ = url;
// Trivially, don't make exceptions for malformed URLs.
if (!EnsureConnected() || url_.is_empty() || !url_.is_valid())
return;

url_ = url;
cosmetic_filters_resources_->ShouldDoCosmeticFiltering(
url_.spec(),
base::BindOnce(&CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering,
Expand Down Expand Up @@ -329,10 +333,6 @@ void CosmeticFiltersJSHandler::OnUrlCosmeticResources(base::Value result) {

void CosmeticFiltersJSHandler::CSSRulesRoutine(
base::DictionaryValue* resources_dict) {
// Trivially, don't make exceptions for malformed URLs.
if (url_.is_empty() || !url_.is_valid())
return;

// Otherwise, if its a vetted engine AND we're not in aggressive
// mode, also don't do cosmetic filtering.
if (!enabled_1st_party_cf_ && IsVettedSearchEngine(url_))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,7 @@ namespace cosmetic_filters {

namespace {

const char kSecurityOrigin[] =
"chrome-extension://mnojpmjdmbbfmejpflffifhffcmidifd";
const char kSecurityOrigin[] = "chrome://cosmetic_filters";

void EnsureIsolatedWorldInitialized(int world_id) {
static base::Optional<int> last_used_world_id;
Expand Down Expand Up @@ -69,15 +68,12 @@ void CosmeticFiltersJsRenderFrameObserver::DidCreateScriptContext(
}

void CosmeticFiltersJsRenderFrameObserver::DidCreateNewDocument() {
// There could be empty and "about:blank" URLs, empty URLs are duplicated
// with DidCreateDocumentElement, so we just skip them, "about:blank"
// should fallback to the main frame rules
if (url_.is_empty())
return;
if (url_.spec() == "about:blank") {
// There could be empty, invalid and "about:blank" URLs,
// they should fallback to the main frame rules
if (url_.is_empty() || !url_.is_valid() || url_.spec() == "about:blank")
url_ = url::Origin(render_frame()->GetWebFrame()->GetSecurityOrigin())
.GetURL();
}

if (!native_javascript_handle_) {
native_javascript_handle_.reset(
new CosmeticFiltersJSHandler(render_frame(), isolated_world_id_));
Expand Down