-
Notifications
You must be signed in to change notification settings - Fork 871
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
Added adblock info tab. #25490
Added adblock info tab. #25490
Conversation
b5c6a38
to
47aebcc
Compare
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
A Storybook has been deployed to preview UI for the latest push |
766d4de
to
7dc5963
Compare
378c674
to
da8de55
Compare
components/brave_shields/content/browser/domain_block_navigation_throttle.h
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/third_party/typescript/ts_library.py
Outdated
Show resolved
Hide resolved
...es/third_party/devtools-frontend/src/front_end-core-protocol_client-protocol_client.ts.patch
Outdated
Show resolved
Hide resolved
patches/third_party/devtools-frontend/src/third_party-typescript-typescript.gni.patch
Outdated
Show resolved
Hide resolved
third_party/devtools-frontend/src/front_end/panels/network/RequestAdblockView.ts
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/third_party/typescript/ts_library.py
Show resolved
Hide resolved
@boocmp this is looking great. One question: if a network request would have been blocked by adblock-rust but isn't because of an exception rule, would that show up? Those requests would be very useful to highlight. Maybe those could be in green (since they are allowed)? Also, we would like to eventually build the ability to detect which adblock rules were inserted into a page, and which were active, including cosmetic rules and scriplets. That definitely is beyond the scope of this PR, but just wanted to put it out there in case we can structure the code to help with that. I would imagine that could live in a new |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
script/brave_license_helper.py
change 👍
da8de55
to
fc5588d
Compare
@ShivanKaul I've added green highlighting of exceptions. |
8c08f78
to
f84c633
Compare
50dcb4d
to
f23857a
Compare
patches/third_party/devtools-frontend/src/front_end-panels-network-NetworkItemView.ts.patch
Outdated
Show resolved
Hide resolved
...es/third_party/devtools-frontend/src/front_end-core-protocol_client-protocol_client.ts.patch
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/front_end/panels/network/NetworkDataGridNode.ts
Outdated
Show resolved
Hide resolved
patches/third_party/devtools-frontend/src/third_party-typescript-typescript.gni.patch
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/scripts/build/generate_devtools_grd.py
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/third_party/typescript/ts_library.py
Outdated
Show resolved
Hide resolved
chromium_src/third_party/devtools-frontend/src/front_end/core/sdk/NetworkManager.ts
Outdated
Show resolved
Hide resolved
chromium_src/third_party/blink/public/devtools_protocol/browser_protocol.pdl
Show resolved
Hide resolved
f1b368e
to
60381dd
Compare
build/commands/lib/config.js
Outdated
@@ -591,6 +591,12 @@ Config.prototype.buildArgs = function () { | |||
args.enable_brave_page_graph_webapi_probes = false | |||
} | |||
|
|||
// Devtools: Now we patch devtools frontend, so it is usefull to see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useful
std::optional<std::string> rewritten_url; | ||
}; | ||
|
||
CONTENT_EXPORT void SendAdblockInfo(int frame_tree_node_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pls write a comment for the functions.
They repost to the UI thread, that's also is worth mentioning here
components/brave_shields/content/browser/domain_block_navigation_throttle.cc
Outdated
Show resolved
Hide resolved
f1d397c
to
8cffc78
Compare
[puLL-Merge] - brave/brave-core@25490 DescriptionThis PR adds adblock information to the DevTools Network panel, allowing developers to see details about adblock decisions for network requests. The changes include new backend instrumentation to capture adblock info, modifications to the DevTools protocol to support sending this data, and frontend changes to display the information in the UI. ChangesChanges
Possible Issues
Security HotspotsNo significant security hotspots were identified in this change. The adblock information exposure is limited to the DevTools, which is already a privileged interface. |
Resolves brave/brave-browser#6458
Submitter Checklist:
QA/Yes
orQA/No
;release-notes/include
orrelease-notes/exclude
;OS/...
) to the associated issuenpm run test -- brave_browser_tests
,npm run test -- brave_unit_tests
wikinpm run presubmit
wiki,npm run gn_check
,npm run tslint
git rebase master
(if needed)Reviewer Checklist:
gn
After-merge Checklist:
changes has landed on
Test Plan: