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

Top-level document blocking modal should not be applied to iframes #14826

Closed
antonok-edm opened this issue Mar 18, 2021 · 4 comments · Fixed by brave/brave-core#8389
Closed
Assignees
Labels
feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. privacy-pod Feature work for the Privacy & Web Compatibility pod QA Pass - Android ARM QA Pass - Android Tab QA Pass-macOS QA/Yes release-notes/exclude

Comments

@antonok-edm
Copy link
Collaborator

The "suspicious site ahead" modal should not be rendered in iframes that are blocked - the "Proceed" and "Go back" buttons do not do anything, and filter lists are not designed to block "potentially useful" frames.

image

Eventually, these should be hidden, but in the meantime it's better to render them with the less-conspicuous "broken document" page that is default in Chromium (and was the behavior before top-level-document blocking was implemented).

@antonok-edm antonok-edm added feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop labels Mar 18, 2021
@pes10k pes10k added the privacy-pod Feature work for the Privacy & Web Compatibility pod label Mar 18, 2021
@ryanbr
Copy link

ryanbr commented Mar 20, 2021

Shouldn't block $third-party either, which could cause a quite a few false positives. (uBO parity)

third-party

https://t.myvisualiq.net/click_pixel?et=c&ago=212&ao=546&aca=71700000079900483&si=-2&ci=-2&pi=-2&ad=58700006712595232&sv1=92700060738722924&advt=-2&chnl=-2&vndr=1195&sz=6583&u=Cj0KCQjwutaCBhDfARIsAJHWnHsiNkwZDWLZOyZzfYNBG34i_P7ZlsBnTsxbAumgOfkEnqliqLQvgGsaAtiJEALw_wcB&red=https://www.bestbuy.com/site/samsung-27-4-cu-ft-side-by-side-refrigerator-stainless-steel/6397576.p?skuId=6397576&ref=212&loc=1&ref=212&loc=1&gclsrc=ds

(Changed to $script and $image): easylist/easylist@1da86a1

@rebron rebron added the priority/P2 A bad problem. We might uplift this to the next planned release. label Mar 26, 2021
@antonok-edm
Copy link
Collaborator Author

antonok-edm commented Mar 26, 2021

@ryanbr I suspect that block is from a different filter; none of the other sites nearby in the list from easylist/easylist@1da86a1 are causing the same modal.

edit: indeed, myvisualiq.net appears in Peter Lowe's list

@stephendonner
Copy link

Verified FIXED using

Brave 1.24.47 Chromium: 90.0.4430.41 (Official Build) nightly (x86_64)
Revision e9c92b1eaca8487e212f3f6bc081fdb6d4863759-refs/branch-heads/4430@{#723}
OS macOS Version 11.2.3 (Build 20D91)

Steps:

  1. loaded https://apps.facebook.com/wordswithfriends with Shields down
  2. loaded speedtest.net with Shields down
facebook.com - fixed speedtest.net - fixed facebook.com - before speedtest.net - before
Screen Shot 2021-04-05 at 11 02 24 AM Screen Shot 2021-04-05 at 10 55 16 AM Screen Shot 2021-04-05 at 11 03 42 AM

@stephendonner stephendonner added QA Pass-macOS and removed QA/In-Progress Indicates that QA is currently in progress for that particular issue labels Apr 5, 2021
@kjozwiak kjozwiak added this to the 1.23.x - Beta milestone Apr 6, 2021
@srirambv
Copy link
Contributor

srirambv commented Apr 14, 2021

Verification passed on OnePlus 6T with Android 10 running 1.23.70 x64 build

Facebook.com Speetest.net
image image

Verification passed on Samsung Tab A with Android 10 running 1.23.70 x64 build

Facebook.com Speetest.net
image image

Note: For Android ARM had to enable Cosmetic filtering for Facebook app to load

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/shields/adblock Blocking ads & trackers with Shields OS/Android Fixes related to Android browser functionality OS/Desktop priority/P2 A bad problem. We might uplift this to the next planned release. privacy-pod Feature work for the Privacy & Web Compatibility pod QA Pass - Android ARM QA Pass - Android Tab QA Pass-macOS QA/Yes release-notes/exclude
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants