A native menu and a native inject for adblock content picker

[atuchin-m]

Description

The epic: https://github.com/brave/brave-browser/issues/39610 The previous task: https://github.com/brave/brave-browser/issues/40696

Currently content picker typescript doesn't need Brave extension to work, but it's still injected via the background page. We need:

1. a native inject mechanism
2. a native menu instead of an extension-based
3. another impl of API to add a user-defined rule.

No changes in the feature logic are expected.

[atuchin-m]

@rebron To fix https://github.com/brave/brave-browser/issues/39610 we need to reimplement the adblock-related items in the menu, that is done in the PR https://github.com/brave/brave-core/pull/25411.

Here is some product-related questions to answer. Could you please take a look?

• Do we want to rename the menu items or the current state is ok?
• Do we still need to have Leo icon? We don't use it for other items.
• Where the menu Brave should be located? Before the PR we use the extension section, but it's not an extension-related anymore. The the PR I add the item after Print.
• Should we disable or hide the menu when Allows all trackers and ads is set in Shields? Currently we always show it, but added filters will not work if Shields are down.
• Should we hide the items in some menu mode (the text selected, the image, the link)? Before the PR we don't care, always show.

before	after

[rebron]

@rebron To fix #39610 we need to reimplement the adblock-related items in the menu, that is done in the PR brave/brave-core#25411.

Here is some product-related questions to answer. Could you please take a look?

• Do we want to rename the menu items or the current state is ok?

I'm wondering if we just bring in Block element up to the menu and we can remove Manage custom filters here. wdyt @ShivanKaul @antonok-edm There's probably still a better name for this. Content blocking if we're keeping the sub-menu.

• Do we still need to have Leo icon? We don't use it for other items.

It's fine to remove the Brave logo, especially if we do the above.

• Where the menu Brave should be located? Before the PR we use the extension section, but it's not an extension-related anymore. The the PR I add the item after Print.

Put it back to under Translate to English. Your current location may have Cast... as a menu item. And Create QR Code for this page has Send to your Devices if Sync is enabled.

• Should we disable or hide the menu when Allows all trackers and ads is set in Shields? Currently we always show it, but added filters will not work if Shields are down.

Always show. It's fine even with allow all tracker and ads to have this option.

• Should we hide the items in some menu mode (the text selected, the image, the link)? Before the PR we don't care, always show.

That makes sense to me. Don't show when selecting text and on a link. I think with an image, users may want to block that image.

[rebron]

• Should we disable or hide the menu when Allows all trackers and ads is set in Shields? Currently we always show it, but added filters will not work if Shields are down.

Always show. It's fine even with allow all tracker and ads to have this option.

Actually @ShivanKaul @antonok-edm. I guess with Shields down, blocking elements doesn't work then probably doesn't make sense to have this menu item available as an option.

[atuchin-m]

Actually @ShivanKaul @antonok-edm. I guess with Shields down, blocking elements doesn't work then probably doesn't make sense to have this menu item available as an option.

It's an issue now: if Shields are done, you can still select an item to block and it will be immediately blocked (not by adblock, but by injecting a CSS by the script). But after you reload the page the element will be still visible.

[atuchin-m]

option 1	option 2

Image menu position(before)	Image menu position(option 1)

[atuchin-m]

BTW, @rebron WDYT about replacing Block element to Block elements? The first sounds like:

1. you have to target to particular element, but it works in another way;
2. you can select only one element. In fact, a new rule often covers multiple items.

[rebron]

Sounds good. Can change it to Block elements for now.

[ShivanKaul]

This is fantastic work @atuchin-m! https://github.com/brave/brave-browser/issues/40821#issuecomment-2328467130 are all very pertinent questions.

I'll chat with @antonok-edm about the context menu label/behaviour (whether under Brave > or top-level) and get back to you by EOD.

Where the menu Brave should be located? Before the PR we use the extension section, but it's not an extension-related anymore. The the PR I add the item after Print.

No strong feelings here, but it might make more sense for it to be right above Inspect, since the next step, creating the filter by selecting it on the page, is in the same category of user actions as Inspecting an HTML element IMO.

Should we disable or hide the menu when Allows all trackers and ads is set in Shields? Currently we always show it, but added filters will not work if Shields are down.

You're right about the behaviour in https://github.com/brave/brave-browser/issues/40821#issuecomment-2330746626, it's definitely a bit confusing. It SGTM to just have it not appear if Ads & Tracker setting is Disabled (or allowed like you said). Shields down would override that setting too.

Should we hide the items in some menu mode (the text selected, the image, the link)? Before the PR we don't care, always show.

I see that a link is actually selectable/blockable currently. I propose hiding the item when text selected (where it doesn't show up, you can only block the div/para), but not in the other 2 situations.

[ShivanKaul]

Okay, here's a suggestion:

1. Move the Block elements to be top-level. Get rid of the Manage custom filters.
2. Add a Manage custom filters button alongside Create and Quit after the user selects an element.

[atuchin-m]

@rebron @ShivanKaul PTAL:

1. I've moved the menu in before the extension section (aka under Translate to ..);
2. The new button Manage filters is added. BTW, these button are not translatable now (the same for Create and Quit)
3. The menu item is visible iff:
   • the page url is http or https; i.e. it's hidden on brave://settings or on NTP.
   • no text is selected;
   • the selected element isn't link (no href);
   • if it's image we show the item despite the previous rule;
   • Shields isn't in Allows all trackers and ads state;

[ShivanKaul]

Thanks @atuchin-m.

1. Looking at the screenshots, I do think it would be better for Block elements to live in the View Page Source and Inspect section. If it's not too much trouble, let's make this change.
2. Just want to clarify the "text selection" behaviour: if I select text, and then right-click on the text, the text stays selected. I agree that it doesn't make sense to have the menu item be visible then. However, if I select text, and right-click somewhere else on the page, the text selection goes away. In this latter case, we do want the menu item to be visible. I'm pretty sure this is what you're saying, but wanted to double-check.
3. We do want links/hrefs to be blockable.

Everything else SGTM.

[atuchin-m]

@ShivanKaul thanks for the comment.

1. it looks harder to implement because there is no fixed anchor. Inspect could be invisible.
2. yep, we do show the menu item if the selection goes away in the click moment. The condition is about the opposite to Leo AI tool, you could try it yourself.
3. The current PR hides the item for text links (if it's not an image). We could change it to show it for all the links. Just to clarify that the content picker doesn't preselect the hovered element. It's a page level action like inspect: after clicking it a user have to select the element again.

[ShivanKaul]

Gotcha, that all seems fine then. I guess I can't think of a case where a user might want to block an item on the page that was a hyperlink but not an image, so we can leave it out for now.

[atuchin-m]

@ShivanKaul @rebron Ok, let's move the item, it isn't a rocket science. Also it is always shown for links.	blank space click	link click

[petemill]

IMO it should still have the Brave shields icon in the menu, but if that's been decided otherwise then fine.

[ShivanKaul]

Your rationale is that it's a Brave Shields feature? I considered that, just seems slightly inconsistent to have a Brave logo (which happens to be the same as the Shields logo) for this but not Leo context menu or Copy Clean Link.

[atuchin-m]

For QA: it makes sense to verify all the related changes together: https://github.com/brave/brave-core/pull/25330 https://github.com/brave/brave-core/pull/25358 https://github.com/brave/brave-core/pull/25411

Scenario 1:

1. Launch the browser, open any site (i.g reddit.com)
2. Right click on blank space and click Block elements
3. Click on the item, content picker mode should be activated.
4. Select a item and click Create.
5. Verify that the item is blocked immediately
6. Reload the page, verify the item is blocked
7. For any controversial behavior check v1.69.x (Brave => Block element) as the reference

Scenario 2 (Manage filters):

1. Select Block elements
2. Select any element, click Create
3. Click Block elements again, click Manage filters button.
4. Verify the shields settings are opened and there is a new rule related to the domain you test (i.e. reddit.com)

Case 3 (Menu position and show/hide logic). Verify that:

1. Block elements item is always above Inspect
2. the item is shown for any link we right-clicked and isn't shown for selected text.
3. the item is hidden when Shields are down
4. the item is hidden on NTP or brave://settings

[LaurenWags]

Adding QA/Blocked pending response from https://github.com/brave/brave-browser/issues/40696#issuecomment-2362112901

[kjozwiak]

The above requires 1.71.95 or higher for 1.71.x verification 👍

[issuant]

@atuchin-m @kjozwiak Will this be coming to Android? #33241

[MadhaviSeelam]

Verification PASSED using

Brave | 1.71.109 Chromium: 129.0.6668.89 (Official Build) beta (64-bit)
-- | --
Revision | 538f203fb4c3449f0684dbabd4ef6e777c4e4a3b
OS | Windows 11 Version 23H2 (Build 22631.4249)

Testcase 1: (Create)

1. Installed 1.71.109
2. launched the browser (relaunched to pull griffin)
3. visited nytimes.com
4. opened context menu in a blank space and clicked Block elements option
5. confirmed clicking Block elements activated the content picker mode
6. clicked on selected element and rules-box is activated
7. clicked Create in the rules-box
8. confirmed the element is blocked immediately
9. opened brave://settings/shields/filters page
10. confirmed the rule is written to the create custom filters box
11. Reloaded the page
12. confirmed the item is still blocked
13. opened brave://settings/shields/filters page again
14. confirmed rule is still shown
15. removed the rule and clicked save button
16. reloaded nytimes.com tab
17. confirmed blocked element reloaded

step 4	step 5	step 7	step 8	step 10	step 12	step 15	step 17

Testcase 2 (Manage filters):

1. continued from testcase 1 -visited nytimes.com
2. opened context menu in a blank space and clicked Block elements option
3. clicked on selected element and rules-box is activated
4. clicked create in the rules box
5. verified the selected element is blocked
6. confirmed clicking Create in the rules-box writes the rule into brave://settings/shields/filters without any issues
7. repeated step 4-5
8. clicked Manage filters button in the rules box
9. confirmed brave://settings/shields/filters page is opened and there is a new rule related to the domain you test (i.e. nytimes.com)

step 3	step 4	step 7	step 9

Testcase 3 (Menu position and show/hide logic).

1. continued from testcase 2
2. visited https://en.wikipedia.org/wiki/Main_Page in a new tab
3. confirmed Block elements item is always above Inspect via context menu
4. confirmed Block elements is being displayed via the context menu for links
5. confirmed Block elements is not being displayed for selected text in the context menu
6. confirmed Block elements is hidden on NTP
7. confirmed Block elements is hidden for brave://pages
8. confirmed Block elements is not being displayed when Allow all trackers & ads in a page
9. confirmed Block elements is being displayed when Block trackers & ads & Aggressive is selected

step 3	step 4	step 5	step 6	step 7a	step 7b

step 8a	step 8 b	step 9a	step 9b

[LaurenWags]

Verified with

Brave | 1.71.105 Chromium: 129.0.6668.89 (Official Build) beta (x86_64)
-- | --
Revision | f69ba167992437799f1dbcce81e90ba501a3d63e
OS | macOS Version 14.7 (Build 23H124)

Verified test plan from https://github.com/brave/brave-browser/issues/40821#issuecomment-2352446992.

Case 1 - PASSED

1. Launch the browser, open any site (i.g reddit.com) 2. Right click on blank space and click `Block elements`  3. Click on the item, content picker mode should be activated.  4. Select a item and click `Create`.  5. Verify that the item is blocked immediately  6. Reload the page, verify the item is blocked

Case 2 (Manage filters) - PASSED

1. Select `Block elements` 2. Select any element, click `Create`  |  --- | --- 3. Click `Block elements` again, click `Manage filters` button. 4. Verify the shields settings are opened and there is a new rule **related to the domain you test** (i.e. brave.com)  5. Confirmed if I remove the rule and select `Save changes` that when I go back to the page and reload, the element is shown again.  |  --- | ---

Case 3 (Menu position and show/hide logic) - PASSED

1. `Block elements` item is always above `Inspect` Clicked on a few different areas of brave.com to get different context menus to confirm that "Block elements" is above "Inspect" Right click on blank space | Right click on an image | Right click on a button --- | --- | ---  |  |  2. the item is shown for any link we right-clicked and isn't shown for selected text. Link | Text --- | ---  |  3. the item is hidden when Shields are down Note, per discussion via https://bravesoftware.slack.com/archives/C7VLGSR55/p1728502567294939?thread_ts=1728501796.503369&cid=C7VLGSR55, this is how this feature is working: - "Block Elements" visible when shields completely disabled - "Block Elements" not visible when shields set as "Allow all trackers & ads" - "Block Elements" visible when shields set as "Aggressively block trackers & ads" or "Block trackers & ads" Shields down | Aggressively block trackers & ads | Block trackers & ads | Allow all trackers & ads --- | --- | --- | ---  |  |  |  4. the item is hidden on NTP or brave://settings Confirmed no "Block elements" on NTP or brave://settings Example | Example | Example --- | --- | ---  |  | 

[LaurenWags]

Verified with

Brave | 1.71.112 Chromium: 130.0.6723.44 (Official Build) (64-bit)
-- | --
Revision | 7d51df656b247f9432ee714a6d160142a1e11c13
OS | Linux

Verified test plan from https://github.com/brave/brave-browser/issues/40821#issuecomment-2352446992.

Case 1 - PASSED

1. Launch the browser, open any site (i.g reddit.com) 2. Right click on blank space and click `Block elements`  3. Click on the item, content picker mode should be activated.  4. Select a item and click `Create`.  5. Verify that the item is blocked immediately  6. Reload the page, verify the item is blocked

Case 2 (Manage filters) - PASSED

1. Select `Block elements` 2. Select any element, click `Create`  |  --- | --- 3. Click `Block elements` again, click `Manage filters` button. 4. Verify the shields settings are opened and there is a new rule **related to the domain you test** (i.e. brianbondy.com)  5. Confirmed if I remove the rule and select `Save changes` that when I go back to the page and reload, the element is shown again.  |  --- | ---

Case 3 (Menu position and show/hide logic) - PASSED

1. `Block elements` item is always above `Inspect` Clicked on a few different areas of brave.com to get different context menus to confirm that "Block elements" is above "Inspect" Right click on blank space | Right click on an image | Right click on a button --- | --- | ---  |  |  2. the item is shown for any link we right-clicked and isn't shown for selected text. Link | Text --- | ---  |  3. the item is hidden when Shields are down Note, per discussion via https://bravesoftware.slack.com/archives/C7VLGSR55/p1728502567294939?thread_ts=1728501796.503369&cid=C7VLGSR55, this is how this feature is working: - "Block Elements" visible when shields completely disabled - "Block Elements" not visible when shields set as "Allow all trackers & ads" - "Block Elements" visible when shields set as "Aggressively block trackers & ads" or "Block trackers & ads" Shields down | Aggressively block trackers & ads | Block trackers & ads | Allow all trackers & ads --- | --- | --- | ---  |  |  |  4. the item is hidden on NTP or brave://settings Confirmed no "Block elements" on NTP or brave://settings Example | Example | Example --- | --- | ---  |  |