new Shields

[cezaraugusto]

npm run test-unit

closes brave/brave-browser#1339
closes brave/brave-browser#771
closes brave/brave-browser#507
closes brave/brave-browser#471

[bsclifton]

Some notes for reviewers after @cezaraugusto, @tomlowenthal, and myself met 😄

• This PR should functionally match what is in place today
• brave-core has been missing the Allow until restart option, which is captured in Support allow-until-restart option of per-origin noscript settings brave-browser#188
• both Muon and brave-core are missing the "Allow forever" option, which is captured in Support allow-forever option in per-origin noscript settings brave-browser#1889

[bsclifton]

Nitpick: We should probably hide the Allow all and/or Block all if there are 0 items blocked

[bsclifton]

Even when I hover over these entries (blocked resources), there doesn’t appear to be a way to either see the full resource or (when selecting manually and copy/pasting) copying the full resource to the clipboard

My suggestion here would be to show the full resource name and then wrap to next line, like the current implementation:

[bsclifton]

Kind of a weird white box shown after the origin when you inspect scripts blocked:

UPDATE: on Windows, it becomes a little more obvious (a scrollbar is being added?!?); maybe this is due to overflow?

[bsclifton]

Nitpick: kind of a weird looking focus ring? Important to show I believe, for keyboard users. But the ring extends itself into the counter

[bsclifton]

Here’s one that seems to be Windows specific; and since this is supposed to be a native drop-down, I’m not sure what is happening?
(text is a little hard to read)

Existing one (in our live release) doesn't seem to have any color issues:

[bsclifton]

Tested on macOS and Windows (needs a Linux test). Several issues called out- the ability to see full resource name (ex: for copy/pasting) is important... same with the weird boxes / color being off for select box. The nitpick items are nice-to-haves IMO, but not blockers

Great job with this 😄 I can re-review after you're able to check out those items

[diracdeltas]

when i go to vox.com with scripts blocked and select 'allow once' for all scripts, it still shows 2 scripts blocked no matter how many times i repeat the 'allow all' process:

when i expand the scripts section, it shows only one script blocked from origin null/ :(

[diracdeltas]

noscript null origin issue: #78 (comment)

[yrliou]

reproduced #78 (comment) in current brave-core and browser-laptop, so I think we should open a separate issue to track this one.

browser-laptop:

current brave-core:

[bsclifton]

@diracdeltas since that behavior was present before, I created brave/brave-browser#1901 to track it

While this new approach might make that more visible, the bug was present before. I'd like to propose approving and then fixing in 0.57.x. We could then ask folks if uplifting the fix to 0.56.x would be appropriate

[bsclifton]

Works great after the fixes! 😄Awesome job on this one

[yrliou]

codes LGTM, tested on linux and mac.
It would be nice to have the updated design for blocked scripts list from @karenkliu because the current UI would confuse users and also we would lost details of blocked scripts that we currently have.
I'll let others to decide if it should block this PR, cc @bsclifton @tomlowenthal @karenkliu

follow up opened